I thought I’d write a brief post on some exception handling guidelines after noticing in the code at work that there are a lot of try catch blocks that simply catch the base Exception class, among other bad exception handling practises.

A common mistake that a lot of people make is that they use try catch blocks to handle the flow of code, for example one might do the following:

try
{
    FileStream fileStream = File.Open(@"c:\file.txt", FileMode.Open);
}
catch
{
    MessageBox.Show("File does not exist!");
}

This is bad, don’t do it! You should rather be doing this:

if (File.Exists(@"c:\file.txt"))
{
    FileStream fileStream = File.Open(@"c:\file.txt", FileMode.Open);
}
else
{
    MessageBox.Show("File does not exist!");
}

Now of course you can put additional error handling around the File.Open statement above in case the file cannot be opened because there is a lock etc.

The next common bad practise is having an empty catch statement or catching the base Exception class. Having an empty catch statement (such as in the example above) is bad because there isn’t really much you can do in the catch block without the exception object, people often do this if they want to suppress an exception, which sometime you do want to do (more about that later). And why is catching the base Exception class bad? Well catching the base Exception class isn’t bad, catching only the base exception class show be avoided. If you’re only catching the base Exception class then you probably don’t have a good idea of what your code in the try block is doing, you should at least know some of the exception types that can be raised by your code, and if you don’t know you’re just being lazy! You simply need to hover over the method to see the exceptions that can be raised (assuming of course that the developer included the exceptions in the XML documentation).

So for example, we could modify our example above to the following:

if (File.Exists(@"c:\file.txt"))
{
    try
    {
        FileStream fileStream = File.Open(@"c:\file.txt", FileMode.Open);
    }
    catch (UnauthorizedAccessException)
    {
        MessageBox.Show("You do not have sufficient access rights");
    }
}
else
{
    MessageBox.Show("File does not exist!");
}

As I’m sure you know you can specify more than one catch block so you can handle certain exceptions differently. So the example above could be expanded as follows:

if (File.Exists(@"c:\file.txt"))
{
    try
    {
        FileStream fileStream = File.Open(@"c:\file.txt", FileMode.Open);
    }
    catch (UnauthorizedAccessException)
    {
        MessageBox.Show("You do not have sufficient access rights");
    }
    catch (IOException ex)
    {
        MessageBox.Show("Error reading file, reported error: " + ex.Message);
    }
    catch (Exception ex)
    {
        MessageBox.Show("Unknown error occured, reported error: " + ex.Message);
    }
}
else
{
    MessageBox.Show("File does not exist!");
}

You’ll notice than the exception base class is last, you should always order your exceptions from the most derived type to the least derived type.

Now I’m sure I’ve said more than once that catching the base Exception class is bad, but don’t take that to mean you should never do it, it simply means if you can catch a more specific type then you should. Situations often arise when you don’t know what the specific exception types are, for example when integrating with a third party component, they may not publish the possible exceptions, you could of course always use .NET Reflector to find out

When catching an exception be sure to always do something, i.e. handle the exception, this could be logging the error to a database, the event log, or simply displaying a message to the user. If you need to release resources if an exception occurs then use the finally block (or use a using block), don’t release the resources in the catch block!

This obviously isn’t an exhaustive list of the things not to do when handling exceptions, there are many websites out there with guidelines for exception handling best practises, so there is no excuse not to Google and learn!