30

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?

menjaraz
  • 7,551
  • 4
  • 41
  • 81
  • Please explain why you added the try/finally block in this code? Andreas may be correct in inferring that it's the understanding of what try/finally means that is behind the question. – David Heffernan Jul 06 '11 at 18:39
  • not sure what you mean, but I added the try..finally to make sure TSomeObject is released from memory once its done what needs to be done –  Jul 08 '11 at 10:06
  • 1
    In this answer there is a number of different try-finally patterns with their cons and pros described - https://stackoverflow.com/a/46930029/976391 – Arioch 'The Nov 18 '22 at 12:51

7 Answers7

42

You just need two try/finally blocks:

Screen.Cursor:= crHourGlass;
try
  Obj:= TSomeObject.Create;
  try
    // do something
  finally
    Obj.Free;
  end;
finally
  Screen.Cursor:= crDefault;
end;

The guideline to follow is that you should use finally rather than except for protecting resources. As you have observed, if you attempt to do it with except then you are forced to write the finalising code twice.

Once you enter the try/finally block, the code in the finally section is guaranteed to run, no matter what happens between try and finally.

So, in the code above, the outer try/finally ensures that Screen.Cursor is restored in the face of any exceptions. Likewise the inner try/finally ensures that Obj 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.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 15
    + for "in most cases you should **not** attempt to handle exceptions." – Cosmin Prund Jul 07 '11 at 05:55
  • Nicely explained! Q: Can we predict when the `finally` block will be executed? [This article](http://sheepdogguides.com/dt4u.htm) says that on exception "_execution jumps to the finally part"_. What I have known so far is that the `finally` block is *always* executed, but with no guarantee to *when* it is executed. Plus, if it really jumped to `finally` on exception, wouldn't `except` and `finally` be behaving similarly, allowing their interchangeable usages? BTW, the last question is not for you per se, rather for the author of that article, but you are more than welcome to explain. TIA. – Sнаđошƒаӽ Apr 01 '19 at 10:14
  • @Sнаđошƒаӽ The finally block executes whether or not there is an exception, so it is not interchangeable with except. And as for when it executes, well it executes after everything in the try block is executed. Note that the try block may exit early due to Exit, Break, Continue or raise. – David Heffernan Apr 01 '19 at 11:07
  • 1
    Thanks, but my primary concern hasn't been answered, and I am sure that's because I didn't make it clear enough. Let me try again. In case of an exception being raised in a try-finally block, does the execution jump to `finally` block as soon as the exception is raised (as mentioned in the article I cited earlier)? TIA. And I know `except` and `finally` are not interchangeable; I only mentioned it to strengthen my position against the idea in that article ;) – Sнаđошƒаӽ Apr 01 '19 at 13:34
  • 2
    @Sнаđошƒаӽ It's more complicated. The exception can be raised in functions called in the try block, or indeed, functions called by those functions, and so on. There can be other exceptions handlers there that execute before the finally block here is reached. – David Heffernan Apr 01 '19 at 13:38
18

Your original code isn't quite as bad as you think (it's bad, though):

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;

Obj.Free will be executed no matter what happens when you // do something. Even if an exception occurrs (after try), the finally block will be executed! That is the whole point of the try..finally construct!

But you also want to restore the cursor. The best way is to use two try..finally constructs:

procedure TForm1.Button1Click(Sender: TObject);
var
  Obj: TSomeObject;
begin

  Screen.Cursor := crHourGlass;
  try
    Obj := TSomeObject.Create;
    try
      // do something
    finally
      Obj.Free;
    end;
  finally
    Screen.Cursor := crDefault;
  end;

end;
Andreas Rejbrand
  • 105,602
  • 8
  • 282
  • 384
  • @David that is my problem, when an error occurs the Cursor is stuck as an hour glass –  Jul 06 '11 at 18:31
  • 3
    @Andreas I am going to quote you. "You never know. Its like wearing seatbelts. Most of the time, it is just plain unnecessary, but it is worth it, because there is always a risk of an accident." – David Heffernan Jul 06 '11 at 19:40
  • I am using this all the time to 'return' the cursor back to normal. +1 – Gabriel Jul 07 '11 at 12:14
  • No need for the double try..finally, Andreas. Just invert the order in finally - Obj will be always freed, all bad that can happens is that cursor fail to change. – Fabricio Araujo Jul 07 '11 at 20:31
  • @Fabricio: I guess we should also invert the order just before `try` as well, then. But it is an annoying bug if the cursor sticks to being `crHourGlass`, isn't it? And writing a nestled `try..finally` isn't much work... – Andreas Rejbrand Jul 07 '11 at 20:37
  • @Andreas: But will be a visible bug - so someone will only need to trace where the last cursor change happened. Memory leaks are much more irritant to figure out what happened - since they are "silent" bugs (no exception). – Fabricio Araujo Jul 10 '11 at 22:30
  • @Fabricio: The right way is to use two `try..finally`. (That is, I opposes your original comment.) – Andreas Rejbrand Jul 10 '11 at 23:11
  • If the TSomeObject constructor fails, then the status of the cursor is less relevant. Your program is in deeper problem. You would care less about the cursor at that point. Because of this, the second block of code shown the Andreas is juuuust fiiiine. – Gabriel Sep 30 '19 at 08:42
