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,d83fe9988355b1f3,start
X-Google-Attributes: gid103376,public
X-Google-Language: ENGLISH,ASCII-7-bit
Path: 
 g2news1.google.com!news2.google.com!news4.google.com!border1.nntp.dca.giganews.com!nntp.giganews.com!newsfeed00.sul.t-online.de!t-online.de!130.59.10.21.MISMATCH!kanaga.switch.ch!news-zh.switch.ch!switch.ch!cernne03.cern.ch!cern.ch!news
From: Maciej Sobczak <no.spam@no.spam.com>
Newsgroups: comp.lang.ada
Subject: Request for critics
Date: Wed, 22 Mar 2006 11:01:41 +0100
Organization: CERN - European Laboratory for Particle Physics
Message-ID: <dvr7a5$9o3$1@sunnews.cern.ch>
NNTP-Posting-Host: abpc10883.cern.ch
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
X-Trace: sunnews.cern.ch 1143021701 9987 (None) 137.138.37.241
X-Complaints-To: news@sunnews.cern.ch
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.7.12) Gecko/20060203 Red Hat/1.7.12-1.1.3.4
X-Accept-Language: en-us, en
Xref: g2news1.google.com comp.lang.ada:3550
Date: 2006-03-22T11:01:41+01:00
List-Id: <comp.lang.ada>

For the purpose of my learning, please criticise the following code:

with Ada.Text_IO;
use Ada.Text_IO;

procedure Automata is

    Size : constant := 66;
    Max_Steps : constant := 31;

    type Cell_State is (Active, Inactive);
    type Row_Of_Cells is array (1 .. Size) of Cell_State;

    type Cell_Triplet is array (-1 .. 1) of Cell_State;

    Automata : Row_Of_Cells := (Size / 2 => Active, others => Inactive);

    procedure Show(Row : in Row_Of_Cells) is
    begin
       for I in Row_Of_Cells'Range loop
          case Row(I) is
             when Active   => Put('*');
             when Inactive => Put(' ');
          end case;
       end loop;
       New_Line;
    end Show;

    function New_State(Neighbourhood : in Cell_Triplet)
                       return Cell_State is
       Left  : Cell_State := Neighbourhood(-1);
       Right : Cell_State := Neighbourhood( 1);
    begin
       if (Left = Active and Right = Inactive)
         or (Left = Inactive and Right = Active) then
          return Active;
       else
          return Inactive;
       end if;
    end New_State;

begin
    Show(Automata);
    for Step in 1 .. Max_Steps loop
       declare
          New_Automata : Row_Of_Cells := (others => Inactive);
       begin
          for Cell_Index in
            Row_Of_Cells'First + 1 .. Row_Of_Cells'Last - 1 loop

             New_Automata(Cell_Index) := New_State(Cell_Triplet(
                Automata(Cell_Index - 1 .. Cell_Index + 1)));

          end loop;
          Automata := New_Automata;
       end;
       Show(Automata);
    end loop;
end Automata;

<problem-description>
The above is a short simulation of one particular 1D cellular automata 
(the state of the whole automata is printed as a single line and the 
time progresses downwards on the printout), where the next state of each 
cell is a function of the current states in the immediate neighbourhood 
of the given cell. In other words, given ABCDEFGH as a state of the 
automata, the next state of cell D is a function of cells C, D and E. 
Taking two possible states, there are 256 possible rules. The above 
program simulates only one of them.
</problem-description>

The goal is the readability and proper expression of the problem.

How do you name loop control variables for array traversals? Is 
"Cell_Index" overly verbose and would be "I" just enough?

Would you drop the Cell_Triplet type and pass the neighbourhood as three 
separate parameters instead?

And so on. Pleasy be picky with every aspect you find important.

Any performance considerations for this code (except, of course, that 
Put() is called for each cell and might be replaced by whole-line 
printouts instaed)?


BTW: The C version is just a bit shorter:

#include <stdio.h>
int main(){char s[]=
"                                *                                 "
;char p,n;int st,i;puts(s);for(st=0;st!=31;++st){p=' 
';for(i=1;i!=65;++i){n=(s[i-1]^s[i+1])|0x20;s[i-1]=p;p=n;}s[i-1]=p;puts(s);}return 
0;}

;-)  (the last three characters are not part of the program)

-- 
Maciej Sobczak : http://www.msobczak.com/
Programming    : http://www.msobczak.com/prog/