When making changes to code that has no test coverage, a good first step is to write characterization tests to capture the behaviour that is already there. But how do we know where to write these tests? The sort of obvious answer is to write tests for any methods that we change, and if the code is simple enough, this will often be sufficient. In legacy code however, there is no telling how a change in one place might affect behaviour in another, seemingly unrelated place. Unless we have a test in place, me might never know about these effects.

In particularly tangled legacy code, a good approach is to think about the changes you are going to make, and then spend time digging into what that change will affect, and what the affected things will affect, and so on.

Reasoning About Effects

When we talk about effects in software, what we mean is the things that happen in response to some functional change. For example, when we change the 3 to a 4 in the following code:

int GetBalancePoint() {
    const int SCALE_FACTOR = 3;
    int result = startingLoad + (LOAD_FACTOR * residual * SCALE_FACTOR);
    foreach(Load load in loads) {
        result += load.getPointWeight() * SCALE_FACTOR;
    }
    return result;
}

it changes the result of the method when it is called. It could also have an effect on the results of methods that call this method as well, and so on, all the way to the system boundary. Despite this, many parts of the code won’t have different behaviour, as they don’t call GetBalancePoint directly or indirectly.

Let’s try an exercise in effect reasoning. Make a list of all of the things that can be changed after a CppClass object is created that would affect results returned by any of its methods.

public class CppClass {
    private String name;
    private List declarations;
 
    public CppClass(String name, List declarations) {
        this.name = name;
        this.declarations = declarations;
    }
 
    public int getDeclarationCount() {
        return declarations.size();
    }
 
    public String getName() {
        return name;
    }
 
    public Declaration getDeclaration(int index) {
        return ((Declaration)declarations.get(index));
    }
 
    public String getInterface(String interfaceName, int [] indices) {
        String result = "class " + interfaceName + " {\npublic:\n";
        for (int n = 0; n < indices.length; n++) {
            Declaration virtualFunction
                    = (Declaration)(declarations.get(indices[n]));
            result += "\t" + virtualFunction.asAbstract() + "\n";
        }
        result += "};\n";
        return result;
    }
}
  • Someone could add additional elements to the declarations list after passing it to the constructor. Because the list is held by reference, changes made to it can alter the results of getInterface , getDeclaration, and getDeclarationCount.
  • Someone can alter one of the objects held in the declaration list or replace one of its elements, affecting the same methods.

We can sketch this: declarationsgetDeclarationCount(). If declarations changes — say its size grows — getDeclarationCount() can return a different value.

Similarly for getDeclaration(int index): both declarations and the objects it holds → getDeclaration(int index). Its return value can change if declarations changes, or if the declarations inside it change.

The same reasoning applies to getInterface, and all of these can be bundled into one combined diagram. Feathers calls these effect sketches. The rule is simple: a separate bubble for each variable that can be affected and each method whose return value can change (they may live on the same object or different objects — it doesn’t matter), with an arrow drawn to everything whose runtime value can change because of it.

If we wanted to make a change to CppClass, in order to properly assess the effects of the change, we may need to look even further out to anywhere that the class is instantiated. Recall that the constructor took in an argument declarations that is mutable, and so any changes to the collection after it’s gone into the constructor may have unintended consequences.

Source

Working Effectively with Legacy Code, Michael Feathers — Chapter 11, I Need to Make a Change. What Methods Should I Test?