Try or not to try

The widely used and a simple thing like exception handling is often done in a wrong way. There are documentation about try/finally and try/except statements of course. And there is a lot of different places around the web where the things are described about a certain use cases. I just wanted to explain my vision about that.

So first I will describe when we usually use the try statements then the thing how we should not use them and after that I will try to illustrate different usage patterns for the try/finally/except statements.

When to use

We only care about to use of these blocks when we know that the exceptions can be raised while executing our code. So usually we are not going to guard the local variable assignment or IF operator which compares the value of the parameter with constant. I say usually, because even such a statements can throw the exception – it all depends on the context and implementation. When you have operator overloading you can get the exception thrown from the operator overloading method. Or when using the variant types the conversion will fail for a certain data type with EVariantCastError or any other similar cases.

The use of such try/finally and try/except constraints starts when we deal with resources! So what’s the resources:

  • Allocating memory for a pointer;
  • Constructing the instance of the class (there I talk about the standard TObject descendants with the usual lifetime);
  • Allocating any system resources like handles, tokens, critical sections etc;
  • Using some error/finalization logic (when certain tasks should be performed in case of error or always after certain operation was completed).

So when Finally and when Except

This is the simplest thing to solve. You need except to handle a certain error – so to execute your code in case if certain (or any) exception is raised. And when you need to ensure that a certain part of your code is executed in the end of the some code block then you use finally.

When to not use

Hiding the errors

Try to never suppress the exceptions with the empty except end clause. This is just bad behavior! I will be not the first who writes about this – there are different books about coding style, writing better code etc, maybe also a lot of topics in the internet about this in other programming languages as well. Some core points are clearly enough stated in this stack overflow answer. So please try to avoid the following constructions:

begin
  ...
  try
    DoSomething;
    ...
  except
  end;
  ...
  DoSomethingLater;
end;

Overusing the try statements

Sometimes it can happen that the code which is absolutely safe is put into the try finally/except block and the block itself is extensively used in the loop for a certain processing of the data. Try blocks are not just a empty statements. When you add try blocks the compilers adds extra instructions and handling to your code which actually slows down your code. Of course it’s very small penalty in general code you write every day, but when we are talking about redundant try blocks in some code which is executed billion or quadrillions times – that will be just waste of the CPU cycles!

So consider following example using the loop:

for i := 1 to 10 do
  Write(i);
Readln;

which is translated to the following assembly in Win32:

tryblocks_without_try

Just imagine if we add the redundant try finally block for the loop body:

for i := 1 to 10 do
  try
    Write(i);
  finally
  end;
Readln;

the code generated will much bigger:

tryblocks_with_try

So now you should see the performance penalty of each redundant try block added to your source code.

Generic exception use

When raising the errors in your application, please do not use the code like this:

raise Exception.Create('Something went wrong!');

It’s not about the message but about the type of the exception and the exception handling in general. When you raise such an exception (of base class) the caller cannot handle it in a comfortable way! Sure you can handle it by checking the message of the exception, but that’s not the way we go.

The exceptions should be tied to the context where they can be raise or at least have some logical link. Define exception classes for your error cases like this:

type
  EMyErrorBase = class(Exception);
  
  EMyErrorNoValue = class(EMyErrorBase)
    ...
  end;

  EMyErrorNoInput = class(EMyErrorBase)
    ...
  end;

Again: do not use one exception type for all kind of errors – then you are in the same situation like before (it is of course better than just raising Exception, but still is not good). Build the structure of your exceptions – because even implementation is called SEH (structured exception handling) 🙂

You can check one “fail” unit from VCL library. Just take a look at Vcl.Imaging.pngimage:

  {Error types}
  EPNGOutMemory = class(Exception);
  EPngError = class(Exception);
  EPngUnexpectedEnd = class(Exception);
  EPngInvalidCRC = class(Exception);
  EPngInvalidIHDR = class(Exception);
  EPNGMissingMultipleIDAT = class(Exception);
  EPNGZLIBError = class(Exception);
  EPNGInvalidPalette = class(Exception);
  EPNGInvalidFileHeader = class(Exception);
  EPNGIHDRNotFirst = class(Exception);
  EPNGNotExists = class(Exception);
  EPNGSizeExceeds = class(Exception);
  EPNGMissingPalette = class(Exception);
  EPNGUnknownCriticalChunk = class(Exception);
  EPNGUnknownCompression = class(Exception);
  EPNGUnknownInterlace = class(Exception);
  EPNGNoImageData = class(Exception);
  EPNGCouldNotLoadResource = class(Exception);
  EPNGCannotChangeTransparent = class(Exception);
  EPNGHeaderNotPresent = class(Exception);
  EPNGInvalidNewSize = class(Exception);
  EPNGInvalidSpec = class(Exception);

That’s not the way it should be done! Why? Imagine you load the TPNGImage from stream and want to check if it fails because of the PNG format error and not because of any other error – then you have only one chance to handle that like this:

try
  SomePngImage.LoadFromStream(SomeStream);
except
  on E: Exception do
    if not E.ClassName.StartsWith('EPNG') then
      raise;
