How to correctly write Try..Finally..Except statements?

Question

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.

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;

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.

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;