While playing with Ada and exercising its constructs, I have written the
following classic Sieve of Eratosthenes:

with Ada.Text_IO;
with Ada.Integer_Text_IO;
use Ada.Text_IO;
use Ada.Integer_Text_IO;

procedure Primes is

    -- classic sieve
    procedure Print_Primes(Upper : in Positive) is
       Prime_Numbers : array(2..Upper) of Boolean := (others => True);
    begin
       for N in Prime_Numbers'Range loop
          if Prime_Numbers(N) then
             Put(N);
             New_Line;

             declare
                I : Positive := 2 * N;
             begin
                while I <= Prime_Numbers'Last loop
                   Prime_Numbers(I) := False;
                   I := I + N;
                end loop;
             end;
          end if;
       end loop;
    end Print_Primes;

    Upper_Limit : Positive;

begin
    Put("Enter the upper limit: ");
    Get(Upper_Limit);

    Print_Primes(Upper_Limit);
end Primes;

Now my question is: What would do do to make this code "better"?

Would you add comments? Loop labels? Change names? Is one-character name
for the loop control variable considered OK?
Would you put the "sieving" part in a separate function? Maybe even more
nested? Or maybe would you remove the separate function and put
everything in the main function altogether?
Would you define a separate type for the array or is the anonymous type
considered harmless?
Would you add more vertical whitespace? Or maybe it's already too much
of them?
Would you use different scopes for variable declarations?

The question is not whether this program is correct, but rather what
kind of pedantic touches would you apply to make it "better" (for your
definition of "better", like more readable, etc.)?

I'm interested in style and what do you consider to be a *good* Ada
code. Please do not hesitate to be as picky as you want - that's exactly
the point of this post.

Note: This is not a homework, I'm a self-learner.

Ok, you want some nitpicking, here is some :)

After my signature, you can find the way I would have coded it. I
would have separated the prime numbers *computation* from the prime
number *display*. The computation may be reused in many contexts. What
if you now need to compute the primes in another part of your program
and store them in a file? It doesn't seem logical to have to modify
any part involved in prime numbers computation, especially if you want
to be able to test it in isolation.

My example is much more verbose than yours. However, it is also much
more reusable. I've also included an example of unit testing for the
Check_Primes procedure.

Of course, all that is a question of taste.

  Sam

with Ada.Text_IO;         use Ada.Text_IO;
with Ada.Integer_Text_IO; use Ada.Integer_Text_IO;
with Primes;              use Primes;

procedure Primes_Printer is

   procedure Print_Primes (Primes : Candidates_Array);
   --  Print all indices for which the value is True

   procedure Print_Primes_Up_To (Upper_Bound : Potential_Prime);
   --  Print all primes between 2 and Potential_Prime

   ------------------
   -- Print_Primes --
   ------------------

   procedure Print_Primes (Primes : Candidates_Array) is
   begin
      for N in Primes'Range loop
         if Primes (N) then
            Put (N); New_Line;
         end if;
      end loop;
   end Print_Primes;

   ------------------------
   -- Print_Primes_Up_To --
   ------------------------

   procedure Print_Primes_Up_To (Upper_Bound : Potential_Prime) is
      Primes : Candidates_Array (2 .. Upper_Bound) := (others => True);
   begin
      Check_Primes (Primes);
      Print_Primes (Primes);
   end Print_Primes_Up_To;

   Upper_Limit : Potential_Prime;

begin
   Put ("Enter the upper limit: ");
   Get (Upper_Limit);

   Print_Primes_Up_To (Upper_Limit);
end Primes_Printer;

package Primes is

   subtype Potential_Prime is Positive range 2 .. Positive'Last;

   type Candidates_Array is array (Potential_Prime range <>) of Boolean;
   pragma Pack (Candidates_Array);

   procedure Check_Primes (Candidates : in out Candidates_Array);
   --  Mark non prime numbers in Candidates by setting their entry
   --  to False. Entries must be previously set to True. The only
   --  exception is when Candidates'First is greater than 2: in this
   --  case, multiples of numbers between 2 and Candidates'First+1
   --  must contain False and will not be checked again.

