9

(I already asked this at CodeReview where it got closed as off-topic. Hopefully it's on-topic here.)

I have a static arrays of a derived type (like LabelsA: array[0..3] of TLabel; in the following sample code) and a routine accepting an open array of the base type (like procedure DoSomethingWithControls(const AControls: array of TControl);), and I want to call DoSomethingWithControls with those static arrays. Please see my sample:

procedure DoSomethingWithControls(const AControls: array of TControl);
var
  i: Integer;
begin
  for i := Low(AControls) to High(AControls) do
    Writeln(AControls[i].Name);
end;

procedure Test;
var
  LabelsA: array[0..3] of TLabel;
  LabelsB: array[0..1] of TLabel;

  procedure Variant1;
  type
    TArray1 = array[Low(LabelsA)..High(LabelsA)] of TControl;
    TArray2 = array[Low(LabelsB)..High(LabelsB)] of TControl;
  begin
    DoSomethingWithControls(TArray1(LabelsA));
    DoSomethingWithControls(TArray2(LabelsB));
  end;

  procedure Variant2;
  type
    TControlArray = array[0..Pred(MaxInt div SizeOf(TControl))] of TControl;
    PControlArray = ^TControlArray;
  begin
    DoSomethingWithControls(Slice(PControlArray(@LabelsA)^, Length(LabelsA)));
    DoSomethingWithControls(Slice(PControlArray(@LabelsB)^, Length(LabelsB)));
  end;

  procedure Variant3;
  var
    ControlsA: array[Low(LabelsA)..High(LabelsA)] of TControl absolute LabelsA;
    ControlsB: array[Low(LabelsB)..High(LabelsB)] of TControl absolute LabelsB;
  begin
    DoSomethingWithControls(ControlsA);
    DoSomethingWithControls(ControlsB);
  end;

begin
  Variant1;
  Variant2;
  Variant3;
end;

There are some possible variants of calling DoSomethingWithControls:

  • Variant 1 is quite simple but needs an "adapter" types like TArray1 for every size of TLabel array. I would like it to be more flexible.

  • Variant 2 is more flexible and uniform but ugly and error prone.

  • Variant 3 (courtesy of TOndrej) is similar to Variant 1 - it doesn't need an explicit cast, but Variant 1 offers a tiny bit more compiler security if you mess something up (e.g. getting the array bounds wrong while copy-pasting).

Any ideas how i can formulate these calls without these disadvantages (without changing the element types of the arrays)? It should work with D2007 and XE6.

Community
  • 1
  • 1
Uli Gerhardt
  • 13,748
  • 1
  • 45
  • 83
  • why not use TObjectList.ToArray? – whosrdaddy Jul 22 '15 at 08:06
  • @whosrdaddy: Sorry, forgot to mention D2007. :-/ – Uli Gerhardt Jul 22 '15 at 08:08
  • I'm surprised that you can't just pass an array of `TLabel` – David Heffernan Jul 22 '15 at 08:25
  • @David: Yeah, I guess that's that co(ntra?)-variance thingy. :-) – Uli Gerhardt Jul 22 '15 at 08:28
  • @DavidHeffernan Static arrays have no intrinsic length information. `Low` and `High` work by compiler magic for static arrays, if I recall correctly. If you could pass them into a function the compiler would have to keep track of the length. Naturally, it could be turtles all the way down.... – J... Jul 22 '15 at 10:05
  • 1
    @J... The compiler passes the length info when you pass a static array to an open array param – David Heffernan Jul 22 '15 at 10:12
  • @DavidHeffernan Well so it does... I think I'm confusing things the other way around - that you can't pass untyped dynamic arrays as parameters (for the same reason that length and index are managed differently). Agreed it is weird that an array of `TLabel` doesn't work, then. – J... Jul 22 '15 at 10:33
  • @J... I'd understand a var open array being rejected, but a const open array presents no danger. – David Heffernan Jul 22 '15 at 10:44

5 Answers5

3

These casts are all rather ugly. They will all work, but using them makes you feel dirty. It's perfectly reasonable to use a helper function:

type
  TControlArray = array of TControl;

function ControlArrayFromLabelArray(const Items: array of TLabel): TControlArray;
var 
  i: Integer;
begin
  SetLength(Result, Length(Items));
  for i := 0 to high(Items) do
    Result[i] := Items[i];
end;

And then you call your function like this:

DoSomethingWithControls(ControlArrayFromLabelArray(...));

Of course, this would be so much cleaner if you could use generics.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • My OCD reflex is to complain about the (neglibible, I know) memory allocation and copy overhead of the dynamic array which ideally shouldn't be needed. It would be really nice if one could return an open array like `Slice` does (with a little help from the compiler). :-) – Uli Gerhardt Jul 22 '15 at 11:01
  • You could also make an overload for `DoSomethingWithControls` that takes an `array of TLabel` to make it neater. – J... Jul 22 '15 at 11:02
  • 1
    I agree regarding heap allocation but this is GUI code and won't be your bottleneck surely. – David Heffernan Jul 22 '15 at 11:27
  • @UliGerhardt I would keep that OCD in check because [premature optimization](http://c2.com/cgi/wiki?PrematureOptimization) is bad. – DBedrenko Jul 23 '15 at 07:23
  • @NewWorld, I try to. But wouldn't it be nice if there was a beautiful, elegant, zero-overhead solution? ;-) – Uli Gerhardt Jul 23 '15 at 07:59
  • @Uli I think you are going to have to choose between elegant and zero overhead. The options that feel viable here are the helper method to convert to dyn array, and Sertac's zero overhead implementation detail reliant trickery. – David Heffernan Jul 23 '15 at 08:02
1

Not extremely beautiful either but you could trick the compiler like this:

procedure Variant3;
var
  ControlsA: array[Low(LabelsA)..High(LabelsA)] of TControl absolute LabelsA;
begin
  DoSomethingWithControls(ControlsA);
end;
Ondrej Kelle
  • 36,941
  • 2
  • 65
  • 128
  • Thanks, @TOndrej! Unfortunately this suffers from the same drawback as `Variant1` - I'd have to declare an `absolute` variable for every call. – Uli Gerhardt Jul 22 '15 at 08:27
  • I agree it's a drawback although strictly speaking, it's not the same as `Variant1`. You only said you wanted to avoid declaring a new adapter type. ;-) – Ondrej Kelle Jul 22 '15 at 08:29
  • OK, Variant3 has an **additional** drawback: It compiles even if I get the array size wrong. Variant1 wouldn't compile. ;-) – Uli Gerhardt Jul 22 '15 at 08:34
  • If I mistakenly write `TArray2 = array[Low(LabelsA)..High(LabelsA)] of TControl;` in `Variant1` (note the A instead of the correct B) I get "E2089 Invalid typecast". `ControlsB: array[Low(LabelsA)..High(LabelsA)] of TControl absolute LabelsB;` (same error!) happily compiles. – Uli Gerhardt Jul 22 '15 at 08:39
  • I see. Usually, one can defeat oneself. ;-) – Ondrej Kelle Jul 22 '15 at 08:41
  • Yeah, I'm doing dirty casts here anyways, so I already have to be careful. :-) That's one of the motivations of my original question. – Uli Gerhardt Jul 22 '15 at 08:49
  • 1
    IMO, `absolute` should be removed from the language. – Rudy Velthuis Jul 22 '15 at 10:01
  • There's not really a use case for absolute. You can perfectly well cast to achieve the same effect. I can't understand why it is still with us. – David Heffernan Jul 22 '15 at 17:05
