Re: String manupulation

["David C. Hoos" <david.c.hoos.sr@ada95.com>]

Some additional style comments:

1. One might wish to extend the "Space" object
    to be "Whitespace" with the appropriate set of
    characters, in case one is reading a file with
    tabs or other "whitespace" characters.

2.  The use of a construct like FS'Length would
     better be replaced with FS'Last to cover cases like
     the string being parsed is an input parameter to the
     subprogram, and the caller calls with a slice not
     starting with index 1.

----- Original Message -----
From: "Ted Dennison" <dennison@telepath.com>
Newsgroups: comp.lang.ada
To: <comp.lang.ada@ada.eu.org>
Sent: Wednesday, August 22, 2001 10:53 AM
Subject: Re: String manupulation


> In article <9lvq0p$cpm$1@snipp.uninett.no>, Reinert Korsnes says...
> >
> >Thanks to all of you.  Is this the (dirty ?) way to do it (?) :
>
> Some style comments:
>
> o  Anything that never changes after the declaration should be declared
> "constant".
>
> o  There's already an instantiation of Integer_IO for Integer. Its
> Ada.Integer_Text_IO.
>
> o Unless it won't work for some reason, I generally prefer to use 'Image
rather
> than a numeric Text_IO instantiation.
>
> o You don't need to "with" a package if you already "with" one of its
children.
>
> o Except in extreme quick-n-dirty code, I always specify the parameter
names in
> multi-parameter subprogram calls.
>
> o (Contraversial) I prefer to aviod the "use" clause.
>
> o There's no indication where the "1" you initialize those variables to
comes
> from or why its there. Instead, only initialize the one(s) you need to,
and do
> it from the array it is using to index through.
>
> Also, while transforming that appropriately, I noticed that you have a bug
where
> it doesn't handle a final substring of length 1 properly. That's the kind
of
> thing you are more apt to notice the more closely you tie your indices to
your
> array.
>
> Also, Find_Token returns 0 for Last when no more spaces are found. If you
supply
> a string that doesn't end in a space, your code will loop endlessly. The
> *proper* way to terminate your "Find_Token" based loop is to terminate
when no
> further token is found (Last = 0).
>
> Given all that, I'd transform it to:
> ------------------------
> with Ada.Text_IO;
> with Ada.Strings.Fixed;
> with Ada.Strings.Maps;
>
> procedure Rtest1 is
> FS    : constant String := " This is a test to split a string ";
> C1    : constant String := "123456789012345678901234";
> Space : constant Ada.Strings.Maps.Character_Set :=
Ada.Strings.Maps.To_Set(" ");
>
> First : Natural := Fs'First;
> Last  : Natural;
> begin
> Ada.Text_IO.Put_Line (C1);
> Ada.Text_IO.Put_Line (FS);
>
> loop
> Ada.Strings.Fixed.Find_Token
> (Source => FS(First..FS'Length),
> Set    => Space,
> Test   => Ada.Strings.Outside,
> First  => First,
> Last   => Last
> );
>
> exit when Last = 0;
>
> Ada.Text_IO.Put_Line
> (Integer'Image(First) & Integer'Image(Last) & " " & Fs (First..Last));
>
> First := Last + 1;
> end loop;
> end Rtest1;
> -------------
>
> To satify those who don't agree with the "no use" bit, I'll also include a
> version with "use"s. Obviously *I* don't think it looks better, but some
here
> surely will:
>
> -------------
> with Ada.Text_IO;
> with Ada.Strings.Fixed;
> with Ada.Strings.Maps;
>
> use Ada.Text_IO;
> use Ada.Strings.Fixed;
> use Ada.Strings.Maps;
>
> procedure Rtest1 is
> FS    : constant String := " This is a test to split a string ";
> C1    : constant String := "123456789012345678901234";
> Space : constant Character_Set := To_Set(" ");
>
> First : Natural := Fs'First;
> Last  : Natural;
> begin
> Put_Line (C1);
> Put_Line (FS);
>
> loop
> Find_Token
> (Source => FS(First..FS'Length),
> Set    => Space,
> Test   => Ada.Strings.Outside,
> First  => First,
> Last   => Last
> );
>
> exit when Last = 0;
>
> Put_Line
> (Integer'Image(First) & Integer'Image(Last) & " " & Fs (First..Last));
>
> First := Last + 1;
> end loop;
> end Rtest1;
> -------------
>
> ---
> T.E.D.    homepage   - http://www.telepath.com/dennison/Ted/TED.html
>           home email - mailto:dennison@telepath.com
> _______________________________________________
> comp.lang.ada mailing list
> comp.lang.ada@ada.eu.org
> http://ada.eu.org/mailman/listinfo/comp.lang.ada
>