Image by Jim Frazier
I’ve recently been working on a code-base which wasn’t designed with test-driven development methodologies, or with unit testing in mind. As I implemented unit-tests to this code, there were some modifications I had to do on the code.
Some modifications popped up more than others. There were a few things that are more common than others – Let’s look at what they were and how they affect the code and tests.
Creating new object instances in classes
This is by far the most common change I have encountered.
You have a class you need to test, but this class is creating another object it depends on inside itself, usually with the new keyword.
The reason this often appears in code that wasn’t written to be tested is that doesn’t necessarily reduce the quality much. In addition, it may often feel complex to pass every dependency in the constructor parameters.
The reason why this makes testing difficult is that the object used by the tested class is often something it interacts with. This is problematic, because this means your test is actually depending on the other object’s functionality to work – and this is a big issue especially if the other object does something like accessing the file system or cookies.
The way to fix this is simple, and I already hinted at it above: Remove the code which creates the object, and instead inject the object into the class from outside it.
Typically, the code looks something like this:
class SomeClass { public SomeClass() { } public doFoo() { var x = new AnotherClass(); x.doSomething(); } } |
This is a better version:
class SomeClass { private _x; public SomeClass(AnotherClass a) { this._x = a; } public doFoo() { this._x.doSomething(); } } |
In the second version of the code, instead of creating a new instance of AnotherClass in the code, we pass it to the constructor. This way we can easily submit it for a mock in testing.
This modification also tends to improve flexibility and reduce coupling. Since your class no longer depends on this specific implementation, you can give it any subclass of it. A dependency on a interface instead of a class would be even better.
Methods that do too much
The second most common thing I encountered was methods that do so much they become difficult to test.
Reasons why large methods are difficult to test is that they often have very many code execution paths (ifs, elses, etc.) or side-effects (they claim to do one thing but do many).
Methods with side-effects are usually also quite inflexible and difficult to reuse.
To make a big method testable, extract parts of it into new method(s).
You may have some code like this:
public getSomething() { var data = this._x.getSomething(); //process data foreach(var d in data) { //do something to data if(...) { } else if(...) { } else { } } //Get some other data based on this data var otherData = new array(); foreach(var d in data) { otherData = this._y.getStuff(d); otherData.someValue = d; } } |
The above code isn’t very complex yet (because it doesn’t really have any code in it because I didn’t have any code at hand with the problem).
However, you can probably see that it first gets some data, processes it and then gets some other data based on that.
It might be a good idea to return the first set of data from this method. You already get data and process it in this method.
public getSomething() { var data = this._x.getSomething(); //process data foreach(var d in data) { //do something to data if(...) { } else if(...) { } else { } } return data; } public getSomethingElse(data) { var otherData = new array(); foreach(var d in data) { otherData = this._y.getStuff(d); otherData.someValue = d; } return otherData; } |
In this second version we have two methods instead of one. You can immediately see that it’s much more obvious what the purprose of the methods is, even with the dummy names I’m using with the example code ;)
Not only does this make the methods testable because they are more self contained, this also improves readability, makes the code easier to understand and often easier to reuse, because it’s split into logical parts.
Classes that do too much
This is related to the previous case. You may sometimes encounter classes that do too much.
An excellent example of this is a class which processes data – say it sorts some users, and fetches their relatives and does some processing. But it doesn’t just do processing: It also does raw file I/O to load the users!
While that may not initially seem like a totally awful idea, it’s something to be avoided. A class is best to serve just one purprose, otherwise you’ll end up with low cohesion when you have huge classes that do too much. Another good reason to split up class responsibilities is that you improve flexibility.
Say you wanted to replace files with a database in the example class. It’s much easier to do if the class uses another object to load the data instead of doing it itself. You can simply replace the loader class with a new one which uses a database, without having to touch your processing code.
This improves your code by making classes easier to understand and easier to reuse.
Other things
These are just the most common things I encountered in the code base I was working with. Miško Hevery has written a good post on top 10 things that make code hard to test, which is worth reading.
What have you encountered that made code difficult to test?