16

As others have explained, you need to protect the cursor change with try finally block. To avoid writing those I use code like this:

unit autoCursor;

interface

uses Controls;

type
  ICursor = interface(IInterface)
  ['{F5B4EB9C-6B74-42A3-B3DC-5068CCCBDA7A}']
  end;

function __SetCursor(const aCursor: TCursor): ICursor;

implementation

uses Forms;

type
  TAutoCursor = class(TInterfacedObject, ICursor)
  private
    FCursor: TCursor;
  public
    constructor Create(const aCursor: TCursor);
    destructor Destroy; override;
  end;

{ TAutoCursor }
constructor TAutoCursor.Create(const aCursor: TCursor);
begin
  inherited Create;
  FCursor := Screen.Cursor;
  Screen.Cursor := aCursor;
end;

destructor TAutoCursor.Destroy;
begin
  Screen.Cursor := FCursor;
  inherited;
end;

function __SetCursor(const aCursor: TCursor): ICursor;
begin
  Result := TAutoCursor.Create(aCursor);
end;

end.

Now you just use it like

uses
   autoCursor;

procedure TForm1.Button1Click(Sender: TObject);
var
  Obj: TSomeObject;
begin
  __SetCursor(crHourGlass);

  Obj:= TSomeObject.Create;
  try
    // do something
  finally
    Obj.Free;
  end;
end;

and Delphi's reference counted interface mechanism takes care of restoring the cursor.

Johan
  • 74,508
  • 24
  • 191
  • 319
ain
  • 22,394
  • 3
  • 54
  • 74
  • Very clever, perhaps too clever. I wouldn't advocate this, especially not to someone relatively new to try/finally and try/except. And if you must do this, why bother with ICursor. Just get __SetCursor to return IInterface. – David Heffernan Jul 06 '11 at 19:09
  • Why not, there is nothing dangerous there. The compiler just creates an invisible `try finally` block for you... The ICursor is there to provide addidional functionality (not included in the sample), ie property to read the original cursor to be restored. – ain Jul 06 '11 at 19:18
  • Oh it works perfectly. It's just another level of complexity. – David Heffernan Jul 06 '11 at 19:27
  • 4
    Very clever, but Delphi just adds an invisible `try finally` behind the scenes to make sure the reference count goes to zero, so you are just adding complexity to safe a few keystrokes whilst forcing the compiler to generate essentially the same code as doing an extra try-finally yourself. I fail to see the benefit here It looks like a code obfustification contest. IMO the explicit try finally is much better because it explains the intend of the code better and code is written for human consumption, not for the machine. – Johan Jul 07 '11 at 08:26
  • 11
    I don't see it as adding complexity; the benefit is that you don't have multiple (visible) nested try finally blocks which make code harder to read. – ain Jul 07 '11 at 08:40
  • -1 obfuscated code. When you write something, consider who has to read it next. Not just code btw. – Sam Jul 14 '11 at 01:42
  • This is clever, but not recommended. Don't make the code hard to read for no real benefit. Keep it simple. – kling Dec 02 '14 at 14:43
  • 2
    This concept is known as RAII and very commonly used in C++. It guarantees to free the resources and makes any nested try-finally blocks (which obfuscated the code way more in my opinion) obsolete. See https://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization – niks May 14 '18 at 08:28
  • Indeed clever. But I avoid interfaces whenever possible. +1 anyway for the nice theoretical idea :) – Gabriel Sep 30 '19 at 08:45
  • I do something similar with stack-allocated objects `Obj := TSomeObj.Create; AutoFree(Obj);` The **AutoFree** function wraps the object in an interface, which destroys the contained object when the interface is destroyed - which is this case is the bottom of the function. We're using the features of the Delphi language to make code better! – Ian Boyd Oct 11 '22 at 21:33
