Error handling

Next part of my notes from Clean Code: A Handbook of Agile Software Craftsmanship, by Robert C. Martin.

Don’t return and don’t pass null

Returning null is creating additional work and problems for methods that consume our function. We do not want methods to consist mainly of lines checking for nulls.

        public void RegisterItem(Item item)
        {
            if (item != null)
            {
                ItemRegistry registry = peristentStore.GetItemRegistry();
                if (registry != null)
                {
                    //...
                }
            }
        }

In the code above we have too much of checking for null values. Still peristentStore object was not checked and can cause problems.

Instead of returning null value, it is better to throw an exception or return a special case object like an empty list. If GetEmployees method in the example below returned an empty List<Employee>, we could get rid of if (employees != null) and go ahead with the loop straight away.

            List[Employee] employees = GetEmployees();
            if (employees != null)
            {
                foreach (Employee e in employees)
                {
                    totalPay += e.GetPay();
                }
            }

It is also a good idea not to pass null values as arguments.

Define exception classes in terms of a caller’s needs

This is an example of poor exception classification.We have a try-catch for calling an external library. It catches all the exceptions the Open method can throw.

 ACMEPort port = new ACMEPort();
            try
            {
                port.Open();
            }
            catch (DeviceResponseException e)
            {
                ReportPortError(e);
                logger.Log("Device response exception", e);
            }
            catch (ATM1212UnlockedException e)
            {
                ReportPortError(e);
                logger.Log("Unlocked exception", e);
            }
            catch (GMXError e)
            {
                ReportPortError(e);
                logger.Log("Device response exception", e);
            }
            finally
            {
                //...
            }

We get a lot of duplicates. At the same time, all the operations are standard logging error methods that do pretty much the same operations.

Instead, we can build LocalPort class covering ACMEPort class that will transform all its exceptions into PortDeviceFailure.

    public class LocalPort
    {
        private ACMEPort innerPort;

        public LocalPort()
        {
            innerPort = new ACMEPort();
        }

        public void Open()
        {
            try
            {
                innerPort.Open();
            }
            catch (DeviceResponseException e)
            {
                throw new PortDeviceFailure(e);
            }
            catch (ATM1212UnlockedException e)
            {
                throw new PortDeviceFailure(e);
            }
            catch (GMXError e)
            {
                throw new PortDeviceFailure(e);
            }
        }
    }

This way we are not dependent on the external API exceptions any more. It will be also easier to change the external library in the future.

References

“Clean Code: A Handbook of Agile Software Craftsmanship” – Robert C. Martin