There are several advantages and disadvantages of Test Driven Development. In this post, I have no intention of repeating any of these, instead, I’d like to show a way to use TDD effectively while changing legacy code.
The Problem
Have a look at this legacy code:
package com.zsoltfabok.dojo.legacy.stepback;
public class Comparator {
public boolean same(String string) {
char[] data = string.toCharArray();
char[] first = null;
char[] second = null;
boolean value = false;
for (int i = 0; i < data.length; i++)
if (data[i] == ' ') {
first = new char[i];
second = new char[data.length - i - 1];
System.arraycopy(data, 0, first, 0, i);
System.arraycopy(data, i + 1, second, 0, data.length - i - 1);
if (first.length == second.length)
for (int j = 0; j < first.length; j++)
if (first[j] == second[j])
value = true;
else {
value = false;
break;
}
else
return false;
break;
}
return value;
}
}This code does not do much; it accepts two words separated with a space, and returns true/false depending on whether the words are equal or not. This works quite well, but imagine that we are asked to enhance it, so that the comparison is not case sensitive any more. As a first step, have some test cases in order to preserve the original behaviour:
public class ComparatorTest {
private Comparator comparator = new Comparator();
@Test
public void shouldReturnTrueForTheSameWords() {
assertTrue(comparator.same("word word"));
}
@Test
public void shouldReturnFalseForDifferentWords() {
assertFalse(comparator.same("ward word"));
}
}There can be more, but for now, they are enough (check the screencast and the source repository for the whole test suite). TDD and the green bar, among other things, make you/me confident that the upcoming changes won’t affect the already implemented behaviour. This is the most important thing in it for me. Now back to the legacy code. Have the test case, which will drive the change:
@Test
public void shouldReturnTrueForTheSameButDifferentlyCasedWords() {
assertTrue(comparator.same("wORd word"));
}After having this test case on board, which turns the bar red, the following will happen:
-
introducing the change (implementation of the non case sensitive comparison)
-
massive refactoring (nobody doubts it I assume)
Such a huge work may take “hours” even for that small piece of code above (I admit that in this case it won’t take hours). Obviously, you cannot commit it, cannot ship it to the customer, you cannot show it to others besides your pair, etc. until the change is not done.
Ideas for Solving the Problem
The TDD mantra says red-green-refactor, red-green-refactor,… but is does not say anything about the time spent in each phase, a factor that I consider very important. For example, staying too long in green and refactor means less or no progress, staying too long in red means losing control over the code, causing quality and feature degradation. In order to avoid these cases I’ll take two ideas:
-
very small and focused steps (Kent Beck, Extreme Programming Explained 2nd Edition)
-
spend limited time in red (Joshua Kerievsky, The Limited Red Society)
The small steps help to make progress in green, and the limited time in red helps to stay on the right track. If you realize after some minutes that you are doing changing and refactoring in one step, you may do your last step back, and perform only one of them. I prefer doing the refactoring first, because the green code is shippable, shareable, and nevertheless I feel more secure, because the test cases prevent me from introducing bugs in the existing functionality. So a third item to the list:
- take a step back: do the refactoring first and the change afterwards
One can argue that only the change has value to the customer so that shall be done first and refactoring afterwards. In my point of view, this statement highly depends on the definition of value. If the customer got the code, the value wouldn’t only mean the functionality but quality as well. However, I admit that there are cases when the change comes first and refactoring afterwards.
Ignoring Test Cases
One way of stepping back is to ignore the new test case. Do not delete it, just @Ignore it:
@Ignore("Introduces new functionality, but I must refactor first")
@Test
public void shouldReturnTrueForTheSameButDifferentlyCasedWords() {
assertTrue(comparator.same("wORd word"));
}An ignored test case is similar to a TODO note, but a bit more effective. You know that the test case is there, because Eclipse shows the number of ignored test cases. Now everything is green again, you spent less time in red, but need a smaller step. Do the refactoring now. The first test cases make sure that nothing is going to be broken, and with wise refactoring, the introduction of the change will be fast and easy.
When the refactoring is done, remove the @Ignore tag, and introduce the change. You can apply this technique as many times as you want.
Don’t get me wrong, there is nothing wrong with such test cases, on the contrary: they are very good test cases. They not only drive the implementation, but also point out that the code base needs refactoring.
Write Test Cases With Temporarily Wrong Assertion
The example above introduces something new, but sometimes an already existing functionality has to be fixed. Imagine that the customer complains that the method does not work if the input contains trailing spaces. Now, the test case looks like this:
@Test
public void shouldReturnTrueInCaseOfSameWordsButWithTrailingSpaces() {
assertTrue(comparator.same("word word "));
}Another way of stepping back is to change the assertion here like this:
@Test
public void shouldReturnTrueInCaseOfSameWordsButWithTrailingSpaces() {
assertFalse(comparator.same("word word "));
}This test case provides more safety, because this is how the legacy code works, isn’t it? As soon as the refactoring is done, do not forget to change back to the original test case.
Example
In the following screencast I demonstrate how to use these techniques while working with the example legacy code.
The source code of the example is available on github.
comments powered by Disqus