Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Exception throws: encapsulate them or not?

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?

like image 383
Simon Avatar asked Jun 03 '10 16:06

Simon


4 Answers

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.

like image 51
James Alexander Avatar answered Nov 15 '22 20:11

James Alexander


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).

like image 34
David M Avatar answered Nov 15 '22 20:11

David M


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.

like image 43
alun Avatar answered Nov 15 '22 20:11

alun


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(...);
like image 36
Joe Avatar answered Nov 15 '22 22:11

Joe