We all know that catching Exception is a bad idea - for all sorts of reasons.

Nick Guerrera has written a good series of blog posts ( Part 1, 2, 3 ) talking about the dangers of attempting to handle any (every) kind of exception and why FxCop warns against code that does this.

The crux of Nick’s argument is that there’s no way of writing a generic handler that does the right thing to handle every case. He’s right. Code that handles an InvalidOperationException must be different from code that handles an SmtpException.

Mustn’t it.

Except that I’ve been writing a lot of code like this:

public void Warble(string name)
{
    try
    {
        // Complex processing goes here
        ...
    }
    catch(Exception ex)
    {
        const string Template = "While Warbling for {0}";
        string message    
            = string.Format(
                Template,
                name);
        throw new InvalidOperationException(message, ex);
    }
}

The point of this code is to add context to the original error. Whether the original exception is an ArgumentNullException, an InvalidOperationException or something else, the new exception gives more information about what was happening when the problem occurred.

Importantly, the information represented by the original exception isn’t lost - as the original is retained as the InnerException within the new exception.

Of course, FxCop complains about this code, as it’s a technical violation of the rule “Do Not Catch General Exception Types”.

But, is this actually bad code? What do you think? Why?

Comments

blog comments powered by Disqus