1

Declare an overloaded procedure:

procedure DoSomethingWithControls(const AControls: array of TControl); overload;
var
  i: Integer;
begin
  for i := 0 to High(AControls) do
    if Assigned(AControls[i]) then
       Writeln(AControls[i].Name)
    else
      WriteLn('Control item: ',i);
end;

procedure DoSomethingWithControls(const ALabels: array of TLabel); overload;
type
  TControlArray = array[0..Pred(MaxInt div SizeOf(TControl))] of TControl;
  PControlArray = ^TControlArray;
begin
  DoSomethingWithControls(Slice(PControlArray(@ALabels)^, Length(ALabels)));
end;

This is a general solution to your variant2. One declaration for all cases, so less prone to errors.

LU RD
  • 34,438
  • 5
  • 88
  • 296
1

Below example is based on how open array parameters are internally implemented. It won't work with "typed @ operator" however.

  procedure Variant4;
  type
    TCallProc = procedure (AControls: Pointer; HighBound: Integer);
  var
    CallProc: TCallProc;
  begin
    CallProc := @DoSomethingWithControls;

    CallProc(@LabelsA, Length(LabelsA) - 1);
    CallProc(@LabelsB, Length(LabelsB) - 1);
  end;

Passing High(Labels) for HighBound is perhaps better as long as all static arrays are 0 based.

Sertac Akyuz
  • 54,131
  • 4
  • 102
  • 169
0

Since a dynamic array can be passed into method as an open array, and option would be to convert the static array to a dynamic array.

If you don't mind the overhead of copying the array, consider the following:

Write a function to convert an open array of labels into a dynamic TControlArray array.

type
  TControlArray = array of TControl;

{$IFOPT R+} {$DEFINE R_ON} {$R-} {$ENDIF}
function MakeControlArray(const ALabels: array of TLabel): TControlArray;
begin
  SetLength(Result, Length(ALabels));
  Move(ALabels[0], Result[0], Length(ALabels) * SizeOf(TObject));
end;
{$IFDEF R_ON} {$R+} {$UNDEF R_ON} {$ENDIF}

Now Variant4 can be written as:

procedure Variant4;
begin
  DoSomethingWithControls(MakeControlArray(LabelsA));
  DoSomethingWithControls(MakeControlArray(LabelsB));
end;

Test cases:

