Take the following code as a sample:
procedure TForm1.Button1Click(Sender: TObject);
var
Obj: TSomeObject;
begin
Screen.Cursor:= crHourGlass;
Obj:= TSomeObject.Create;
try
// do something
finally
Obj.Free;
end;
Screen.Cursor:= crDefault;
end;
if there was an error happening in the // do something
section, the TSomeObject that was created I assume will not be freed and the Screen.Cursor will still be stuck as an Hour Glass, because the code was broke before getting to those lines?
Now unless I am mistaking, an Exception statement should be in place to deal with any such occurence of an error, something like:
procedure TForm1.Button1Click(Sender: TObject);
var
Obj: TSomeObject;
begin
try
Screen.Cursor:= crHourGlass;
Obj:= TSomeObject.Create;
try
// do something
finally
Obj.Free;
end;
Screen.Cursor:= crDefault;
except on E: Exception do
begin
Obj.Free;
Screen.Cursor:= crDefault;
ShowMessage('There was an error: ' + E.Message);
end;
end;
Now unless I am doing something really stupid, there should be no reason to have the same code twice in the Finally block and after, and in the Exception block.
Basically I sometimes have some procedures that may be similar to the first sample I posted, and if I get an error the cursor is stuck as an Hour Glass. Adding the Exception handlers help, but it seems a dirty way of doing it - its basically ignoring the Finally block, not to mention ugly code with copy-paste from the Finally to Exception parts.
I am still very much learning Delphi so apologies if this appears to be a straight forward question/answer.
How should the code be correctly written to deal with the Statements and correctly freeing objects and capturing errors etc?
As others have explained, you need to protect the cursor change with
try finally
block. To avoid writing those I use code like this:Now you just use it like
and Delphi's reference counted interface mechanism takes care of restoring the cursor.
I would do it like this:
You just need two
try/finally
blocks:The guideline to follow is that you should use
finally
rather thanexcept
for protecting resources. As you have observed, if you attempt to do it withexcept
then you are forced to write the finalising code twice.Once you enter the
try/finally
block, the code in thefinally
section is guaranteed to run, no matter what happens betweentry
andfinally
.So, in the code above, the outer
try/finally
ensures thatScreen.Cursor
is restored in the face of any exceptions. Likewise the innertry/finally
ensures thatObj
is destroyed in case of any exceptions being raised during its lifetime.If you want to handle an exception then you need a distinct
try/except
block. However, in most cases you should not attempt to handle exceptions. Just let it propagate up to the main application exception handler which will show a message to the user.If you handle the exception to low down the call chain then the calling code will not know that the code it called has failed.
I think the most "correct" version would be this:
Your original code isn't as bad as you think:
Obj.Free
will be executed no matter what happens when you// do something
. Even if an exception occurrs (aftertry
), thefinally
block will be executed! That is the whole point of thetry..finally
construct!But you also want to restore the cursor. The most pedantic way is to use two
try..finally
constructs:[However, I would also not mind
too much. The risk of
Screen.Cursor := crHourGlass
failing is pretty low, but in such a case the object will not be freed (thefinally
will not run because you aren't inside thetry
), so the doubletry..finally
is safer.]Having done a lot of code in services/servers that needs to handle exceptions and not kill the app I usually go for something like this:
Note the try finally; inside the try except; and the placement of Obj creation.
if the Obj creates other things inside it's constructor, it may work half-way and fail with an exception inside the .create(); but still be a created Obj. So I make sure that the Obj is always destroyed if it's been assigned...