Exception throws: encapsulate them or not?

Question

Hi there.

Once I read an MSDN article that encouraged the following programming paradigm (its not 100% true... see edit):

public class MyClass
{
    public void Method1()
    {
        NewCustomException();
    }

    public void Method2()
    {
        NewCustomException();
    }

    void NewCustomException()
    {
        throw new CustomException("Exception message");
    }
}

Do you think this paradigm makes sense? Wouldn't it be enough to store the exception message in a static const field and then pass it to the exception's constructor, instead of encapsulating the whole exception throw?

EDIT:

Use exception builder methods. It is common for a class to throw the same exception from different places in its implementation. To avoid excessive code, use helper methods that create the exception and return it.

I just noticed (see citation), that the article tells to return an exception:

public class MyClass
{
    public void Method1()
    {
        throw NewCustomException();
    }

    public void Method2()
    {
        throw NewCustomException();
    }

    CustomException NewCustomException()
    {
        return new CustomException("Exception message");
    }
}

What do you think about this?

Answer 1

My understanding is that passing an exception instance around is a faux pas if for no other reason than you lose the stack trace associated with the exception. Calling another method would change the stack trace and thereby make it effectively useless. I'd recommend at a minimum getting the stack trace off the exception and passing it as an argument to some helper if that's the road you're going to go down.

Answer 2

That's a refactor too far in my book. You have to go back up a line in the stack trace to see exactly where the problem occured. If your custom exception is always using the same message, put it in the CustomException class. If it's only the same within the code you've quoted, then yes, put it in a const field (you can't have static const - it's implicitly static).

Answer 3

I don't see the point of making a method that simply throws an exception. But, I do think trowing custom exceptions has value. If all of the exceptions you throw are children of a custom exception, it allows you to quickly see if the thrown exception is one you are accounting for or something you have not handled yet. Also, you can then catch MyBaseException and it is not as bad as catching Exception.

Answer 4

It is handy to do this if you don't know how you plan to handle exceptions, exactly. Do you want to just throw it? Or perhaps later you are going to log the exception somewhere then throw it? Or maybe pass some arguments (i.e. method name, etc.) that get bundled in with the exception?

In this case, creating a separate method that handles the exception situation is convenient when you want to change it.

I don't usually bother with this - instead, just figure out upfront how you are going to handle exceptions (i.e. what string information you are going to putin the message).

Answer 5

I would have a method that builds an Exception, rather than one that throws it. As in the sample below. I seem to remember seeing a Microsoft guideline that recommended this, but I can't remember where.

With this technique, if you want to change the exception type for any reason, you only need to do so in one place (e.g. a change from ConfigurationException to ConfigurationErrorsException when upgrading from .NET 1.x to .NET 2.0).

Also you respect the DRY principle by having a single copy of the code that builds the exception with its message and any other data included in the exception.

You obviously wouldn't do this in trivial cases (e.g. you wouldn't replace throw new ArgumentNullException("myParamName") by throw BuildArgumentNullException("myParamName"))

private static Exception BuildSomeException(... parameters with info to include in the exception ...)
{
    string message = String.Format(...);
    return new SomeException(message, ...);
}

...
throw BuildSomeException(...);

Answer 6

Another problem you get doing that is that there will be lots of places where you wont even be able to throw an exception because the compiler wont allow it. Consider these two methods added to your class:

    public string GetFoo1(bool bar)
    {
        if (bar)
            return "";
        else
            NewCustomException();
    }

    public string GetFoo2(bool bar)
    {
        if (bar)
            return "";
        else
            throw new CustomException("Exception message");
    }

GetFoo1 will not compile while GetFoo2 will.

Answer 7

I generally prefer to store exception messages as resources. That serves several purposes:

• If the requirement comes down to localize exception messages, it's a no-brainer.
• Exception messages tend to be more standardized across developers since it's extra work to create a new, but only slightly different message.
• If you ensure that messages are referenced by an identifier, and include the identifier with the exception when it's thrown, then tracing a message to the code that threw it is easier.

Downside is it does require (just) slightly more effort up front than hard-coding the messages.