Re: if statements

[Robert Dewar <robert_dewar@my-deja.com>]

In article <3A09A39F.2822C01B@cepsz.unizar.es>,
  Alejandro Villanueva <190921@cepsz.unizar.es> wrote:
> Robert Dewar wrote:
>
> > In article <3A078B6F.D34B024B@erols.com>,
> >   Daniel Allex <dallex@erols.com> wrote:
> > > procedure Swap ( A,     B : in out Integer ) is
> > >       Temp : Integer := 0;
> > >    begin
> > >       if A <= B then
> > >          Temp := A;
> > >          A := B;
> > >          B := Temp;
> > >       end if;
> > >    end;
> >
> > Finally he gets someone to do his homework for him, though
> > not very competently (this looks like another student
> > tackling the excercise for the first time :-)
> >
>
> Robert, you do always the same... why do you have to say that?
> It IS a valid solution, and not the solution a geek will give
> after all. And MOST important: it WORKS!

If there is one thing in the Ada world that one expects it is
to get away from th idea that the MOST important thing about
code is that "it WORKS". Ada is about building reliable,
maintainable software, and the fact that it works is necessary
but no where *near* sufficient.

Let's look at the example above and see what's wrong with it

> > > procedure Swap ( A,     B : in out Integer ) is
> > >       Temp : Integer := 0;
> > >    begin
> > >       if A <= B then
> > >          Temp := A;
> > >          A := B;
> > >          B := Temp;
> > >       end if;
> > >    end;

First, the big chunk of white space between A, and B seems
quite gratuitous, I cannot think of any good excuse for it.

Second, the initialization of Temp is confusing and unnecessary.

Third, the indentation is non-standard. The RM suggests a
standard indentation, it should be used almost always.

Fourth, it is good practice to always put end labels for
procedures.

Fifth, Temp is a variable, when it should be a constant. People
very much overuse variables.

Sixth, Temp is defined too globally, you want to be easily
able to see that Temp is just a local variable for the Swap
and that its value is dead on exit.

Seventh, the name Swap is a very poor choice for the procedure,
since this is not what it does.

Eighth, why on earth swap the elements if they are equal, that
seems silly!

I would write the above as:

   procedure Conditional_Swap (A, B : in out Integer) is
   begin
      if A < B then
         declare
            Temp : constant Integer := A;
         begin
            A := B;
            B := Temp;
         end;
      end if;
   end Conditional_Swap;

I actually would prefer as a matter of style to introduce
a separate procedure called Swap, so that the guts of the
procedure would look like

    if A < B then
       Swap (A, B);
    end if;

Well all this is of course overkill for such a trivial
statement (one must wonder why anyone who finds it less
trouble to write a message to CLA than to solve such a
trivial problem themselves), but in fact the lesson to
be learned even at a very trivial level is precisely the
sort of attitudes to correct style that can then hopefully
scale up to larger examples.

So there you are, Robert's complete solution to the problem
first posed (after the deadline unfortunately :-)

I don't think that will make the plane crash :-) Though it
may be considered pedantic. As I say, bringing out the full
weight of careful style considerations for such a simple
program is overkill, but you have to start somewhere.


Sent via Deja.com http://www.deja.com/
Before you buy.