6

I think the most "correct" version would be this:

procedure TForm1.Button1Click(Sender: TObject);
var
  Obj: TSomeObject;
begin
  Obj := NIL;
  Screen.Cursor := crHourGlass;
  try
    Obj := TSomeObject.Create;
    // do something
  finally
    Screen.Cursor := crDefault;
    Obj.Free;
  end;
end;
dummzeuch
  • 10,975
  • 4
  • 51
  • 158
  • 3
    `Obj` is not freed if `Screen.Cursor := crDefault` raises an exception. – David Heffernan Jul 07 '11 at 10:14
  • 1
    Agreed. The most correct version is definitely the one with two `try..finally`. – Andreas Rejbrand Jul 07 '11 at 11:27
  • 1
    "if Screen.Cursor := crDefault raises an exception" - - - Totally agree with you but can it really happen? What are the chances in real world?? – Gabriel Jul 07 '11 at 12:18
  • 2
    @david: I have to agree. If I switch the order of statements in the finally block, Screen.Cursor would not be changed back if Obj.Free raises an exception. That leaves two try..finally constructs as the only correct solution, as Andreas already said. I usually assume that Screen.Cursor assignments cannot fail, but that might just be wrong. – dummzeuch Jul 09 '11 at 09:17
3

If you found your way here and were looking for how you make a try-except-finally construct from C# in Delphi:

// C#
try
{
    // Do something
}
catch
{
    // Exception!
}
finally
{
    // Always do this...
}

The answer is that you cannot do this directly. Instead, as @sacconago hints at, nest the try blocks as follows:

// Delphi
try
    try
        // Do something
    except
        // Exception!
    end;
finally
    // Always do this...
end;

One nice feature of Delphi is that you can nest the blocks as try...except...finally or try...finally...except, though the former would be more common.

AlainD
  • 5,413
  • 6
  • 45
  • 99
2

I would do it like this:

var
  savedCursor: TCursor;
  Obj: TSomeObject;
begin
  savedCursor := Screen.Cursor;
  Screen.Cursor := crHourGlass;
  Obj:= TSomeObject.Create;
  try
    try
      // do something
    except
      // record the exception
    end;
  finally
    if Assigned(Obj) then
      Obj.Free;
    Screen.Cursor := savedCursor;
  end;
end;
sacconago
  • 29
  • 3
  • 2
    Where is Obj declared? In the original question it is a local variable. If Obj was assigned before, and this code assigns a new instance to Obj, unexpected things could happen. – mjn Aug 06 '14 at 08:33
  • 1
    Don't do this. (1) If the `TSomeObject.Create` constructor fails, you are stuck with an hourglass cursor, which really makes you app look bad. (2) You never need to write `if Assigned(X) then X.Free` beucase `X.Free` basically does `if Assigned(X) then X.Destroy`, so basically you do `if Assigned then if Assigned then Destroy`. (3) If the destructor fails (which it shouldn't, but it isn't impossible), you are stuck with the hourglass cursor. Just keep the standard idiom of using two `try..finally` blocks! – Andreas Rejbrand Sep 30 '19 at 08:59
2

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:

procedure TForm1.Button1Click(Sender: TObject);
var
   Obj: TSomeObject;
begin  
     try
        Obj := NIL;
        try
          Screen.Cursor := crHourGlass;
          Obj := TSomeObject.Create;
          // do something
        finally
          Screen.Cursor := crDefault;
          if assigned(Obj) then FreeAndNil(Obj);
        end;
     except
        On E: Exception do ; // Log the exception
     end;
end;

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...

K.Sandell
  • 1,369
  • 10
  • 19
  • 4
    Obj will leak if `Screen.Cursor := crDefault` raises. Also, what's with `if assigned(Obj) then FreeAndNil(Obj)`? Calls to TObject.Free already include the `if assigned(...)` test. – David Heffernan Jul 07 '11 at 11:48
  • Also, if TSomeObject.Create raises an exception its destructor will automatically be called (test it!). Obj will not be assigned at all in this case because the exception occurred before the assignment, so there is no need to free it. So, either Obj := TSomeObj.Create or Screen.Cursor could be done outside the try..finally. – dummzeuch Jul 09 '11 at 09:14
  • 1
    The 'if assigned(...)' is actually legacy from older days .. and I will change my ways! - Didn't realize an exception in create() will always call destroy .. thanx! – K.Sandell Jul 12 '11 at 13:11