Re: Operation can be dispatching in only one type

["Randy Brukardt" <randy@rrsoftware.com>]

"Dmitry A. Kazakov" <mailbox@dmitry-kazakov.de> wrote in message 
news:qchmzfxola9n$.1owojhwd91cbc.dlg@40tude.net...
> On Thu, 3 Dec 2009 17:08:01 -0600, Randy Brukardt wrote:
>
>> It should be noted that our local coding standard would consider the 
>> above
>> incorrect; there always must be an else branch or at least a comment
>> explaining why there is no else branch:
>
> What about:
>
>   if Read_Error (File) then
>      raise Data_Error;
>   end if;

I hardly ever write this. I looked at 15 units picked at random and found 
only a handful of raises, most of them in case statements. And some of the 
"if" statements have elses or elsifs anyway. But the couple of examples I 
did find, I did indeed violate the coding standard, because there is no need 
to describe the else case. The beginning of our coding standard says that it 
is guidelines, not something to be followed blindly, and adding noise 
comments surely would qualify.

The one place where this is fairly common (the heading of Claw functions) 
would have been much better written as preconditions (which Ada 95 didn't 
have, of course):

      if not Is_Valid (Window) then
           raise Not_Valid_Error;
      end if;

doesn't need any explanation, but it would be better to have it clearly in 
the spec rather than just in comments. Writing in Ada 2012 as proposed today 
(and that might change before it gets standardized):

     procedure Do_Something (Window : Claw.Basic_Window_Type; ...)
             with Pre => Is_Valid (Window);

is surely better because it puts the check in the spec where the caller can 
see it, rather than in comments that can be ignored. (And it also opens up 
the possibility of tools/compilers warning when it is *not* true at the 
point of a call, both potentially eliminating generated code and making 
error detection earlier.)

> or
>
>   if Estimated_Error <= Threshold then
>      return Estimated_Result;
>   end if;

This should always have an else comment, IMHO. In the example you have:

    if if Estimated_Error <= Threshold then
        return Estimated_Result;
   -- else continue iteration
   end if;

because the fact that iteration is intentionally being continued is 
important. I always try to comment why and how loops are exited, because I 
find I can't figure it out easily when returning to the code in a year or 
five.

> or "exit when", which is a hidden if without else.

I also find that it is very rare that I can use "exit when". If I can, I 
would prefer to do so, as it doesn't need much commenting (continuation is 
obvious). But almost always I find that something needs to be done (usually 
to save the result of the iteration) before exiting for good, so I usually 
end up with an if statement:

     for I in Data'Range loop
         exit when Item = Buffer (I);
    end loop;
    -- Oops, we don't know where the item was found or *if* it was found, 
which was the point of the iteration.

So instead:

    Found := Data'Last + 1;
    for I in Data'Range loop
        if Item = Buffer (I) then
            Found := I;
            exit;
        -- else keep looking
        end if;
    end loop;
    -- Found now tells us where the item was, or if it was absent.

which is a pattern that seems to happen a lot in my programs.

> Maybe "if" with two alternatives should better be "case". Though it would
> look strange most of us:
>
>   case Condition is
>      when True =>
>         X := This;
>      when False =>
>         X := That;
>   end case;

That's not *that* weird; it's not unusual to find out that there are more 
than two possibilities and a case statement is often the best way to handle 
that.

Anyway, this is my (and by extension, RR's) coding standard. YMMV.

                           Randy.