Where to free dynamically allocated TFrame's components' objects?

Question

I have a form containing a TFrame. The TFrame contains a ComboBox that is dynamically populated. Each ComboBox entry has an associated object. By the time the overridden destructor for the TFrame is called, the Items in the ComboBox have already been cleared without freeing their associated objects. This happens whether I drop the ComboBox on the form in designer view, or dynamically create it in code with either nil or the TFrame as its owner. I currently use the OnDestroy event of the containing TForm to call a clean-up procedure of the contained TFrame.

Is there a better way that would not need an explicit procedure call by the TFrame's container? Where ideally should the objects added dynamically to the ComboBox be freed?

Answer 1

You say that when the destructor for the TFrame is called, the Items of the ComboBox have already been cleared. That's not the case, ComboBox items are never cleared. When Items is destroyed by the ComboBox, they've got a count of only 0.

When you exit your application and the VCL destroys the form containing the frame and the ComboBox, the native ComboBox control is also destroyed by the OS since it is placed in a window being destroyed. When you later access the items to be able to free your objects in the frame destructor, the VCL have to recreate a native ComboBox control, having an item count of 0.

The solution I'd propose is easy. Don't leave freeing your frame to the framework, instead, destroy your frame in the OnDestroy event of your form. That would be before the underlying window of the form is destroyed, hence you'll be able to access your objects.

form unit

procedure TMyForm.FormDestroy(Sender: TObject);
begin
  MyFrame.Free;
end;

frame unit

destructor TMyFrame.Destroy;
var
  i: Integer;
begin
  for i := 0 to ComboBox1.Items.Count - 1 do
    ComboBox1.Items.Objects[i].Free;
  inherited;
end;

Answer 2

You could utilize the TFrame's WM_DESTROY handler like this:

unit Unit2;

interface

uses
  Windows, Messages, SysUtils, Classes, Controls, Forms, StdCtrls;

type
  TFrame1 = class(TFrame)
    ComboBox1: TComboBox;
  private
    procedure WMDestroy(var Msg: TWMDestroy); message WM_DESTROY;
    procedure FreeComboBoxItems;
  public
    constructor Create(AOwner: TComponent); override;
  end;

implementation

{$R *.dfm}

constructor TFrame1.Create(AOwner: TComponent);
begin
  inherited;
  // Add some object items to the ComboBox
  ComboBox1.AddItem('a', TButton.Create(nil));
  ComboBox1.AddItem('b', TMemoryStream.Create);
  ComboBox1.AddItem('c', TList.Create);
end;

procedure TFrame1.WMDestroy(var Msg: TWMDestroy);
begin
  // Make sure the TFrame is actually destroying - not recreated
  if (csDestroying in ComponentState) then
    FreeComboBoxItems;
  inherited;
end;

procedure TFrame1.FreeComboBoxItems;
var
  I: Integer;
begin
  OutputDebugString('TFrame1.FreeComboBoxItems');
  with Self.ComboBox1 do
    for I := 0 to Items.Count - 1 do
    begin
      OutputDebugString(PChar(Items.Objects[I].ClassName + '.Free'));
      Items.Objects[I].Free;
    end;
end;

end.

---

Another option is to create a Base ancestor TAppBaseForm class and a TAppBaseFrame for the entire application, and derive all your Forms as TAppBaseForm and all Frames as TAppBaseFrame. This way the TAppBaseForm could notify all it's child TAppBaseFrame that the owner Form is destroyed on TAppBaseForm.FormDestroy event handler. At that point the ComboBox items are still valid (as described by Sertac Akyuz's answer).

Answer 3

Your question isn't really usefull, because - generally speaking - it is discouraged to store data (or objects in your case) in a GUI control. See also David's comment on how to change your design.

What makes the question kind of interresting to answer though is the difference between the combo box being a child of a form directly and being a child of another child of the form (your frame in this case). Apparently, the combo box items are destroyed before the destructor of that frame is called. Obvious alternatives to explore are then: overriding Frame.BeforeDestruction, overriding Frame.DestroyWindowHandle, overriding Frame.DestroyWnd, or catching WM_DESTROY in an overridden Frame.WndProc, but none of them is called before the items are already gone.

The next thing to try is to repeat this for the combo box. It turns out that when WM_DESTROY arrives at the combo box that the items are still there. However, beware of catching that message ónly when the control really is being destroyed, because the VCL might recreate a combo box frequently. Implement it using an interposing class for TComboBox, as follows:

unit Unit2;

interface

uses
  Windows, Messages, Classes, Controls, Forms, StdCtrls;

type
  TComboBox = class(StdCtrls.TComboBox)
  protected
    procedure WndProc(var Message: TMessage); override;
  end;

  TFrame1 = class(TFrame)
    ComboBox1: TComboBox;
  end;

implementation

{$R *.dfm}

{ TComboBox }

procedure TComboBox.WndProc(var Message: TMessage);
var
  I: Integer;
begin
  if (Message.Msg = WM_DESTROY) and (csDestroying in ComponentState) then
    for I := 0 to Items.Count - 1 do
      Items.Objects[I].Free;
  inherited WndProc(Message);
end;

end.

Now, to answer your question: "Is this a better way?"

Yes it is, because it offers assurance of the object's destruction at the frame's level. In other words: you don't have to remember to deal with this for every instance seperately.

And no it is not, because this solution requires that the objects in the combo box are allowed to be freed in whatever circumstance which restricts usage to an unnecessary extra boundary.

So, is this answer usefull? Well, if it prevents you from using your current approach, then it is.

---

Besides, I also found another alternative by setting the frame's Parent property to nil in the containing form OnDestroy handler:

procedure TForm2.FormDestroy(Sender: TObject);
begin
  Frame1.Parent := nil;
end;

In this case, you can safely destroy the objects stored in the combo box within the frame's destructor. But this solution is even worse than your current one, because it is not descriptive. Then Frame1.FreeComboObjects is much better.