“Tenso” – Coding while at 0% code coverage

Two of the most typical consultant’s job are trying to save a failing project or implement new features and fix bugs within a project which code is not as clear or as good as one wishes.

Uncle Bob mentions that if all checked in code is a little better than the code which was checked out, the average code quality will be higher than before. If we keep proceeding this way, the project’s overall quality will be much better after a couple weeks.

Those projects which does not contain such good quality code are the ones were we can learn a lot about baby-steps changes and code-improvement: you can improve anything and everything.

The same thing would apply with tests. If whenever we make a change to some piece of non-tested code, one writes its respective code, really soon we will have a better designed and testable software.

Paul Hammant once told me he used to love to face those projects and implement new features/refactor the project without breaking it, while introducing test cases – usually those projects do not contain any automated tests.

It takes a lot of guts to face those changes. Imagine a project which contains  no automated tests at all and you wish to change a functionality. If 100% code coverage is not enough, imagine 0%…

The first step would be to create an automated test which works for the functionality you are willing to change (the way it works right now). Your job is to get the test passing and it should probably take some time only to get it written, because the code was not designed for tests.

Let’s see an example of code written while not thinking about tests. The first class shows a thread local based code which allows one to access a 3rd-party system, i.e. a web service:


public class Connections {
private static final ThreadLocal<Resource> resource = new ThreadLocal<Resource>();
static { // ... startup code }
public static Resource getResource() {
return resource;
}
}


And now we show a common web service access using the above mentioned class:


public class Orders {
public static List<Order> active() {
List<Order> orders = Connections. getResource().list();
for(Iterator<Order> it = orders.iterator(); it.hasNext();) {
Order order = it.next();
if(order.wasCancelled()) it.remove();
}
return orders;
}
public static void add(Order order) {
return Connections. getResource().post(order);
}
}


Note that this code works just fine. A software could be running it for a while until someone asks to change the list criteria for all clients.

Lets suppose that the logic has changed, requiring that one should only be allowed to see the orders which were not cancelled and not yet delivered at the same time. You could simply implement the change in that Orders.all method or start implementing tests right away.

The process of creating an automated test for Order.all shows one of the typical problems while trying to create tests in projects which were not design for them: tight coupling between classes.

This tight coupling usually appears in two forms: instantiating specific types (i.e. new XXXX) or statically accessing members of another type. In both situations, a test case implies also accessing the other type.

The problem is, how to create a unit test case, without changing its code? Well, one could write some code which would actually access the 3rd party system, but that’s not our aim with this test. We just want to assure that the for (and including if) was written correctly.

This case can be refactored with the use of dependency injection but we need to be careful, our changes could break the functionality.

Although Orders and Connections are tightly coupled, we can introduce a new parameter within the Orders class. Let’s


public class Orders {
private final Resource resource;
public Orders(Resource resource) { this.resource = resource; }
public static List<Order> active() {
List<Order> orders = resource.list();
for(Iterator<Order> it = orders.iterator(); it.hasNext();) {
Order order = it.next();
if(order.wasCancelled()) it.remove();
}
return orders;
}
public static void add(Order order) {
return Connections. getResource().post(order);
}
}


Notice that I did not make any changes to the add method, because I am still not going to write any test case related to it.

In order to introduce the new parameter within the Orders() constructor, its a good idea to use an ide feature and make everyone who is calling its constructor to start invoking “new Orders(Connections.getResource())”…

This way, no functionality was changed at all, the project is compiling and working as before and we are ready to create our test case, which shall pass a mocked Resource to the constructor and allows us to run this unit test.

As soon as the test is passing, one is ready to make changes to the business logic contained within the list method itself.

This is one of the possible ways of dealing with legacy non-tested code in order to incrementaly built a well-enough-covered codebase.