From: Randy Brukardt on
"Georg Bauhaus" <rm.dash-bauhaus(a)futureapps.de> wrote in message
news:4b179ffb$0$6591$9b4e6d93(a)newsspool3.arcor-online.net...
> Randy Brukardt schrieb:
>> [...]
>> This is not at all a simple topic, and the sort of simple rule used
>> by Java does little other than give a false sense of security (and a lot
>> of
>> annoying errors in code that has no problems).
>
> After reading your recent hints to compatibility and single
> pass compilation preventing flow analysis for legality decisions,
> I noticed the possibility of a restrictions pragma, too.
>
> Is it really annoying when programmers are forced to state,
> explicitly, what they seem to be knowing? That is, for example,
> "I know that no ELSE is missing in the following code,
> therefore I have omitted a no-op else branch"
>
> X : Some_Type;
> begin
> if I_Know_What_This_Does (P) then
> X := Assign_It;
> end if;
>
> Use_It := Involve (X);
>
> When I try to change a program, I find it annoying to
> *not* see an ELSE branch that does then become a missing ELSE
> branch. The desired change turns a one-way flow into a blind alley...

Compatibility again would cause problems for a general rule; a restriction
could be stronger.

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:

if I_Know_What_This_Does (P) then
X := Assign_It;
-- else can't happen as P must be valid - previously checked.
end if;

Although I generally prefer:

if I_Know_What_This_Does (P) then
X := Assign_It;
else
raise Program_Error; -- Can't happen as P must be valid - previously
checked.
end if;

Or in our compiler:

if I_Know_What_This_Does (P) then
X := Assign_It;
else
J2Trace.Internal_Error ("P must be valid"); -- Often including P in
the message.
end if;

....
> The incompleteness of information here can be avoided,
> to some extent I think, and that's the reason I find
> the Java rule to be helpful, or the Ada warning, or the
> pragma which you have suggested.
> Not really annoying. I can no longer like seeing only half
> of the cases of Boolean covered in an IF, explicitly.

Ada has generally left this to coding standards. I'm sure Jean-Pierre will
be happy to tell us that Ada-Control has options for enforcing the existence
of ELSE branches.

One could imagine providing such possibilities within the language, but it
is fairly rare that we've done that. That's partially because Restrictions
(almost always) apply to the entire partition, including the runtime
libraries. But independently developed subsystems (like the runtime system
or Claw or GTKAda or AWS) are likely to have used different coding
standards. And rewriting them to *your* coding standard is busy work at
best, and a great way to introduce bugs at worst.

Randy.


From: Dmitry A. Kazakov on
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;

or

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

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

All these cases represent some lengthy action, maybe iterated or recursive.
The action is left upon some condition.

If not senseless "else null;", then semantically, under "else" there must
be the rest of the action (recursive, infinite).

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;

--
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de
From: Randy Brukardt on
"Dmitry A. Kazakov" <mailbox(a)dmitry-kazakov.de> wrote in message
news:qchmzfxola9n$.1owojhwd91cbc.dlg(a)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.


From: Dmitry A. Kazakov on
On Fri, 4 Dec 2009 20:45:19 -0600, Randy Brukardt wrote:

> 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);

Why not to allow such constraints for subtypes? E.g.

subtype Valid_Window_Type is Window_Type when Is_Valid;

then simply:

procedure Do_Something (Window : Valid_Window_Type; ...)

> 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.)

Yes, but I do prefer the subtypes as the vehicle, rather than arbitrary
preconditions put here and there on the subprograms. The latter is kind of
weak typing to me.

--
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de
From: Randy Brukardt on
"Dmitry A. Kazakov" <mailbox(a)dmitry-kazakov.de> wrote in message
news:1gcigitaii0u0.1psu2vj52e66g$.dlg(a)40tude.net...
> On Fri, 4 Dec 2009 20:45:19 -0600, Randy Brukardt wrote:
>
>> 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);
>
> Why not to allow such constraints for subtypes? E.g.
>
> subtype Valid_Window_Type is Window_Type when Is_Valid;
>
> then simply:
>
> procedure Do_Something (Window : Valid_Window_Type; ...)

That's also under consideration, but there are some problems with it. One
problem is that such types are very confusing in array indexes/slices (do
the values all participate or just the ones that pass the predicate?) There
are a couple of others as well.

Another issue is that not all preconditions/postconditions can be written
this way. For one thing, a precondition can depend on multiple parameters at
once. Another issues is that the entry condition and exit conditions may be
different for a parameter. For instance, from Claw:

procedure Create (Window : Basic_Window_Type; ...)
with Pre => not Is_Valid (Window),
Post => Is_Valid (Window);

So subtypes cannot completely replace pre and post conditions, but they can
be a complement to them.

Randy.