end;

Stringly-typed! If they would define the EPNGException (or use ENGError, whatever) and then descend all of their exceptions from it then it will much cleaner and better!

Usage patterns

When using the try blocks in most of the cases developers do not need to think about the way they use the statement because they already have the patterns in their mind and do that automatically. But while that’s true for the experienced developers (which I assume are not the target pool of this post :)) it is not true for the beginners.

Why is this important? Because using the try blocks correctly can and will avoid the following issues in your applications:

  • Hidden errors which are hard to detect in production system (because they are just suppressed);
  • Memory leaks of the objects created in code which “believes” that the code between creation and releasing of the instance will never fail (that is very popular issue between all kind of developers – even the experienced ones sometimes trust too much in their code);
  • Resource leaks of the handles/tokens and other stuff (because of the same reason);
  • Performance penalty when overusing the try statements;
  • Application debugging hell, when many things are exception-validated (kind of exception-driven checks) .

Creating the object

var
  X: TObject;
begin
  X := TObject.Create;
  try
    // Do the business with the X
  finally
    X.Free;
  end;
end;

Notes:

  • Always create before try (outside of the try/finally block);
  • Do not put any code between Create and try.

Creating multiple objects

var
  x1, x2, x3: TObject;
begin
  ...
  x1 := nil;
  x2 := nil;
  x3 := nil;
  try
    ...
    x1 := TObject.Create;
    ...
    x2 := TObject.Create;
    ...
    x3 := TObject.Create;
    ...
  finally
    x3.Free;
    x2.Free;
    x1.Free;
  end;
end;

Notes:

  • Set all objects to nil before the try block;
  • Release all objects in finally (preferably in reverse order of creation);
  • Do not check for nil in finally as it is already done in Free method of RTL!

Adding the object to a collection

type
  TCar = class //non interfaced
  end;
...
var
  X: TCar;
  L: TObjectList<TCar>;
begin
  ...
  X := TCar.Create;
  try
    // Do any business needed before adding to the collection
    L.Add(X);
  except
    X.Free;
    raise;
  end;
end;

Notes:

  • Never have the code between adding the object to the collection and except clause;
  • Do not forget the raise in the except clause!

The following line:

L.Add(TCar.Create);

is a potential memory leak. Sure I am a bit paranoid there, but if you get any exception in Add method the TCar instance will be leaked!

Working with a file

var
  F: TFileStream;
begin
  F := TFileStream.Create('C:\..', fmOpenRead or fmShareDenyNone);
  try
    // Do your job with the file stream
  finally
    F.Free;
  end;
end;

Creating the interfaced object and passing it as interface

type
  TMultiService = class(TInterfacedObject, IService1, IService2)
  private
    ...
  end;

  TSlave = class(..., ISlave)
  public
    constrctor Create(A1: IService1; A2: IService2);
  end;

var
  M: TMultiService;
  S: ISlave;
begin
  M := TMulti.Create;
  try
    S := TSlave.Create(M, M);
    M := nil;
    { Do any other business there }
    ...
  except
    M.Free;
    raise;
  end;
end;

In this case once you constructed the TInterfacedObject and assigned it to the variable of object type (TMultiService) it will be not disposed automatically. That’s why you should apply standard try/finally pattern there. But there is one thing to remember. Once you passed the variable of object type (in our case M) to a variable or parameter of interface type (IService1, IService2) the lifetime of the instance from that moment is managed by the reference counting.

Notes:

  • Avoid using the CONST prefix for the interface type parameters if your method is going to accept direct constructors of the interfaced objects like “Method(TService.Create)” (as when you write TService.Create the reference is not set, Delphi just constructs the TInterfacedObject with FRefCount = 0. And now when you pass it to the “const” parameter and it is never used inside of the routine you will get the memory leak as the reference counting did not took place! And this is not a bug – it’s the way how it works and should).
  • Once the instance is reference counted never call Free on the instance;
  • Protect passing the object-type variable to interface with try/finally blocks;
  • You do not need “M := nil;” if you do not do anything after passing the object-type variable;
  • You do not need to apply this mechanism in standard cases when you can just pass TService1.Create and TService2.Create (assuming they implement related interfaces) as a parameters – as this will be absolutely safe:
    • If the exception happens in the constructor call the instance is released automatically anyway;
    • Once the constructor succeeded the value is already reference counted as the parameter types are interfaces.

Thank you for reading this post. I hope you learned something new and this information was of some use.

