Aug 11 2008

Try…Catch For No Reason

Published by at 2:49 pm under .NET,Programming

This is an old article and the information contained within it may be out of date, not reflect my current views and/or contain broken links. If you feel this article is still valid and requires updating, you can use the contact form to let me know. However, I make no guarantee that it will get updated.

I’ve seen this time and time again and I’m sure just about every developer out there has seem the same sort of thing:

try {
    //many lines of code
catch (Exception ex) {
    throw new Exception("Something went wrong dude!");

This is probably the single most un-helpful piece of code a developer can write.  All you are doing is making you life and future developers lives harder when it comes to debugging.  The whole point of the try…catch block is for times when you know an exception may happen and it allows you to gracefully handle it without the whole system crashing to the ground.

So lets have a closer look at what’s wrong with this code:

Too many lines wrapped

You wrap code in a try…catch block the idea is to only wrap the code that is likely to throw an exception.  Whilst it isn’t impossible, it does make identifying the offending line of code harder to find.  This may sound like a trivial reason to complain but when you have to deal with this day after day it does get a little tedious.

Indiscriminate exception handling

By catching exceptions of type Exception you’re really saying to developers that come after you “I don’t care enough about this code to know what exceptions might happen.?  In many instances different exception types can be handled in different ways.  I’m betting that the default response of most developers that catch Exception will be to throw a cryptic message at the user and drop out of whatever the method was – probably without clean up resources either.

It is possible to have multiple catch blocks that catch different exception types if you think that there are multiple types of exceptions that might be thrown:

try {
    File.WriteAllText"(@"c:\test.txt", "Hello world!");
catch (FileNotFoundException ex) {
    // do something
catch (SecurityException ex) {
    // do something else

This means that you can handle different exceptions in different ways.  Also, any type of Exception that you don’t handle gets passed up the call stack.

The exception isn’t handled

In this particular instance the exception isn’t handled, so why on earth put the try…catch block there in the first place?!  If all you’re going to do is throw a new exception or re-throw the old exception without doing anything you’re better off leaving out the try…catch block.  It just adds overhead to instances when it runs through without exception.

Original exception is masked

Again, in this example a new exception is thrown.  If you must do this then at least include the original exception as the inner exception (the exception that cause the new exception to be thrown).  The Exception class even has a constructor to help with this:

public Exception( string message, Exception inner )

Alternatively, you can throw the original exception.

New exception is generalised

By throwing the base Exception class you are forcing developers that use your code to have to catch exceptions of type Exception – see above for why this can be bad.  If you can’t find a built-in exception class to use, and there are many, create your own.  Visual Studio 2008 has a code snippet for creating an exception class, all you have to do is fill-in the blanks.

In an ideal world…

… I’d like .Net to have a similar method to java of having to specify (use attributes) what exceptions a method throws or catches.  This allows you to easily find out what exceptions may originate in a method your code calls, and let other developers know what exceptions your code doesn’t handle.  It also allows for pretty good compile time exception checking.  It means extra code has to be written but I think it results in better code.

One response so far