Re: S-expression I/O in Ada

[Georg Bauhaus <rm.dash-bauhaus@futureapps.de>]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On 27.08.10 15:19, Natasha Kerensikova wrote:
> Hello,
> 
> Here is my third attempt, hopefully avoiding the mistakes I did before.

Just a few quick comments on style, more or less.

The first is to use -gnatwa every now and then, or a similar
feature of your Ada compiler or analysis tool.  It will tell
you which variables are in effect constants, for example.

Second, if you assign entire aggregates instead of performing
several assignment component by component, this ensures you
will never miss a component, should the definition of the
type change.

Finally, some variables (like First in Iterate_Over_Lists)
may or may not serve some purpose, and can be omitted.
OTOH, I guess one might want to weigh this against having
a meaningful name for a larger construct or consider a
more locally declared constant instead (in a declare block).

diff attached
Georg

[-- Attachment #2: sexp.diff --]
[-- Type: text/plain, Size: 4913 bytes --]

*** sexp.ada	2010-08-27 15:44:20.000000000 +0200
--- ../sexp.ada	2010-08-27 15:36:52.000000000 +0200
***************
*** 450,454 ****
  
     function Root(From : in Container) return Cursor is
!       Result : constant Cursor := (Parent => null, Node => From.Root);
     begin
        return Result;
--- 450,454 ----
  
     function Root(From : in Container) return Cursor is
!       Result : Cursor := (Parent => null, Node => From.Root);
     begin
        return Result;
***************
*** 461,466 ****
     begin
        if Position.Node /= null and then Position.Node.Next /= null then
!          Result := (Parent => Position.Node,
!                     Node   => Position.Node.Next);
        end if;
        return Result;
--- 461,466 ----
     begin
        if Position.Node /= null and then Position.Node.Next /= null then
!          Result.Parent := Position.Node;
!          Result.Node   := Position.Node.Next;
        end if;
        return Result;
***************
*** 473,478 ****
        if Position.Node /= null then
           if Position.Node.Next /= null then
!             Position := (Parent => Position.Node,
!                          Node   => Position.Parent.Next);
           else
              Position := No_Element;
--- 473,478 ----
        if Position.Node /= null then
           if Position.Node.Next /= null then
!             Position.Parent := Position.Node;
!             Position.Node   := Position.Parent.Next;
           else
              Position := No_Element;
***************
*** 570,575 ****
           raise Constraint_Error with "Position cursor is not a list";
        end if;
!       Result := (Parent => Position.Node,
!                  Node   => Position.Node.Child);
        return Result;
     end Sublist;
--- 570,575 ----
           raise Constraint_Error with "Position cursor is not a list";
        end if;
!       Result.Parent := Position.Node;
!       Result.Node   := Position.Node.Child;
        return Result;
     end Sublist;
***************
*** 659,664 ****
                    Process_Atom(Current.Atom.all);
                 when List_Node =>
!                   First := (Parent => Current,
!                             Node   => Current.Child);
                    Process_List(First);
              end case;
--- 659,664 ----
                    Process_Atom(Current.Atom.all);
                 when List_Node =>
!                   First.Parent := Current;
!                   First.Node   := Current.Child;
                    Process_List(First);
              end case;
***************
*** 674,678 ****
        Process : not null access procedure(Data : in Atom_Data))
     is
!       Current : constant Node_Access := Start.Node;
     begin
        while Current /= null loop
--- 674,678 ----
        Process : not null access procedure(Data : in Atom_Data))
     is
!       Current : Node_Access := Start.Node;
     begin
        while Current /= null loop
***************
*** 689,702 ****
        Process : not null access procedure(First : in Cursor))
     is
!       Current : constant Node_Access := Start.Node;
!       --First   : Cursor;
     begin
        while Current /= null loop
           if Current.Kind = List_Node then
!             --First := (Parent => Current,
!             --          Node   => Current.Child);
!             --Process(First);
!             Process(First => Cursor'(Parent => Current,
!                                      Node   => Current.Child));
           end if;
        end loop;
--- 689,700 ----
        Process : not null access procedure(First : in Cursor))
     is
!       Current : Node_Access := Start.Node;
!       First   : Cursor;
     begin
        while Current /= null loop
           if Current.Kind = List_Node then
!             First.Parent := Current;
!             First.Node   := Current.Child;
!             Process(First);
           end if;
        end loop;
***************
*** 710,714 ****
                                            Arguments : in Cursor))
     is
!       Current : constant Node_Access := Start.Node;
        Arg : Cursor;
     begin
--- 708,712 ----
                                            Arguments : in Cursor))
     is
!       Current : Node_Access := Start.Node;
        Arg : Cursor;
     begin
***************
*** 717,722 ****
              Execute(To_String(Current.Atom.all), No_Element);
           elsif Current.Child.Kind = Atom_node then
!             Arg := (Parent => Current.Child,
!                     Node   => Current.Child.Next);
              Execute(To_String(Current.Child.Atom.all), Arg);
           end if;
--- 715,720 ----
              Execute(To_String(Current.Atom.all), No_Element);
           elsif Current.Child.Kind = Atom_node then
!             Arg.Parent := Current.Child;
!             Arg.Node   := Current.Child.Next;
              Execute(To_String(Current.Child.Atom.all), Arg);
           end if;