23 comments

  1. I don’t think your example of adding an object to a collection is right. Rather than using try … except you should use try … finally like this:

    X := TCar.Create;
    try
    // Do any business needed before adding to the collection
    L.Add(X);
    X := nil;
    finally
    X.Free;
    end;
    end;

    Like

    1. It is right because:
      1) X := TCar.Create; //you create the object
      try
      // Do any business needed before adding to the collection
      L.Add(X); //you add it to the collection
      except
      X.Free; //you release it as it was not added to the list
      raise; //reraise the exception
      end;

      But your option is also correct and makes the same amount of lines 🙂
      You write X := nil; and I write raise 🙂

      Like

    2. Added := False;
      X := TCar.Create;
      try
      // Do work.
      L.Add(X);
      Added := True;
      finally
      if Added then
      X := nil
      else
      X.Free;
      end;

      Like

      1. Redundant variable: When I added Added (heh, heh) I was trying to avoid calling X.Free. I thought that would cause an error but you later pointed out that it does not cause an error.

        Also, I added Added (couldn’t resist doing it again) for Thomas Mueller since he seems to want to set X to nil (which I agree with).

        Liked by 1 person

  2. Good article. I like most of it. I would change “Creating multiple objects” like this:

    x1 := TObject.Create;
    try
    x2 := TObject.Create;
    try
    x3 := TObject.Create;
    try
    // Do work.
    finally
    x3.Free;
    end;
    finally
    x2.Free;
    end;
    finally
    x1.Free;
    end;

    In your example a raised exception before all 3 objects are created will cause an access violation when one of the uncreated objects has its Free method called (unless there is some Free method compiler magic).

    Like

    1. You don’t need to. There is no magic. I set the object to nil in the beginning. And when you call the Obj.Free when Obj = nil nothing happens. As free checks Self for nil (it was always like this and you can rely on this feature).

      Liked by 1 person

      1. You confirm what I suspected: Calling Obj.Free when Obj = nil does not cause an access violation. I call this compiler magic.

        Even if it is correct I still feel calling x3.Free and x2.Free when they were not created (because an exception was raised when creating x1) is bad.

        Liked by 1 person

  3. Creating the interfaced object and passing it as interface:

    M := TMulti.Create;
    try
    S := TSlave.Create(M, M);
    except
    M.Free;
    raise;
    end;
    M := nil;
    { Do any other business there }

    Is this better?

    Like

    1. Once we change the declaration from const to standard there is no memory leak. Good point from you!
      procedure UseService(Service1: IService1); does not leak. It looks like there is less reason to use “const” prefix for interface parameters!

      Like

  4. Ehhhh. I still don’t have solid enough reasons why using custom exceptions is as important and better than using the generic Exception. The point of using custom exceptions looks interesting but it’s irrelevant to my development experience and impractical for me.

    First I would highlight that I’m the application programmer and normally I don’t create libs at my daily routines.

    The case that a caller catches and analyses an exception which was generated in my code is obvious, but my experience tells me that this is the very, very rare case. In 99% of cases what the caller should/can do is to release resources as you’ve wrote and pass the handling further. Probably this is because of the fact that I don’t check business rules with the exceptions. In other words, in my practice the exceptions usually point to code mistakes or data mistakes. Developers should be notified of this and find out the reason. How do the custom custom exceptions help with it?

    Systems like madExcept fasten exception to context much better than the custom exceptions, and I think that in general systems that provide stack trace have to be used in any commercial project.
    So for me the only case of using custom exceptions is I want to add to an exception message particular parameters that may help me with determining the reason for the exeption.

    If you don’t want or don’t have a possibility of using madExcept, eurekalog, the custom exceptions doesn’t help with handling bugs anyway as a user sees a message only, not a class. If you rely on loggin exception you may log a context with the same effort.

    I don’t say that custom exceptions are useless, I just can’t understand the practice of writing dedicated exception classes for any context.

    Like

      1. Well I read briefly the Exceptions and Exception Handling artice. Can’t say it answers my question. However I would recommend everybody reading Code Craft: The Practice of Writing Excellent Code of Pete Goodliffe as well.

        which contains articles that are extending that discussion (it’s not about a code design, it’s rather about a philosophy and common sense)

        Like

    1. I think you are correct in that exception class hierarchy is most useful for components and libraries. But when you are writing any application of significance you should be writing your own business specific libraries – that you can reuse in different modules, windows and applications. And then you do have the need to differentiate between Exception and EMyBusinnessRules or similar.

      Like

      1. As I’ve mentioned I don’t use exceptions while checking the business rules. I rather prefer using function return values and out/var params. Meanwhile еру exceptions in my world are for code/data detected mistakes. I don’t insist that this is the only good approach (again, you can refer to Code Craft: The Practice of Writing Excellent Code there is really good discussion on this subject). But I’ve just read the Exceptions and Exception Handling artice from the book Zigmund has been suggesting me https://pascal.today/2016/08/07/coding-in-delphi/. There is an article about trapping the exceptions:

        Don’t Generically Trap Exceptions
        Sometimes I see code that looks like this:
        try
        SomeCodeThatMightCauseAProblem
        except
        on E: Exception do
        begin
        MessageDlg(E.Message, mtWarning, [mbOK], 0);
        end;
        end;

        Can you swear on the Bible that you don’t do that? 🙂

        Like

  5. That’s a good article. However I want to point you out that “Overusing the try statements” does not matter in x64 … Since x64 uses “structured exception handling” model. So when writing :
    for i := 1 to 10 do
    try
    Write(i);
    finally
    end;
    The compiler generates the same code when writing:
    for i := 1 to 10 do
    Write(i);

    Liked by 2 people

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