3

The two sections commented in the first code block are seemingly identical yet yield different result (due to problems elsewhere). I do not understand how they are different. The only thing I change is to comment out the first part (the for loop) or the second part (the assignment lines) and get different results.

var
  Amount: Integer;
  I: Integer;

begin
Amount := 3;

// This produces undesired results (**the for loop**)
for I := 0 to Amount-1 do
  CharDataBool[I] := CharToArray(CharDataText[I]);

// This works as expected (**the assignment lines**)
CharDataBool[0] := CharToArray(CharDataText[0]);
CharDataBool[1] := CharToArray(CharDataText[1]);
CharDataBool[2] := CharToArray(CharDataText[2]);

The code below has questionable practice, and in a way is the source of the problem, but my question is about the code above. The problem above only manifests if the CharToArray function leaves false unassigned as can be seen here:

function CharToArray(Source: TCharDetails): TCharArray;
var
  X, Y: Integer;
begin
  SetLength(Result, Source.Width, 10);

  for Y := 0 to 9 do
    for X := 0 to Source.Width-1 do
      if Source.S[Y*Source.Width+X] = 'x' then
        Result[X,Y] := true
      else Result[X,Y] := false;     // Adding this solves the problem

end;

Without going into "leaving values unknown is real bad" which it is, I just want to understand why the problem manifests with (the for loop) and not with (the assigment lines) in the first section of code? How are the three assignment lines different from the for loop?

Below can be copied over Unit1 in default VCL project

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Vcl.StdCtrls;

type
  TForm1 = class(TForm)
  private
    { Private declarations }
  public
    { Public declarations }
  end;

type
  TCharArray = array of array of boolean;

  TCharDetails = record
    S: String;
    Width: Integer;
  end;

var
  CharDataText: array of TCharDetails;
  CharDataBool: array of TCharArray;

var
  Form1: TForm1;

procedure Init;



implementation

{$R *.dfm}

function CharToArray(Source: TCharDetails): TCharArray;
var
  X, Y: Integer;
begin
  SetLength(Result, Source.Width, 10);

  for Y := 0 to 9 do
    for X := 0 to Source.Width-1 do
      if Source.S[Y*Source.Width+X] = 'x' then
        Result[X,Y] := true;

end;

procedure Init;
var
  Amount: Integer;
  I: Integer;

  X,Y: Integer;
  S1, S2: String;

begin
  Amount := 2;

  SetLength(CharDataText, Amount);
  SetLength(CharDataBool, Amount);

  ChardataText[0].Width := 10;
  ChardataText[0].S :=
//   1234567890
    '          ' +    // 0
    '  x       ' +    // 1
    ' xx       ' +    // 2
    '  x       ' +    // 3
    '  x       ' +    // 4
    '  x       ' +    // 5
    '  x       ' +    // 6
    '  x       ' +    // 7
    '  x       ' +    // 8
    ' xxx      ';

  ChardataText[1].Width := 10;
  ChardataText[1].S :=
//   1234567890
    'x         ' +    // 0
    '   xxxx   ' +    // 1
    '  x    x  ' +    // 2
    '       x  ' +    // 3
    '      x   ' +    // 4
    '     x    ' +    // 5
    '    x     ' +    // 6
    '   x      ' +    // 7
    '  x       ' +    // 8
    '  xxxxxx x';


  for I := 0 to Amount-1 do
    CharDataBool[I] := CharToArray(CharDataText[I]);   
  S1 := '';
  for Y := 0 to 9 do
    for X := 0 to 9 do
      S1 := S1 + CharDataBool[1,X,Y].ToString;

  CharDataBool[0] := CharToArray(CharDataText[0]);
  CharDataBool[1] := CharToArray(CharDataText[1]);    
  S2 := '';
  for Y := 0 to 9 do
    for X := 0 to 9 do
      S2 := S2 + CharDataBool[1,X,Y].ToString;

  // S1 != S2 ??
  ShowMessage(S1 + #13 + S2);      


end;

initialization

  Init;

end.
Doege
  • 341
  • 4
  • 12
  • 1
    You can do: `Result[X, Y] := Source.S[Y * Source + X] = 'x';`. That shows you why that is more or less equivalent to the assignments in the first section of code. – Rudy Velthuis Feb 23 '19 at 23:18
  • When result is undefined, it is undefined. That leads to the result of the function to be undefined. I mean the result may be desired or undesired when it should be false, depending on what the address currently holds, it is not deterministic. You may get a different udefined result just by changing the calling order for instance.. – Sertac Akyuz Feb 23 '19 at 23:23
  • @Sertac: Result is zeroed out by SetLength, so not undefined. All values of Result are false before the loop. – Rudy Velthuis Feb 23 '19 at 23:51
  • Hmmm... hard to tell without trying it out ourselves. Could you turn this into a [MCVE]? In the meantime, you could try to debug the code and see where what goes wrong. You do confuse X and Y in the second part, but that may not be the full reason the loop fails while the simple assignments don't. – Rudy Velthuis Feb 23 '19 at 23:56
  • 1
    I guess that in the loop, a temporary Result variable is re-used. If you assign a reference type to something, and it is re-used, the contents may change. There is no copy-on-write for dynamic arrays, so the last iteration of the loop overwrites the Result of previous iterations, and each element of CharDataBool references the **same** dynarray. That is probably why you must set items to false too: SetLenght also re-uses the array. That is not correct, and I can't debug this (no [MVCE]), but I guess it is something like what I just described. Please also state Delphi version. – Rudy Velthuis Feb 24 '19 at 00:04
  • 1
    Call `SetLength(Result,0,0);` first thing in the function. See [Do I need to setLength a dynamic array on initialization?](https://stackoverflow.com/a/5315254/576719) The dynamic array is reused in the loop and must be cleared inside the function. – LU RD Feb 24 '19 at 00:06
  • @LURD: yes, that should work. – Rudy Velthuis Feb 24 '19 at 00:08
  • `Result[X, Y] := Source.S[Y * Source + X+1] = 'x';` **+1** as string indexes are 1- and not 0-based. – Doege Feb 24 '19 at 00:39
  • @Doege: That means it should be: `Result[X, Y] := Source.S[Y * Source + X + 1] = 'x';`, i.e. the +1 should be inside the square brackets. It was not known this was a string, until you posted some kind of MCVE. – Rudy Velthuis Feb 24 '19 at 00:42

1 Answers1

4

The function return value is a managed type. The Result is not guaranteed to be re-initialized upon entry to the function.

In the loop example, the compiler optimizes away the implicit local Result re-initialization between the iterations. That means that the previous content of the array is still present and must be cleared.

As a general rule, always initialize=clear managed function results upon entry.

In this case, call SetLength(Result,0,0); first thing in the function.


See Do I need to setLength a dynamic array on initialization? for an example.

LU RD
  • 34,438
  • 5
  • 88
  • 296
  • Oh, but it is initialized upon entry, otherwise it would be an invalid array. It is just not zero-initialized. On the second iteration, it still contains the data for the first ASCII image, and SetLength(Result, Source.Width, 10) is keeping the data. This means that in the loop, the array is re-used, but outside the loop, it isn't. – Rudy Velthuis Feb 24 '19 at 00:58
  • FWIW, `Result[X, Y] := Source.S[Y * Source.Width + X + 1] = 'x';` should work too, because that way, the previous content is completely overwritten. – Rudy Velthuis Feb 24 '19 at 01:00
  • 1
    `result := nil` seems shorter to me. – Arnaud Bouchez Feb 24 '19 at 16:46