From mboxrd@z Thu Jan  1 00:00:00 1970
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on polar.synack.me
X-Spam-Level: 
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham
	autolearn_force=no version=3.4.4
X-Google-Thread: 103376,2a687662f09731bb
X-Google-Attributes: gid103376,public
X-Google-Language: ENGLISH,ASCII-7-bit
Path: 
 g2news1.google.com!news1.google.com!proxad.net!infeed-1.proxad.net!news8-e.free.fr!not-for-mail
Sender: sam@willow.rfc1149.net
From: Samuel Tardieu <sam@rfc1149.net>
Newsgroups: comp.lang.ada
Subject: Re: Request for comments on simple Ada program
References: <dld0t5$df3$1@sunnews.cern.ch>
Date: 15 Nov 2005 18:43:39 +0100
Message-ID: <87psp1lsc4.fsf@willow.rfc1149.net>
User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.4
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Leafnode-NNTP-Posting-Host: 2001:660:330f:f810:211:2fff:fea6:3aa1
Organization: Guest of ProXad - France
NNTP-Posting-Date: 15 Nov 2005 18:43:48 MET
NNTP-Posting-Host: 81.56.47.149
X-Trace: 1132076628 news8-e.free.fr 669 81.56.47.149:56910
X-Complaints-To: abuse@proxad.net
Xref: g2news1.google.com comp.lang.ada:6397
Date: 2005-11-15T18:43:48+01:00
List-Id: <comp.lang.ada>

>>>>> "Maciej" == Maciej Sobczak <no.spam@no.spam.com> writes:

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

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

begin
   Check_Primes (Primes);
   for N in Primes'Range loop
      if Is_Prime (N) /= Primes (N) then
         Put_Line ("Error -- result for" & Potential_Prime'Image (N) &
                   " doesn't match");
         return;
      end if;
   end loop;
   Put_Line ("OK");
end Test_Primes;