Friday, March 02, 2007

Exceptions You Should Always Break On

One of the great features for debugging in Visual Studio 2005 is the exceptions dialog. This can be accessed from the debug -> exceptions menu. If you don't have this menu item (some default settings picked during installation don't include this option) then you might need to 'customise' your tools bars to add it.

The exceptions dialog allows you to specify how the debugger should behave when an exception is thrown. The most common thing to do here is to specify 'always break on xxx' where xxx is a type or category of exceptions. This is similar to the VB6 'Break On All Errors' option, except that you can specify specific exception types to break on instead of all or nothing. Note : If you want the equivalent of VB6's 'Break On All Errors' option, simply tick the check box next to Common Language Runtime Exceptions.

I regularly use this feature when debugging, as often I know an error is occurring somewhere in the code when I perform a particular action, but one of my event handlers, or my global error handling is preventing me from finding the specific location. By saying to break on all errors, or the specific error I'm dealing with, Visual Studio will stop and break into the code, highlight the line that threw the error, as each error occurs. This lets me quickly and easily locate the source of the problem.

This had lead me to some new thinking on exceptions. I am coming to believe there are some exceptions that should never, ever occur. Of course, we all make mistakes, so that is to say there are some exceptions that should only occur when there is truly a bug.

So far I have only applied this thinking to one exception type (although it may apply to others) - System.NullReferenceException.

System.NullReferenceException is the same as the old VB6 'Object reference not set' error. It basically means you tried to use a property or method on a variable that has not been assigned an object value. Now let's think about the times this might happen;

  1. You simply didn't foresee this variable ever being null (shame on you), and this is a bug. In this case, fix your code so it checks whether the variable is null or not. If it is, and that should be exception, see point 4.
  2. Someone passed you a null argument and you don't accept nulls - in which case you should check the argument before using it and throw the System.ArgumentNullException if it is null. Don't forget to provide the name of the argument.
  3. You're using an object you think might be null sometimes, and if you get the null reference exception you catch it and do something else. BAD BAD BAD - throwing exceptions is bad for performance, and unnecessary try/catch blocks look ugly. Instead, use an if to check if the value is null and do one thing, else do the other.
  4. You're using an object that shouldn't be null, but if it is the problem should throw an exception. Cool, fine, great... but don't throw System.NullReferenceException - it doesn't tell you anything ! Instead, throw a more meaningful exception, even if it means creating one yourself. Say you've attempted to find a plug-in object by name in a collection of loaded plug-ins and the result is a null reference, and that should be an error condition. How about creating and throwing a ConfigurationException instead ? At least this way you, the user, their administrator or their support agent has a better chance of figuring out the problem is a missing plug-in if the error occurs in a deployed system. Also, and possibly more importantly, code calling yours can catch ConfigurationException and confidently handle it if necessary, where as catching System.NullReferenceException is kind of ambiguous - did you get it because the add-in reference was null, or some other variable in the same routine ?

If you take this approach then you can permanently check the 'break on thrown' checkbox for System.NullReferenceException. What does this do for you ? It ensures that if you're debugging the exception occurs you will always know where it happened, and you will always know that it's a problem you need to fix. It also ensures you and your colleagues don't write sloppy code that throws too many exceptions (and catches them) causing poor performance.

So, what do you think ? Should System.NullReferenceException be banned from deliberate or expected exception lists ? Are there any other exception types you think belong here ? Let me know !

No comments:

Post a comment