Delphi - Must I free all elements inside TObject before release itself?

Question

Please, about this code:

type
  TClient = class(TObject)
  public
    Host: String;
    Queue: TIdThreadSafeStringList;
  end;

var
  Clients: TThreadList;

procedure TMain.FormCreate(Sender: TObject);
const
  Hosts: Array[0..4] of String = (
    'HOST1', 'HOST2', 'HOST3', 'HOST4, 'HOST5'
  );
var
  I: Integer;
  List: TList;
  Client: TClient;
begin
  Clients := TThreadList.Create;
  Clients.Duplicates := dupAccept;
  for I := Low(Hosts) to High(Hosts) do
  begin
    Client := TClient.Create;
    Client.Host := Hosts[I];
    Client.Queue := TIdThreadSafeStringList.Create;
    Clients.Add(Client);
    Client := nil;
  end;
end;

I would like to know if the correct way of releasing it memory is:

procedure TMain.FormDestroy(Sender: TObject);
var
  I: Integer;
  List: TList;
begin
  List := Clients.LockList;
  try
    for I := 0 to List.Count - 1 do
      TClient(List[I]).Free;
  finally
    Clients.UnlockList;
    Clients.Free;
  end;
end;

Or maybe like this:

procedure TMain.FormDestroy(Sender: TObject);
var
  I: Integer;
  List: TList;
begin
  List := Clients.LockList;
  try
    for I := 0 to List.Count - 1 do
    begin
      TClient(List[I]).Queue.Free;
      TClient(List[I]).Free;
    end;
  finally
    Clients.UnlockList;
    Clients.Free;
  end;
end;

In other words, I would like to know if when I release an object (TClient) all elements (Queue) are released automatically, or must I do it manually.

Thanks!

Answer 1

The queue object needs to be destroyed when the client object is destroyed. However, the correct way to do this is to make the client class take charge of its members.

type
  TClient = class
  private
    FHost: String;
    FQueue: TIdThreadSafeStringList;
  public
    constructor Create(const Host: string);
    destructor Destroy; override;
  end;
....
constructor TClient.Create(const Host: string);
begin
  inherited Create;
  FQueue := TIdThreadSafeStringList.Create;
  FHost := Host;
end;

destructor TClient.Destroy;
begin
  FQueue.Free;
  inherited;
end;

If you do it this way then it's not possible to instantiate the class and fail to instantiate its members. Do it your way, and every time you need to instantiate the class then you have to repeat the code to instantiate the members. It's just all too easy to make a mistake that way. What's more it makes the code harder to read and maintain.