Design Pattern for Error Handling

Question

I've ran into this problem a few times on various projects, and I've wondered if there's a better solution than the one I normally end up using.

Say we have a series of methods that need to execute, and we want to know if something goes wrong within one of the methods and break out gracefully (potentially undo-ing any previous changes...), I typically do the following (pseudo C# because it's what I'm most familiar with):

private bool SomeMethod()
{
    bool success = true;
    string errorMessage = null;
    success = TestPartA(ref errorMessage);
    if (success)
    {
        success = TestPartB(ref errorMessage);
    }
    if (success)
    {
        success = TestPartC(ref errorMessage);
    }
    if (success)
    {
        success = TestPartD(ref errorMessage);
    }
        //... some further tests: display the error message somehow, then:
        return success;
}

private bool TestPartA(ref string errorMessage)
{
    // Do some testing...
    if (somethingBadHappens)
    {
       errorMessage = "The error that happens";
       return false;
    }
    return true;
}

I just wondered (and this is my question) if there's a better methodology for coping with this kind of thing. I seem to end up writing a lot of if statements for something that seems like it should be slicker.

I've been suggested having a loop over a set of delegate functions, but I'd be worried that would be over-engineering the solution, unless there's a clean way to do it.

Answer 1

I think you should probably be using exceptions. Note you should generally only be catching exceptions at the "top level" in your application.

private void TopLevelMethod()
{
    try
    {
        SomeMethod();
    }
    catch (Exception ex)
    {
        // Log/report exception/display to user etc.
    }
}

private void SomeMethod()
{
    TestPartA();
    TestPartB();
    TestPartC();
    TestPartD();
}

private void TestPartA()
{
    // Do some testing...
    try
    {
        if (somethingBadHappens)
        {
            throw new Exception("The error that happens");
        }
    }
    catch (Exception)
    {
        // Cleanup here. If no cleanup is possible, 
        // do not catch the exception here, i.e., 
        // try...catch would not be necessary in this method.

        // Re-throw the original exception.
        throw;
    }
}

private void TestPartB()
{
    // No need for try...catch because we can't do any cleanup for this method.
    if (somethingBadHappens)
    {
        throw new Exception("The error that happens");
    }
}

I have used the built-in System.Exception class in my example; you can create your own derived exception classes, or use the built-in ones derived from System.Exception.