Search code examples
arraysdelphidelphi-xe7

How to refactor copying/moving data between arrays?


I have a process that reads data into 150+ temp arrays, process the data and copies from temp arrays into work arrays. Work arrays are in one global array so I can import multiple data, means I can repeat the same process up to 100 times and end up with set of on big array that holds 100x work data I can work with, compare and do stuff.

I have 150+ arrays, so 150 times:

    // for each array
    SetLength(myData[Idx].WorkNames,Length(tmpNames)); // <- prepare to copy
    for i := 0 to High(tmpNames) do  // <- copy
      myData[Idx].WorkNames[i]:=tmpNames[i];
    SetLength(tmpNames,0);  //  <- clear tmp array

4 lines of code for each array - 150x4 = 600 loc + initial + empty lines - around 900 loc.

Here is the example of what I do:

type

  TName = record
    NameID:integer;
    Description:string;
  end;

  TItem = record
    ItemID:integer;
    Description:string;
    Active:boolean;
  end;

  TWorkData = record
      WorkDataType:string;
      WorkNames:array of TName;
      WorkItems:array of TItem;
    end;

var

  AllWorkData:array of TWorkData;  //  <- global array that has all work data - up to 100x sets of work data
  tmpNames:array of TName; // <- tmp arrays before saving to work array
  tmpItems:array of TItem; // 

procedure TForm1.Button1Click(Sender: TObject);
var i,Idx:integer;
begin

  // 1. read data into tmp arrays
  ReadDataIntoTmpArrays;
  ProcessTmpData;

  // 2. copy tmp arrays into work data
  Idx:=GetWorkDataIdx; // <- work data sequence number; start with 0
  AllWorkData[Idx].WorkDataType:=GetWorkDataName(Idx);
  SetLength(AllWorkData[Idx].WorkNames,Length(tmpNames));
  SetLength(AllWorkData[Idx].WorkItems,Length(tmpItems));

  for i := 0 to High(tmpNames) do
    AllWorkData[Idx].WorkNames[i]:=tmpNames[i];

  for i := 0 to High(tmpItems) do
    AllWorkData[Idx].WorkItems[i]:=tmpItems[i];

  // 3. clear tmp arrays
  SetLength(tmpNames,0);
  SetLength(tmpItems,0);

end;

Question: Is there something I can do that is easier to maintain, refactor the code?


Solution

  • If you really do want to copy, then do so using generics. You could derive from the TArray class of static class methods declared in System.Generics.Collections. For instance:

    type
      TArray = class(Generics.Collections.TArray)
      public
        class function Copy<T>(const Source: array of T; Index, Count: Integer): TArray<T>; overload; static;
        class function Copy<T>(const Source: array of T): TArray<T>; overload; static;
      end;
    
    ....
    
    class function TArray.Copy<T>(const Source: array of T; Index, Count: Integer): TArray<T>;
    var
      i: Integer;
    begin
      SetLength(Result, Count);
      for i := 0 to high(Result) do begin
        Result[i] := Source[i+Index];
      end;
    end;
    
    class function TArray.Copy<T>(const Source: array of T): TArray<T>;
    var
      i: Integer;
    begin
      SetLength(Result, Length(Source));
      for i := 0 to high(Result) do begin
        Result[i] := Source[i];
      end;
    end;
    

    Note that all of the above requires you to stop using array of TMyType and instead start using the generic dynamic array TArray<TMyType>.

    In your case though you are overcomplicating. Replace:

    SetLength(myData[Idx].WorkNames,Length(tmpNames)); // <- prepare to copy
    for i := 0 to High(tmpNames) do  // <- copy
      myData[Idx].WorkNames[i]:=tmpNames[i];
    SetLength(tmpNames,0);  //  <- clear tmp array
    

    with:

    myData[Idx].WorkNames := tmpNames;
    tmpNames := nil;
    

    If you are prepared to let tmpNames simply leave scope then you can use a single line:

    myData[Idx].WorkNames := tmpNames;
    

    Although, if tmpNames is re-used for a different array, then the nil assignment is needed.

    Then again, as far as I can see from the code in the question you don't need the temporary arrays at all. Why not operate directly on the long lived data structures.

    These array assignments are only permissible if the source and target of the assignment are assignment compatible. Your types are not because you have used distinct types. Switch to TArray<T> to avoid that. See this question for more: Why are two seemingly identical dynamic array types deemed not assignment compatible?

    Remember that dynamic arrays are reference types. In the usage shown here, all you need to do is copy the reference. You only need one instance of the actual array. So copying is not necessary at all.