Nov 18 2005

“Handling” Exceptions

Published by sethyates at 6:14 am under Development, Quality

I really love code reviews where I tell developers that they need to better plan for and handle exceptions, and in the followup code review, they present the following:

try
{
// do some dangerous stuff that may throw an exception.
}
catch (Exception e)
{
throw (e);
}

My usual reaction is to grit my teeth, administer an icy stare and ask a calm, measured question: “And what exactly, are you aiming to accomplish here? (besides, of course, losing your stack trace and the real source of the exception)”.

The problems I have with this “pattern” are many:

  1. You lose your stack trace when you call throw(e);. The stack trace now points directly to this line, so you no longer know the real culprit.
  2. There’s no real “handling” going on here, now is there? What’s the code saying when you read it? “Hey! I can handle ‘Exception’. Oops, no I can’t, gonna have to throw this one on up the chain.”.
  3. Almost always, too generic of an exception is being caught. Come on, tell me that you know how to handle an OutOfMemoryException (or any other exception that derives from the Exception class…that’s almost* every exception) in this case. Really?

When I’ve finished with the code review, the above code is usually refactored into one of two forms:

  1. // do some dangerous stuff that may throw an exception.

    This is most often my favourite refactoring. It accomplishes the same effect as the above code…passing the buck.
  2. try
    {
    // do some dangerous stuff that may throw an exception.
    }
    catch (InvalidOperationException e)
    {
    throw MyFacadeException("There was a problem doing the dangerous thing.", e);
    }

    This is the form of the refactor I prefer if there is truly a need to handle the exception and do something with it (in this case throwing a new exception, wrapping the thrown exception).

There are plenty of exception management best practices, but I typically use the following simple rules:

  1. Avoid an exception if you can (e.g., check for a zero denominator instead of just dividing by zero; check if a file exists rather than just trying to open it).
  2. If you can’t avoid an exception, catch the most specific exception you can (e.g., catch (ArgumentOutOfRangeException) instead of catch (Exception)…) and never, ever catch without a filter.
  3. Only catch the exception if you know what to do with it. If you don’t know what to do with an exception, don’t catch it.
  4. If you’re going to rethrow an exception, do it properly so the stack trace is maintained: throw; instead of throw(e);.
  5. Valid things to do in a catch are:
    • Do nothing. Swallow the exception if you expect it to happen, and rely on the fact that things weren’t done in the try clause to signal the exceptional case.
    • Set some flags, or reset some local variables. Let these be the signal to the exceptional case.
    • Throw a new exception, wrapping it around the thrown exception and providing either a facade exception type or providing better exception information (e.g., the message may be more specific about what was intended rather than the actual nuts-and-bolts error as in “Could not create the lock file required for exclusive write access” would be better than “Access denied”).
    • Rethrow the thrown exception (using throw;, not throw (e);). This is useful if you need to handle some more generic exceptions, but you want to bubble some more specific exceptions.
    • Log a message and return (if you must). Least preferred, as it may be hiding some exceptions from you.

* Some obscure runtime exceptions actually don’t inherit from Exception, and these get caught in the following code:

try
{
...
}
catch
{
...
}

exceptions, code reviews, quality, wtf

Related posts

Comments are closed at this time.

Trackback URI |