procedure TAdHocTests.TestLabelsToControls;
const
  LLabelsA: array[0..3] of TLabel = (Pointer(0),Pointer(1),Pointer(2),Pointer(3));
var
  LLoopI: Integer;
  LLabelsB: array[0..9] of TLabel;
  LEmptyArray: TLabelArray;
begin
  for LLoopI := Low(LLabelsB) to High(LLabelsB) do
  begin
    LLabelsB[LLoopI] := Pointer(LLoopI);
  end;

  DoSomethingWithControls(MakeControlArray(LLabelsA), Length(LLabelsA));
  DoSomethingWithControls(MakeControlArray(LLabelsB), Length(LLabelsB));
  DoSomethingWithControls(MakeControlArray([]), 0);
  DoSomethingWithControls(MakeControlArray(LEmptyArray), 0);
end;

procedure TAdHocTests.DoSomethingWithControls(
    const AControls: array of TControl;
    AExpectedLength: Integer);
var
  LLoopI: Integer;
begin
  CheckEquals(AExpectedLength, Length(AControls), 'Length incorrect');
  for LLoopI := Low(AControls) to High(AControls) do
  begin
    CheckEquals(LLoopI, Integer(AControls[LLoopI]));
  end;
end;
Disillusioned
  • 14,635
  • 3
  • 43
  • 77
  • @DavidHeffernan True, passing `[]` or an empty dynamic array can cause a range error. But this is easily resolved by disabling range-checking specifically for that function. Also, in the specified use-case, why (and for that matter how) would you declare a zero length static array? – Disillusioned Jul 22 '15 at 12:50
  • Your function doesn't operate on static arrays. It accepts an open array and returns a dynamic array. Your edit now enables range checking for all code below this function. Not what is needed. – David Heffernan Jul 22 '15 at 13:29
  • @DavidHeffernan I could point out that if my function doesn't operate on static arrays, then neither does yours. (_Note: you should check your function a little more closely.... you'll find it won't even compile._) ..... However, perhaps you should rather look at the DUnit test case I added and elaborate how my function "doesn't operate on static arrays"?. – Disillusioned Jul 22 '15 at 15:25
  • You misunderstand me. Your function accepts more than static arrays. It can certainly be passed zero length arrays. – David Heffernan Jul 22 '15 at 15:51
  • Regarding my answer, you refer to the typo semi colon? – David Heffernan Jul 22 '15 at 15:53
  • Personally I wouldn't mess with range check setting. I'd write a for loop and let the compiler check type safety. As it is you are bypassing type checking. If you have to use a move then do `Move(Pointer(Source)^, Pointer(Dest)^, len*SizeOf(...))` and avoid the range checking that way. – David Heffernan Jul 22 '15 at 15:56
  • @DavidHeffernan Are you surprised if I misunderstood you? The phrase "doesn't operate on static arrays" doesn't leave much room for alternate interpretation. Especially when considered in conjunction with the second sentence; which also has limited interpretation for that matter. Now when you consider that you were responding to a comment in which I had already effectively conceded the function takes more than static arrays, why would I interpret your comment as saying the same thing? – Disillusioned Jul 22 '15 at 16:54
  • @DavidHeffernan As for _messing with range-checking_: I prefer it on; which was the reason for my first, more simplistic edit. However I like your proposed alternative because I agree that it's better not to change compiler settings on the fly if it can be reasonably avoided.... As for _bypassing type checking_: The function will enforce ot on clients. And we know the function works with arrays of objects, so type-checking that seems moot. Is there a genuine risk you have in mind? – Disillusioned Jul 22 '15 at 17:00
  • I think the rest of the comment was pretty clear. We all understand how open arrays work and how they accept any kind of array. I was only trying to point out a flaw in your code. I was trying to be helpful. – David Heffernan Jul 22 '15 at 17:00
  • 1
    The type checking is a little academic in this case. We all know that TLabel derives from TControl. But it's usually preferable to have the compiler verify these things. I don't see any harm in using a loop. Performance won't be an issue. The loop will work under ARC unlike Move. What's the benefit of using Move? – David Heffernan Jul 22 '15 at 17:04
  • @DavidHeffernan Yes the last 2 sentences of the comment were clear, but unrelated to what kinds of arrays the function accepts. There's no point in justifying flawed comments with other comments that haven't been questioned. – Disillusioned Jul 22 '15 at 17:06
  • I think my meaning was clear. It doesn't operate on static arrays. It operates on open arrays which accept, amongst other things, static arrays and zero length dynamic arrays. If I had said that your function does not **accept** static arrays that would be different. Anyway, you understand me know. This is perfectly normal communication. – David Heffernan Jul 22 '15 at 17:08
  • @DavidHeffernan I'm not concerned about performance, I would hope that the compiler could optimise the loop to be equally efficient. I used `Move` because I find it slightly more concise for copying blocks of data. However pointing out that it won't work under ARC is a valid concern. – Disillusioned Jul 22 '15 at 17:14