-1

Does a more efficient way of populating ListBox with file names from TDirectory.GetFiles exist?

procedure PopListBox(var lb: TListBox; dir, ext: String; so: TSearchOption);
var
  i: Integer;
  iend: Integer;
  oc: TStringDynArray;
begin
  oc := TDirectory.GetFiles(dir, ext, so);
  iend := Length(oc);
  i := 0;
  repeat
    lb.Items.Add(oc[i]);
    Inc(i);
  until (i > (iend - 1));
end;

I would like input from the community on this approach.

Arch Brooks
  • 183
  • 1
  • 3
  • 19
  • What's so inefficient about it? The only other thing I can think of is `FindFirst` / `FindNext` which is trickier to use, hence the implementation of this function. Is this for just Windows or cross-platform? – Jerry Dodge Oct 01 '15 at 00:54
  • Jerry this if for windows initially. Cross platform is next. Strong criticism will only make the code tighter. – Arch Brooks Oct 01 '15 at 01:02
  • Adding `lb.Items.BeginUpdate` and `EndUpdate` will help if there are a lot of entries – David A Oct 01 '15 at 01:05
  • GetFiles is just inefficient. IOUtils is poorly implemented. – David Heffernan Oct 01 '15 at 05:54

1 Answers1

3

It isn't any more efficient, but you can remove a couple of variables and a few lines of code:

procedure PopListBox(var lb: TListBox; dir, ext: String; so: TSearchOption);
var
  oc: TStringDynArray;
  s: string;
begin
  oc := TDirectory.GetFiles(dir, ext, so);
  lb.Items.BeginUpdate;
  try
    for s in oc do
      lb.Items.Add(s);
  finally
    lb.Items.EndUpdate;
  end;
end;
Ken White
  • 123,280
  • 14
  • 225
  • 444
  • With an empty list box I am not sure why the BeginUpdate is necessary. According to the documentation it is called automatically. Please expound further on why it is being used. Thanks – Arch Brooks Oct 01 '15 at 01:25
  • 2
    @ArchBrooks: It stops the UI from being updated (painted) after every item is added, and only updates it once at the end when `EndUpdate` is called. The repeated (and unnecessary) painting is very slow; omitting it is a performance boost. (Try it yourself with a few hundred or thousand items added to a listbox in a loop.) – Ken White Oct 01 '15 at 01:33
  • thanks for that clarity I will be sure to insert and use this approach when populating a list box. . – Arch Brooks Oct 01 '15 at 01:38
  • I myself have questioned in some scenarios this concept - because the repainting doesn't usually happen until after the thread queue continues to process. Unless it's a forced paint, that's another story. But when painting is handled by a Windows Message, that message won't be processed until after the code exits anyway. Still, I use `BeginUpdate` / `EndUpdate` anyway because in most scenarios, it does give improvement. – Jerry Dodge Oct 01 '15 at 01:53
  • @Jerry: Windows itself provides the message that tells the listbox to defer painting, so when and where the messages are pumped is irrelevant. At some point they will be, and placing the control in "you're being updated, so don't paint until we're ready" mode is meaningful. Read the API docs on the listbox control. – Ken White Oct 01 '15 at 02:34
  • Yes indeed, Windows is responsible for producing this message, and this message may occur hundreds of times in 1 second. But the actual painting is done by the same thread which is in a loop in this scenario. Therefore I can't imagine it painting until after this procedure exits and returns to the queue (unless something is forcefully causing it to happen, such as `Application.ProcessMessage` which is poor practice anyway). The way I understand, if a control has multiple paint messages pending, it only acknowledges one of them and ignores the rest. – Jerry Dodge Oct 01 '15 at 05:39
  • @Jerry: BeginUpdate also informs the items not to call some methods or dispatch some events. Before arguing the merits of Begin/EndUpdate, spend some time with the source code for the TListBox and it's Items. – Ken White Oct 01 '15 at 14:43