Creating and Freeing components on-the-fly..

[Ian Branch 128]

Hi Team,

Is it safe/better to do this..

....
....
//
  IdSSLIOHandlerSocketOpenSSL1 := TIdSSLIOHandlerSocketOpenSSL.Create(Self);
  IdSMTP1 := TIdSMTP.Create(Self);
  IdMessage1 := TIdMessage.Create(Self);
  //
....
....
.....
  //
  IdSSLIOHandlerSocketOpenSSL1.Free;
  IdSMTP1.Free;
  IdMessage1.Free;
  //
...
end;

Or this....

....
....
//
  IdSSLIOHandlerSocketOpenSSL1 := TIdSSLIOHandlerSocketOpenSSL.Create(nil);
  IdSMTP1 := TIdSMTP.Create(nil);
  IdMessage1 := TIdMessage.Create(nil);
  //
....
....
.....
  //
  FreeAndNil(IdSSLIOHandlerSocketOpenSSL1);
  FreeAndNil(IdSMTP1);
  FreeAndNil(IdMessage1);
  //
...
end;

Or something else?

Or doesn't it matter?

Regards & TIA,

Ian

[haentschman 92]

Hi...

 

! The naming of the components with "...1" at the end seems like placing it on the form.

 

Better...

....
....
// ! not on the form
  IdSSLIOHandlerSocketOpenSSL := TIdSSLIOHandlerSocketOpenSSL.Create(nil);
  try
    IdSMTP := TIdSMTP.Create(nil);
    try
      IdMessage := TIdMessage.Create(nil);
      try
      //
....
....
.....
      //  
      finally
        IdSSLIOHandlerSocketOpenSSL.Free;
      end;
    finally
      IdSMTP.Free;
    end;
  finally
    IdMessage.Free;
  end;	
  //
...
end;

 

or

 

Placing on the form... and finish. I dont like it.

Edited March 3, 2021 by haentschman

[Ian Branch 128]

1 minute ago, haentschman said:

The naming of the componets with "...1" at the end seems like placing it on the form

Yes, I started with the components on the form but removed them in favour of creating on the fly and just got into the '1' habit. 🙂

Having said that, I take your point.  I feel a global search and replace coming on. 🙂

I guess the main thrust of the question in my mind is should I use '.Free' or 'FreeAndNil(' or doesn't it matter, with .Create(nil)??

 

Ian

[haentschman 92]

Free is enough. (for components) If you have your own objects in use and you check with Assign, then FreeAndNil is correct. Otherwise Free is also enough here.

[Ian Branch 128]

Ahh.  Excellent.  Thank you for the clarification.

[Dalija Prasnikar 1404]

If you are not dropping components on the form, then it is better that you pass nil as owner. That way you can use those components in background threads (please note that not all components are thread safe and can be used in the background, but in this case we are talking about Indy). Which is generally preferable when you are doing any kind of network operations.

 

Having multiple try...finally blocks kills readability, so I prefer using only one. You need initialize references to nil, to avoid issues if construction fails somewhere in the middle and finally might call uninitialized pointer. 

 

  IdSMTP1 := nil;
  IdMessage1 := nil;
  IdSSLIOHandlerSocketOpenSSL1 := TIdSSLIOHandlerSocketOpenSSL.Create(nil);
  try
    IdSMTP1 := TIdSMTP.Create(nil);
    IdMessage1 := TIdMessage.Create(nil);
    ...
  finally
    IdSSLIOHandlerSocketOpenSSL1.Free;
    IdSMTP1.Free;
    IdMessage1.Free;
  end;

 

 

Edited March 3, 2021 by Dalija Prasnikar

[Fr0sT.Brutal 900]

My preference:

- Place on a form only visual and related components (dialogs, etc). Place Data access components in Data module. Other ones are created dynamically

- Use .Create(Form) when component's lifecycle is the same as owner form's. This way the form cares of destroying it

- Use .Create(nil) and destroy manually otherwise

[Ian Branch 128]

Dalija - Noted.  I have just finished doing just that.

FrOsT.Brutal - Noted.

[Remy Lebeau 1436]

Here is a 3rd option:

...
IdSMTP1 := TIdSMTP.Create(nil);
try
  IdSSLIOHandlerSocketOpenSSL1 := TIdSSLIOHandlerSocketOpenSSL.Create(IdSMTP1);
  IdMessage1 := TIdMessage.Create(IdSMTP1);
  ...
finally
  IdSMTP1.Free;
end;

 

[Ian Branch 128]

Hi Remy,

Now hat's an interesting construct.

IIUC, by using IdSMTP1 like that, when IdSMTP1 is Free'd it automatically frees IdSSLIOHandlerSocketOpenSSL1 & IdMessage1.

Sneaky.

 

Ian

Edited March 3, 2021 by Ian Branch