Think twice before raising exception in constructor

Recently prolific Delphi writer Nick Hodges declared a war on the use of nil pointers. It is arguable whether nil pointers usage is bad or not, but the point is the “Guard Pattern” proposed is really an antipattern. Here is a simple demo:

program Project11;

{$APPTYPE CONSOLE}

uses
  SysUtils, Classes;

type
  TMyClass = class
  private
    FList: TStringList;
  public
    constructor Create(AList: TStringList);
    destructor Destroy; override;
  end;

constructor TMyClass.Create(AList: TStringList);
begin
  if AList = nil then
  begin
    raise Exception.Create('Don''t you dare pass me a nil reference!);
  end;
  FList:= AList;
  FList.Add('TMyClass instance created');
end;

destructor TMyClass.Destroy;
begin
  FList.Add('TMyClass instance destroyed');
  inherited Destroy;
end;

procedure Test;
begin
  TMyClass.Create(nil);
end;

begin
  try
    Test;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

If you understand how exceptions raised in a constructor work you see that the above code raises 2 exceptions: the first is the “wanted” exception in the constructor, the second is the “unwanted” access violation in the destructor.

The moral is: if you raise an exception in a constructor be very careful when writing the destructor, cause you need to treat a case of improperly initialized instance.

Advertisements

17 thoughts on “Think twice before raising exception in constructor

  1. See… I have read for many years that a destructor is a place where you cannot count on things as granted. Your destructor code is wrong. You should verify if the list is valid.

    Anyways I dont understand your point related to Nick. If you accept the Nil as parameter in your constructor with no error raising, your code will fail on the destructor either! That clearly shows that you need to be more careful with destructors, this is what I learned from your post.

    Raising an exception inside of a constructor is obviously valid like any other place in the code.

    • The point is that the Nick’s “Guard pattern” does not solve the nil reference problem. You still need to handle nil references in a destructor, even if you try to avoid them by raising a constructor exception.

      • “The point is that the Nick’s “Guard pattern” does not solve the nil reference problem” – just noticed that.

        Honestly, this point is not clear from your comment and this post. I had impression that you are saying that you should never raise exceptions in constructors.

  2. hm.. why you don’t use AfterConstruction and BeforeDestruction? Then you only get the “corret” exception:

    type
    TMyClass = class
    private
    FList: TStringList;
    public
    constructor Create(AList: TStringList);
    destructor Destroy; override;
    procedure AfterConstruction; override;
    procedure BeforeDestruction; override;
    end;

    procedure TMyClass.AfterConstruction;
    begin
    inherited;
    FList.Add(‘TMyClass instance created’);
    end;

    procedure TMyClass.BeforeDestruction;
    begin
    inherited;
    FList.Add(‘TMyClass instance destroyed’);
    end;

    constructor TMyClass.Create(AList: TStringList);
    begin
    if AList = nil then
    begin
    raise Exception.Create(‘Don”t you dare pass me a nil reference!’);
    end;
    FList:= AList;
    end;

    destructor TMyClass.Destroy;
    begin
    inherited Destroy;
    end;
    ….

  3. Your example places the constructor inside the try block. This is a common mistake, and if the constructor fails as you demonstrate the object was never constructed. The problem is not with the constructor raising an exception, it’s with the incorrect usage of try finally block.

    • Good advice, but you should not do work in constructors in the first place. Destructors “undo” constructors work; for example the TFileStream constructor opens a file, destructor closes it. Fortunately it is safe to pass invalid file handle to WinAPI CloseHandle function, so possible TFileStream constructor exception does not require special treatment in TFileStream destructor; generally it is not so.

      • Just a FYI: Windows 8, with an invalid file handle to WinAPI CloseHandle while running under a debugger will raise an exception!

  4. Serg, you are wrong.

    Destructors should not assume that object is fully initialized. This is even indicated in Delphi docs: http://docwiki.embarcadero.com/RADStudio/XE7/en/Methods#Destructors

    “Destroy must be prepared to dispose of partially constructed objects. Because a constructor sets the fields of a new object to zero or empty values before performing other actions, class-type and pointer-type fields in a partially constructed object are always nil. A destructor should therefore check for nil values before operating on class-type or pointer-type fields. Calling the Free method (defined in TObject) rather than Destroy offers a convenient way to check for nil values before destroying an object.”

    This is, like, the first thing to know about destructors.

    • Strictly speaking I disagree with the Docs you cited. Normally we should write destructors without fearing that a constructor can throw exception; and if the constructor can throw exception we should know what may be the cause and what instance fields are uninitialized because of the exception. Constructor exceptions should be very exceptional.

      • And why you want to have such system, where you fear to raise exception, but do not fear to write destructors? Honestly, I see little reason to do so.

  5. Pingback: Новости из мира Delphi 22.12.2014 – 25.01.2015 | Delphi 2010 ru

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s