Produce an array of strings from a single string

Question

I have a function that reads a string like this

(AnimalCount+HumanCount)

and the result of this function is an array of strings

[ "(" , "AnimalCount" , "+" , "HumanCount" , ")" ]

Now, I have done 2 functions in order to do this:

The first one to check if a given char is "special" from the input string

function Validate(Character: Char): Boolean;
begin
  Validate:= Character in [',' , '+' , '-' , '*' , '/' , '(' , ')' , '^' ,
                         '[' , ']' , '%' , '&' , '=' , '<' , '>' , ' ' ,
                         '.' , ';' , ':'];
end;

The second one to separate the strings from the input:

function GetToken(Chain:String):ArrayofString;
Var NTokens,i,j:Integer;
begin
  NTokens:=0;
  SetLength(Result,0);
  For i := 1 to Length(Chain) do
    if Validate(Chain[i]) then
      begin
        Inc(NTokens);
        setlength(Result,NTokens+1);
        Result[NTokens]:=Chain[i];
        Inc(NTokens);
      end
    else
      begin
        If Length(Result)<NTokens+1 then
          begin
            setlength(Result,NTokens+1);
            Result[NTokens]:='';
          end;
        Result[NTokens]:=Result[NTokens]+Chain[i];
      end;    
end;

Additionally, I had to put this after type to make it work this way:

Arrayofstring = array of String;

This functions are working, but I wonder if there is any way to improve it even further. I am new to Delphi, so any help would be appreciated.

Answer 1

Delphi is a nice language in that it is not case sensetive. However, I would still encourage you to stick to one casing. Decide on if you want to use if or If and stick to that. (My personal opinion: Use all lowercase). This also goes for var and for in your code.

---

Your code has a bit of inconsistent spacing. Take a look at this line for example:

Result[NTokens]:=Result[NTokens]+Chain[i];

I would write this as:

Result[NTokens] := Result[NTokens] + Chain[i];

I also think that you can use more spaces in your function header for GetToken (compare it with Validate). And also use a space after each comma (in parameter lists and variable declarations).

---

Your Validate method is quite good. I agree with ChrisWue that the naming could be better.

As for your GetToken method, that can be drastically improved.

Instead of appending char-by-char to the Result[NTokens] string, consider using a var Current: String, add the chars to that string and then when you encounter the next token separator you add the Current to the result.

Regarding your Result, instead of using an array of String, consider the class TStrings or TStringList.

Speaking of TStringList, that actually has support for using delimiters. See this SO question for an example.

If you want to do this yourself (which can be good practice), I would suggest returning a TStringList and calling the add method to add new strings to it. Here's a start for you on returning TStringList, as it's been a while since I used Delphi you will have to fix my mistakes and possibly change some stuff to make it work.

Result := TStringList.Create;
Current := "";
for i := 1 to Length(Chain) do
if IsTokenDelimiter(Chain[i]) then
begin
    if (Length(Current) > 0) then
        Result.Add(Current);
    Result.Add(Chain[i]);
    Current = "";
else
  begin
    Current = Current + Chain[i];
  end;    

If you want to learn more about using built-in systems, take a look around at the TStringList class and see if it's delimiter system can simplify things for you. Perhaps it can't, in which case you might want to google for other Delphi classes that can. It's been a while since I've used Delphi, so I can't recommend a specific something that I know would work for you.

Lastly, I would like you to think about why you need this feature. Where does your string come from? If it's input from the user, then it's probably fine (although you might want to seperate the user inputs). If it is result from a function that you wrote, use a record, an object or a class type. Really, learn how to write your own records and objects and classes! They will simplify many things for you.

Answer 2

1. Validate is a misleading name for what the function does. It does not validate anything - rather it checks if the given character is one of a special set. So it should be named IsSpecial or IsTokenDelimiter.

2. Growing an array one entry at a time is quite a bit of overhead. There are some ways of mitigating this.

3. Consider creating a local variable like currentCharacter in which Chain[i] - this would make it a little bit easier to read.

Answer 3

Here is a function for delimited string to TStringList conversion from my utilities library:

function  DelimitedStringToStringList
          ( const ADelimitedString: String;
            const ADelimiter      : Char ): TStringList;
var
  I               : NativeInt;
  SeparatedString : String;
begin

  SeparatedString := '';
  Result          := TStringList.Create;

  for I := 1 to Length ( ADelimitedString ) do
  begin

    if ADelimitedString [I] = ADelimiter then
    begin

      if SeparatedString <> '' then
      begin
        Result.Add ( SeparatedString );
        SeparatedString := '';
      end;

    end
    else
      SeparatedString := SeparatedString + ADelimitedString [I];

  end;

  if SeparatedString <> '' then
    Result.Add ( SeparatedString );

end;

You may derive your own solution from this.