end Primes;

package body Primes is

   ------------------
   -- Check_Primes --
   ------------------

   procedure Check_Primes (Candidates : in out Candidates_Array) is

      procedure Mark_Multiples (Multiplier : in Potential_Prime);
      --  Mark all multiples of Multiplier (not including itself) as
      --  being non primes.

      --------------------
      -- Mark_Multiples --
      --------------------

      procedure Mark_Multiples (Multiplier : in Potential_Prime)
      is
         M : Potential_Prime := 2 * Multiplier;
      begin
         while M <= Candidates'Last loop
            Candidates (M) := False;
            M := M + Multiplier;
         end loop;
      end Mark_Multiples;

   begin
      for N in 2 .. Candidates'Last loop
         if Candidates (N) then
            Mark_Multiples (N);
         end if;
      end loop;
   end Check_Primes;

end Primes;

with Ada.Text_IO; use Ada.Text_IO;
with Primes;      use Primes;

--  Test correctness of the Check_Primes function for primes up to 10_000

procedure Test_Primes is

   function Is_Prime (N : Potential_Prime) return Boolean;
   --  Check whether N can be divided by any number between 1 and N
   --  (exclusive).

   -------------
   -- Is_Prime --
   --------------

   function Is_Prime (N : Potential_Prime) return Boolean is
   begin
      for I in 2 .. N-1 loop
         if N rem I = 0 then
            return False;
         end if;
      end loop;
      return True;
   end Is_Prime;

   Primes : Candidates_Array (2 .. 10_000) := (others => True);

Moreover (I forgot this point), you can now change the Check_Primes
procedure (in the Primes package body) without affecting the main
program, for example to use something else than Erathostene's
sieves. And the unit test will still be usable (and then becomes also
a regression test, which checks that you haven't broken anything).

Writing the function of the procedure in a comment is OK, although I
would like to keep all the non-checkable stuff outside the source
file.

It depends. Sometimes (particularily when doing embedded stuff), you
do have to comment the tricks you resort to. When I do some low-level
Ada or Forth programming for microcontrollers, some of the words (that
I need to optimize for performance) really need comments. If not for
me (the author of the code), at least for others.

What I do in general is reread every code I write one week later. Each
time I ask myself "why did I do this?", even for one second, I add a
comment. My experience shows that even when I go through the code one
year later, I have no question that are not already answered in the
comments.

Once upon a time (early eighties) when I was a Fortran programmer, my
colleague and I used to start coding by writing a pseudo code. The
syntax for the pseudo code was Ada. Much of the Ada code was left in the
Fortran code as comments.

Yeah, typo while coming up with an educative example :)

Jacob> I use similar comments to separate/introduce procedures, but
Jacob> what do we add by repeating the name of the procedure in the
Jacob> comment?

Visual effect. If you have an editor which emphasizes subprogram names
a lot, they might not be needed. However, putting them let anyone
browsing the code (even in plain text only) see the subprogram names
immediately.

I put "comments" in quotes above, because I think that this is a
bad case of mis-naming in programming language terminology. The
word "comment" implies something skimpy, an addition, a note; in
my view, what is needed is a rationale, description or motivation
that is mainly written *before* the code itself.

As for keeping such text in separate design documents, I would do
it if a customer demanded it, but I think it would be much harder
to manage changes, versions and configurations accurately. Still,
some very high-level descriptions are nice to keep apart from the
source-code, if they are not outdated by day-to-day code changes.

My reason for mentioning what I believe is the optimal comment/code
ratio, is actually due to an article in ACM Queue, where the authors
used a code quality metric, which equalled more comments with higher
code quality.

An ideal language should be expressive and readable enough to
stand on its own. Unfortunately, Ada -- or, to be fair, the way I
use Ada -- is still too low-level for that. Perhaps some future
aspect-oriented language...

The conversation seems to be drifting towards Knuth's "literate
programming". I think it is or was a good idea, but gave more
benefit for Pascal than it would give for Ada, which can be more
literate in itself. Perhaps the time is ripe for something like
"checkable literate programming", modelled on SPARK, where the
annotations/comments are readable and informative but also checkable?