This post is a continuation from our Clean Code series. You can find the previous post here.

You probably have already heard about the Chernobyl disaster, right? If not, HBO made a really great mini series regarding it. Anyway, what I mean is, that’s a very good example of how to NOT handle problems. Caught errors were not handled accordingly, interested parties were not notified about it, the process wasn’t terminated when it should, and so on. Maybe your code won’t be responsible for a nuclear disaster, but it’s always important to do our job correctly and follow the exception handling best practices.

Solving the issue

Let’s start with some code. Previously we were developing a top tier robot, and now it seems that a ticket has been assigned to us regarding our robot not booting up properly. We’ve managed to track it to this following piece of code:

public void StartRobot()
{
    var flightModule = GetFlightModule();
    if(flightModule == null)
    {
        try
        {
            InitializeWheelsSpinningSystem();
        }
        catch (Exception ex) { LogException(ex); }
    }
    else
    {
        try
        {
            OpenWings();
        }
        catch (Exception ex) { LogException(ex); }
    }
    try
    {
        InitializeDefenseModule();
    }
    catch (Exception ex) { LogException(ex); }
    try
    {
        InitializeTargetingSystem();
    }
    catch (Exception ex) { LogException(ex); }
    try
    {
        InitializeAttackModule();
    }
    catch (Exception ex) { LogException(ex); }
}

Horrible, right? Turns out that we’re pretty much relying on luck here, if any exception happens, we just log it and hope for the best. I mean, if our targeting system is faulty, how can we start our attack module? Would you like to have a robot by your side which can’t tell ally from enemy? Let’s start this with small steps:

public void StartRobot()
{
    try
    {
        var flightModule = GetFlightModule();
        if (flightModule == null)
        {
            InitializeWheelsSpinningSystem();
        }
        else
        {
            OpenWings();
        }
        InitializeDefenseModule();
        InitializeTargetingSystem();
        InitializeAttackModule();
    }
    catch (Exception ex)
    {
        LogException(ex);
    }
}

It’s still bad, but at least if we run into any problems, we will stop with our method and avoid collateral damage. But still, it’s generally a bad idea to handle all exceptions, the method should only handle what it’s supposed to (and what it can). There are different ways to do this, let’s explore some of them:

catch (Exception ex) when (ex.Message == "Defense module initialization error")
{
    LogException(ex);
}

This way, we will only handle exceptions with the expected message. This approach is better suited for when you have no control over the code which throws the exception, like NuGet package for example. Normally, we should always throw and catch specific exceptions, like this:

public class DefenseModuleInitializationException : Exception
{
    public DefenseModuleInitializationException(string message) : base(message)
    {
    }
}

When designing a custom exception, you must have some best practices in mind as well:

• Add the “Exception” suffix
• Inherit from the Exception class
• Add custom properties as needed (none in this case)

Our code should therefore look like the following:

public void StartRobot()
{
    try
    {
        var flightModule = GetFlightModule();
        if (flightModule == null)
        {
            InitializeWheelsSpinningSystem();
        }
        else
        {
            OpenWings();
        }
        InitializeDefenseModule();
        InitializeTargetingSystem();
        InitializeAttackModule();
    }
    catch (DefenseModuleInitializationException ex)
    {
        LogException(ex);
    }
}

I haven’t showed the implementation of each method because it’s not relevant, but let’s analyze what GetFlightModule does:

private IFlightModule GetFlightModule()
{
    if (!InternationalInformationsProvider.CountryAllowsRobotsToFly("Britania"))
        throw new Exception("Can't fly :(");
    try
    {
        var module = new FlightModule();
        return module;
    }
    catch (Exception ex)
    {
        return null;
    }
}

We should avoid throwing exceptions whenever possible, since they are costly. If we can handle the scenario with validations, we should go for it. In this case, for example, we could simply return null and let the caller method to handle it properly.

But talking about returning null, catching an exception only to return null is not the correct way to handle errors. Just because we might get a problem creating the FlightModule, doesn’t mean we don’t have any. If you check the StartRobot method again, it will believe that since there’s no FlightModule, it is a ground robot, probably leading to another error. The correct way to proceed here is to not handle it at all. It might sound strange, but if you can’t handle it correctly, don’t.

In the end, our method should be like this:

private IFlightModule GetFlightModule()
{
    if (!InternationalInformationsProvider.CountryAllowsRobotsToFly("Britania"))
        return null;
    var module = new FlightModule();
    return module;
}

There are some more changes that we could do to our code, for example, since our custom exceptions clearly indicate that it will only possibly be thrown inside the InitializeDefenseModule method, we could reduce our try/catch block to reduce the nesting a bit, like this:

public void StartRobot()
{
    var flightModule = GetFlightModule();
    if (flightModule == null)
    {
        InitializeWheelsSpinningSystem();
    }
    else
    {
        OpenWings();
    }
    try
    {
        InitializeDefenseModule();
    }
    catch (DefenseModuleInitializationException ex)
    {
        LogException(ex);
    }
    InitializeTargetingSystem();
    InitializeAttackModule();
}

Conclusion

After our refactoring, we can extract the following exception handling best practices:

• Be careful when catching all exceptions
• Put as much detail as you can in your exception
• Avoid returning null when catching the exception, only if makes sense
• Create custom exceptions when needed
• If possible, handle the condition instead of throwing exceptions
• Do not handle an exception if it’s not the method’s responsibility to do so

And when creating custom exceptions, we should:

• Inherit from the Exception class
• Add custom properties as needed
• Append the “Exception” suffix to the class name

---