Note of CLEAN CODE chapter 7 - Error Handling

Error handling is important, but if it obscures logic, it’s wrong

Use Exceptions rather than return codes

In languages which do not support exceptions, you either set an error flag or return an error code that the caller could check like this

DeviceHandle handle = getHandle(DEV);
if(handle != DeviceHandle.INVALID){

}else{
    logger.log(errorMessage);
}

It clutters the caller. The caller must check for erros immediately after the call. But it is easy to froget, so it is better to throw an exception

public void sendShutDown(){
    try{
        tryShutDown();
    } catch(DeviceHandleException e){
        logger.log(e);
    }
}
    

private tryShutDown() throws DeviceHandleException {
    DeviceHandle handle = getHandle(DEV);
}

private DeviceHandle getHandle(DeviceId id){
    throw new DeviceHandleException(errorMessage);
}

Notice how much clearer it is. Two concerns that were tangled and now the device shut down and error handling are divided.

Write Your try-catch-finally Statement First

When you execute code in the try portion, you are stating that execution can abort at any point and the resume at the catch.

In this way, try blocks are like trasactions. Your catch has to leave your program in a consistent state, no matter what happens in the try

First thing first, we start with a unit test that shows that we’ll get an exception when the file does not exist

@Test(expected = StorageException.class)
public void testRetrive(){
    retriveSection("invalid file");
}

The test drives us to create the stub

public String retrieveSection(String sectionName){
    // dummy return until we have a real implemention
    return new String();
}

Our test fails because it does not throw an exception. Next it attempts to throw one

public String retrieveSection(String sectionName){
    try{
        FileInputStream stream = new FileInputStream(sectionName);
        stream.close();
    }catche(Exception e){
        throw new StorageException("retrive error ", e);
    }
    return new String();
}

Finally, narrow the type of exception we catch to match from FileInputStream constructor

public String retrieveSection(String sectionName){
    try{
        FileInputStream stream = new FileInputStream(sectionName);
        stream.close();
    }catche(FileNotFoundException e){
        throw new StorageException("retrive error ", e);
    }
    return new String();
}

Use Unchecked Exception

Checked excpetions (like IOExcpetion) are not necessary for the robust of productions.

Price of checked exceptions are Open/Closed Principle violation

If you throw a checked exception from a mathod and the catch is three levels above, you must declare that exception (add throws) in the signature of between you and the catch. A change in low level force signature changes on many higher levels.

This breaks the encapsulation, resulting in a cascade of changes that their way from the lowest levels of the software to the highest.

Provide Context with Exceptions

To determine the source and location of an error, you need to create informative error messages and pass them along with your exceptions.

Define Exception Classes in Terms of a Caller’s Needs

When we define exception classes in an application, our most important concern should be how they are caught.

ACEMport port = new ACMEport(12);
try{
    port.open();
} catch(DeviceResponseException e){
    reportPortError(e);
    logger.log("DeviceResponseException ", e);
} catch(GMXError e){
    reportPortError(e);
    logger.log("GMXError ", e);
} catch(OtherException e){
    reportPortError(e);
    logger.log("Others ", e);
} finally {
    ...
}

There is a lot of duplication. In most situations, the work is relatively standard regardless of the cause, record an error and make sure we can proceed

If all we do are similar regardless of the exception, we an simply fi them by wrapping the API we are calling and make sure it returns a common exception type

LocalPort port = new LocalPort(12);
try{
    port.open();
} catch(PortOpenError e){
    reportPortError(e);
    logger.log("DeviceResponseException ", e);
} finally {
    ...
}

and your localPort is a simple wrapper that catch and translates exceptions thrown by ACMPort

public class LocalPort {
    private ACMPort innerPort;
    public LocalPort(int portNumber){
        innerPort = new ACMEport(12);
    }

    public static void open(){
        try{
            port.open();
        } catch(DeviceResponseException e){
            throw new PortOpenError(e);
        } catch(GMXError e){
            throw new PortOpenError(e);
        } finally {
            ...
        }
    };
};

• When you wrap a third-party API, you minimize your dependencies upon it
• you are not tied to a particular vendor’s API design choices, you can define your own ones and write it more clearly

Define the Normal Flow

You define a handler above your code so that you can deal with any aborted computation. Sometimes you don’t want to abort

try{
    MealExpense expense = expenseDAO.getMeals(employee.getId());
    total += expense.getTotal();
}catch(MealExpenseNotFound e){
    total += getMealPerDiem();
}

The exception clutters the logic, it would be better if we did not have to deal with the special case

MealExpense expense = expenseDAO.getMeals(employee.getId());
total += expense.getTotal();

public class PerDiemMealExpense implements MealExpense{
    public getTotal(){
        //return meal expense per diem
    }
}

Don’t Return Null

If you work in code like this, it won’t be bad for you, but it is bad.

if(item != null){
    Registery reg = getRegister();
    if(reg != null){
        ...
    }
}

If we don’t check null, we will get a NullPointerException at a runtime, or someone catches it at a relative top level. Either of them is bad.

The problem might be “There is no null check in if statement”, but actually it has too many!

For example

List<Employee> employees = getEmployee();
if(employees != null){
    for(Employee employee : employees){
        ...
    }
}

Does it have to return a null? Why not write it as

List<Employee> employees = getEmployee();
for(Employee employee : employees){
    ...
}

public List<Employee> getEmployee(){
    if(/** no elements */){
        return Collections.emptyList();
    }
}

Don’t Pass Null

If you pass a null, you need to define a handler for InvalidArgumentException or assert, but it does not solve any problem!