The FMX enumeration anti-pattern

[Update: a couple of months after I wrote this post, Delphi XE3 was released. In it, the issue I discuss below was mostly, if not entirely eradicated from the FMX source code.]

Checking up on DelphiFeeds.com, I see a member of Embarcadero Developer Relations (Stephen Ball) has just blogged some example code. In it, he demonstrates a class helper that adds methods for enumerating a FireMonkey control, doing so in a way that just picks out nested objects of a certain class. Here’s the gist of it:

function TFMXObjectHelper.ChildrenCountOfType(aType: TClass): Integer;
var
  Idx: Integer;
  Obj: TFmxObject;
begin
  Result := 0;
  for Idx := 0 to Pred(Self.ChildrenCount) do
  begin
    Obj := Self.Children[Idx];
    if Obj is aType then Inc(Result);
    Result := Result + Obj.ChildrenCountOfType(aType);
  end;
end;

function TFMXObjectHelper.ChildrenOfTypeAtIndex(aType: TClass; Index: Integer): TFMXObject;
var
  I, Remaining: Integer;
  Obj: TFmxObject;
  CurrItem: TFmxObject;
  Nodes: Integer;
begin
  Remaining := Index;
  for I := 0 to Pred(Self.ChildrenCount) do
  begin
    CurrItem := Self.Children[I];
    if (CurrItem is aType) then
    begin
      if (Remaining <= 0) then
        Exit(CurrItem)
      else
        Dec(Remaining,1);
    end;
    Nodes := CurrItem.ChildrenCountOfType(aType);
    if (Nodes <= Remaining) then
      Dec(Remaining, Nodes)
    else
    begin
      Obj := CurrItem.ChildrenOfTypeAtIndex(aType, Remaining);
      Exit(Obj);
    end;
  end;
end;

And, in use:

var
  Item: TTreeViewItem;
begin
  for I := 0 to Pred(tvAssociations.ChildrenCountOfType(TTreeViewItem)) do
  begin
    Item := tvAssociations.ChildrenOfTypeAtIndex(TTreeViewItem, I) as TTreeViewItem;
    //do some stuff with the item
  end;

Hmm… Let’s say tvAssociations contains 500 items. Using this code, first the entire list of child (and grandchild) controls is walked through to determine how many of them are of the desired class (i.e., TTreeViewItem), then the list is traversed again until the first matching child object is found, then traversed from the start once more to find the second, then from the beginning once again to find the third matching child object and so on. At the risk of sounding picky, isn’t this a good example of an anti-pattern?

‘Now now’, you might say, ‘no need to play high and mighty over a quick demo from a Developer Relations guy!’ And indeed, if this problem extended no further than a quick demo from a member of Developer Relations I would agree. However, if you browse the FMX source code, you will find this anti-pattern repeated again and again. Ever wondered why large menus in FMX can be so damn slow, even when it is the native menu bar being used? Wonder no more.

Part of the issue is that FMX, while borrowing the VCL’s component model, does not use TCollection, which would otherwise force the use of a single class for child objects. Instead, you can in principle put anything on a FMX TListBox (for example). In implementing the Items property of TListBox, the FMX author was then led to use the anti-pattern under discussion, since while in almost all cases a list box’s content will be comprised of TListBoxItem instances only, it might not be.

Particularly in the menu code, this problem is made worse by an addiction to an indexing style…

for I := 0 to Obj.Count - 1
  if Obj[I].Foo then ...

…when a more basic enumeration pattern is all that is needed:

for Item in Obj do
  if Item.Foo then ...

This snippet uses the for/in syntax, though something like FindFirst/FindNext/FindClose implements the underlying pattern just as well.

For sure, the phrase ‘more basic’ here only applies on one level. This is because in order to fully realise the ‘basic’ enumeration pattern, you need to cover the possibility that code may attempt to change the enumerated object’s state during the middle of it being enumerated (in particular, what should happen when an item is added, or attempted to be added?). Even still, the benefit of this approach – assuming it is implemented properly – is that there is no need to enumerate everything of the container internally if the calling code proves only interested in a child object that comes early on. Think of FindFirst/FindNext/FindClose: if those functions didn’t exist, and you instead had a pair of functions on the WalkEverythingToFindTheCount/WalkAgainToFindTheNth anti-pattern, enumerating the top level directory of a drive or device that contained lots of files could take ages!

Nonetheless, if implementing a bare-bones enumerator pattern is considered too taxing, a reasonable halfway house is to implement a single function that returns a dynamic array. This is the style used by Rtti.pas, and while it still involves enumerating everything internally up front, you then avoid repeatedly walking up from the start again thereafter:

type
  TFmxObjectHelper = class helper for TFMXObject
  strict private
    procedure DoAddNestedObjects<T: class>(Parent: TFmxObject;
      TopLevelOnly: Boolean; var Arr: TArray<T>; var Count: Integer);
  public
    function GetNestedObjects<T: class>(TopLevelOnly: Boolean = False): TArray<T>;
  end;

procedure TFmxObjectHelper.DoAddNestedObjects<T>(Parent: TFmxObject;
  TopLevelOnly: Boolean; var Arr: TArray<T>; var Count: Integer);
var
  Child: TFmxObject;
  I: Integer;
begin
  for I := 0 to Parent.ChildrenCount - 1 do
  begin
    Child := Parent.Children[I];
    if Child is T then
    begin
      if Length(Arr) = Count then SetLength(Arr, Count + 64);
      Arr[Count] := T(Child);
      Inc(Count);
    end;
    if not TopLevelOnly then DoAddNestedObjects<T>(Child, TopLevelOnly, Arr, Count);
  end;
end;

function TFmxObjectHelper.GetNestedObjects<T>(TopLevelOnly: Boolean): TArray<T>;
var
  Count: Integer;
begin
  Count := 0;
  DoAddNestedObjects<T>(Self, TopLevelOnly, Result, Count);
  SetLength(Result, Count);
end;

Then, in use:

var
  Item: TTreeViewItem;
begin
  for Item in tvAssociations.GetNestedObjects<TTreeViewItem> do
  begin
    //do some stuff with the item
  end;

Notice this approach also has the benefit of being slightly more stronger typed that the original – i.e., no ‘as’ cast is needed – since the array returned is an array of the class the client code is interested in, not of TFmxObject.