Delphi try finally on multiple objects

Question

I have written this piece of code:

 FD := TList<double>.Create;
 FN := TList<double>.Create;
 RemList := TList<double>.Create;
 dati := TStringList.Create;

 try

  try

   //code

  except
   on E : Exception do 
   ShowMessage('Incorrect values. Error: ' + E.Message);
  end;

 finally

  if Assigned(dati) then
   dati.Free;

  if Assigned(FD) then
   FD.Free;

  if Assigned(FN) then
   FN.Free;

  if Assigned(RemList) then
   RemList.Free;

 end;

I am using firemonkey so I won't have problems when I'll run this on mobile devices because ARC will manage the lifetime. But is this code safe when I am on windows?

I have read that I cannot have a finally and a catch all together so I need to have a nested try block. I guess that the best way is this:

FD := TList<double>.Create;
try

 FN := TList<double>.Create;
 try

  RemList := TList<double>.Create;
  try

   //and so on...

  finally
   Rem.Free;
  end;

 finally
  FN.Free;
 end;

finally
 FD.Free;
end;

Here I am sure that I'll free the object in any case and in a safe way but the code is hard to read. I have seen that Marco Cantu is suggesting the 2nd approach in object pascal handbook and I understand it, but in my case I want to avoid it.

Can something wrong happen in the first block of code I have written?

Yes, one of the destructors may raise causing to skip the following destructors. — Sertac Akyuz, Mar 09 '17 at 22:20
Rule of thumb is don't assume it is safe. I'm looking now into those two particular classes, and so far I see no evidence of a potential exception in their destructors. But I could be wrong, and you should never assume it's safe. Nest them as in your second example instead. The #1 rule of programming: Assume everything is an error, and be sure to appropriately handle them. — Jerry Dodge, Mar 09 '17 at 22:32
Destructor are contacted not to raise. If they do you are already screwed so don't worry about it. The real problem is that if one of the constructors raise you will leak the objects already created. And why are you testing for assigned before calling Free? Free already does that. — David Heffernan, Mar 09 '17 at 22:44
The second is the proper way to do things. You may not like it, but it is the proper way to write multiple `try..except` or `try..finally` blocks. IMO, it's actually easier to read than the first, because you can clearly follow the flow of the code. — Ken White, Mar 09 '17 at 23:03
@RaffaeleRossi I have just checked Marco Cantu Object Pascal Handbook and he has written 1 page and a half about this topic btw. Did you find it difficult to understand? It also has an example which is very similar to your case. — Alberto Miola, Mar 09 '17 at 23:13
After all, if the constructors and destructors are safe, then what would be the purpose of the constructor being before a `try` .. `finally` block in the first place? :-) — Jerry Dodge, Mar 09 '17 at 23:42
@JerryDodge that is a good point! It could be sa short complete answer to this ;) — Alberto Miola, Mar 09 '17 at 23:47

Alberto Miola · Accepted Answer · 2022-12-15T20:29:50.450

2

If I was you I'd use the second block of code because as you already know it's better; the TList<double>.Create (like the other constructors) are "safe" as you say but it's better if you always assume that there is a risk behind the corner.

Note that when I say safe I mean that they won't raise an exception if you call them in that way. An exception could happen in any case that you maybe cannot predict, so you really should protect the code with a try finally.

You are also using the Assigned() which is useless. If you look at the implemetation of Free you will find this code (it's also commented):

procedure TObject.Free;
begin
// under ARC, this method isn't actually called since the compiler translates
// the call to be a mere nil assignment to the instance variable, which then calls _InstClear
{$IFNDEF AUTOREFCOUNT}
  if Self <> nil then
    Destroy;
{$ENDIF}
end;

The Assigned() returns true or false if the object is equal to nil or not, so basically you're doing an useless double check.

edited Dec 15 '22 at 20:29

answered Mar 09 '17 at 23:03

Alberto Miola

4,643
8
35
49

Ok I didn't know about the Free thing. so The Assign is useless? – Raffaele Rossi Mar 09 '17 at 23:06
@RaffaeleRossi http://stackoverflow.com/questions/8548843/why-should-i-not-use-if-assigned-before-using-or-freeing-things – Jerry Dodge Mar 09 '17 at 23:07
Ok thanks both, I'll try to format somehow the block 2 – Raffaele Rossi Mar 09 '17 at 23:10
5

It is wrong to state that these constructors won't raise. Every one can raise. For instance out of memory. – David Heffernan Mar 09 '17 at 23:24

Dalija Prasnikar · Answer 2 · 2023-01-27T12:55:14.347

While second code with nested try...finally is ultimately by the book way to handle object construction/deconstruction blocks, you can also safely group those into single try...finally block.

Some facts that will help us squeezing above code.

Constructors can always throw exceptions
Destructors must never ever throw exceptions. If you are dealing with classes that have broken destructors that can throw you have bigger issues at hand.
Non-managed local variables are not initialized - in non-ARC compiler that includes object references.

So your code would look like this:

// initialize object references so we can safely use them
FD := nil;
FN := nil;
RemList := nil; 
dati := nil;

try
  FD := TList<double>.Create;
  FN := TList<double>.Create;
  RemList := TList<double>.Create;
  dati := TStringList.Create;

  // do something

finally   
   dati.Free;
   RemList.Free;
   FN.Free;
   FD.Free;
end;

Another way to handle multiple objects with single try...finally block is to construct one of the instances outside the block and the rest from within. This kind of handling requires one less nil assignment.

// initialize object references so we can safely use them
FN := nil;
RemList := nil; 
dati := nil;

FD := TList<double>.Create;
try
  FN := TList<double>.Create;
  RemList := TList<double>.Create;
  dati := TStringList.Create;

  // do something

finally   
   dati.Free;
   RemList.Free;
   FN.Free;
   FD.Free;
end;

If any of constructors fail finally block will be executed and all references that were allocated will get cleared. Since Free tests for nil before calling instance destructor, it is safe to call it on nil reference.

Above covers how to write safe construction/deconstruction chain. If your do something code throws exception you can handle it inside with try..except block as you are doing it now.

Keep in mind that above code does not handle exceptions thrown from constructors, it just performs proper cleanup. If you want to handle possible construction exceptions you will have to wrap above code in another try...except block or let them propagate to higher level.

Delphi try finally on multiple objects

2 Answers2

Linked