Strange behavior of move with strings

I am testing some enhanced string related functions with which I am trying to use move as a way to copy strings around for faster, more efficient use without delving into pointers.

While testing a function for making a delimited string from a TStringList, I encountered a strange issue. The compiler referenced the bytes contained through the index when it was empty and when a string was added to it through move, index referenced the characters contained.

Here is a small downsized barebone code sample:-

unit UI;

interface

uses
  System.SysUtils, System.Types, System.UITypes, System.Rtti, System.Classes,
  System.Variants, FMX.Types, FMX.Controls, FMX.Forms, FMX.Dialogs, FMX.Layouts,
  FMX.Memo;

type
  TForm1 = class(TForm)
    Results: TMemo;
    procedure FormCreate(Sender: TObject);
  end;

var
  Form1: TForm1;

implementation

{$R *.fmx}

function  StringListToDelimitedString
          ( const AStringList: TStringList; const ADelimiter: String ): String;
var
  Str           : String;
  Temp1         : NativeInt;
  Temp2         : NativeInt;
  DelimiterSize : Byte;

begin

  Result        := ' ';
  Temp1         := 0;
  DelimiterSize := Length ( ADelimiter ) * 2;

  for Str in AStringList do
    Temp1 := Temp1 + Length ( Str );

  SetLength ( Result, Temp1 );
  Temp1     := 1;

  for Str in AStringList do
  begin

    Temp2 := Length ( Str ) * 2;

    // Here Index references bytes in Result
    Move  ( Str [1],        Result [Temp1], Temp2 );

    // From here the index seems to address characters instead of bytes in Result
    Temp1 := Temp1 + Temp2;
    Move  ( ADelimiter [1], Result [Temp1], DelimiterSize );    
    Temp1 := Temp1 + DelimiterSize;

  end;

end;

procedure TForm1.FormCreate(Sender: TObject);
var
  StrList : TStringList;
  Str     : String;

begin

  // Test 1 : StringListToDelimitedString

  StrList := TStringList.Create;
  Str     := '';

  StrList.Add ( 'Hello1' );
  StrList.Add ( 'Hello2' );
  StrList.Add ( 'Hello3' );
  StrList.Add ( 'Hello4' );

  Str := StringListToDelimitedString ( StrList, ';' );
  Results.Lines.Add ( Str ); 
  StrList.Free;

end;

end.

Please devise a solution and if possible, some explanation. Alternatives are welcome too.

Solution

Let's look at the crucial bit of code:

// Here Index references bytes in Result
Move  ( Str [1],        Result [Temp1], Temp2 );

// From here the index seems to address characters instead of bytes in Result
Temp1 := Temp1 + Temp2;
Move  ( ADelimiter [1], Result [Temp1], DelimiterSize );

Now, some explanations. When you index a string, you are always indexing characters. You are never indexing bytes. It looks to me as though you wish to index bytes. In which case using the string index operator makes life hard. So I suggest that you index bytes as follows.

First of all initialise Temp1 to 0 rather than 1 since we will be using zero-based indexing.

When you need to index Result using a zero-based byte index, do so like this:

PByte(Result)[Temp1]

So your code becomes:

Temp1 := 0;
for Str in AStringList do
begin
  Temp2 := Length(Str)*2;
  Move(Str[1], PByte(Result)[Temp1], Temp2);
  Temp1 := Temp1 + Temp2;
  Move(ADelimiter[1], PByte(Result)[Temp1], DelimiterSize);    
  Temp1 := Temp1 + DelimiterSize;
end;

In fact I think I'd write it like this, avoiding all string indexing:

Temp1 := 0;
for Str in AStringList do
begin
  Temp2 := Length(Str)*2;
  Move(Pointer(Str)^, PByte(Result)[Temp1], Temp2);
  Temp1 := Temp1 + Temp2;
  Move(Pointer(ADelimiter)^, PByte(Result)[Temp1], DelimiterSize);    
  Temp1 := Temp1 + DelimiterSize;
end;

I'd suggest better names than Temp1 and Temp2. I also question the use of NativeInt here. I'd normally expect to see Integer. Not least because a Delphi string is indexed by signed 32 bit values. You cannot have a string with length greater than 2GB.

Note also that you are not allocating enough memory. You forgot to account for the length of the delimiter. Fix that and your function looks like this:

function StringListToDelimitedString(const AStringList: TStringList;
  const ADelimiter: String): String;
var
  Str: String;
  Temp1: Integer;
  Temp2: Integer;
  DelimiterSize: Integer;
begin
  Temp1 := 0;
  DelimiterSize := Length(ADelimiter) * SizeOf(Char);

  for Str in AStringList do
    inc(Temp1, Length(Str) + DelimiterSize);

  SetLength(Result, Temp1);
  Temp1 := 0;
  for Str in AStringList do
  begin
    Temp2 := Length(Str) * SizeOf(Char);
    Move(Pointer(Str)^, PByte(Result)[Temp1], Temp2);
    inc(Temp1, Temp2);
    Move(Pointer(ADelimiter)^, PByte(Result)[Temp1], DelimiterSize);
    inc(Temp1, DelimiterSize);
  end;
end;

If you want to avoid pointers, then write it like this:

function StringListToDelimitedString(const AStringList: TStringList;
  const ADelimiter: String): String;
var
  Str: String;
  StrLen: Integer;
  ResultLen: Integer;
  DelimiterLen: Integer;
  ResultIndex: Integer;
begin
  DelimiterLen := Length(ADelimiter);

  ResultLen := 0;
  for Str in AStringList do
    inc(ResultLen, Length(Str) + DelimiterLen);

  SetLength(Result, ResultLen);

  ResultIndex := 1;
  for Str in AStringList do
  begin
    StrLen := Length(Str);
    Move(Pointer(Str)^, Result[ResultIndex], StrLen*SizeOf(Char));
    inc(ResultIndex, StrLen);
    Move(Pointer(ADelimiter)^, Result[ResultIndex], DelimiterLen*SizeOf(Char));
    inc(ResultIndex, DelimiterLen);
  end;
end;