A little bit about refactoring in TDD
Originally, this was going to be part of my article “Taking a broader view of what test-driven development is,” but I thought it was getting too long, so I’ve broken it up into smaller parts. This is the second of three parts.
The refactoring step gets shortchanged in many articles about test-driven development (TDD), including articles I’ve written. This is so much so that it’s led some to write articles with clickbait titles about the one thing no one ever told you about TDD.
People have told me about refactoring. Just not as much as they tell me about the other two steps in the TDD cycle.
For one thing, I suspect it was Kent Beck’s idea to stick refactoring into the TDD cycle. Much more importantly, however, refactoring is often unnecessary in toy examples, so the example refactoring feels contrived.
Although I’ve found the Fraction class very useful, it might feel like a toy example to some. I will now contrive a reason to do refactoring on the Fraction class that we were working on in the previous part.
If you were following along the previous part with your favorite Java IDE, you should have a Fraction class with equals() and toString() tested, over-ridden and proven to work according to the specifications I will now reiterate.
One of the things that should have been tested and proven is that toString() will present a fraction in lowest terms regardless of the parameters passed to the Fraction constructor.
Likewise, two Fraction objects that represent the same fraction should be recognized as equal regardless of whether or not the numerator and denominator parameters passed to the constructor were in lowest terms or not for either one of them.
Did you take my advice to have the Fraction constructor put the fraction in lowest terms? Almost certainly you did. But let’s say that you didn’t, for the sake of this contrived scenario.
Then, to make testEquals() pass, you had Fraction.equals() take care of putting the fractions in lowest terms, which means doing in equals() for the this fraction and the obj fraction, potentially several times, what should have happened just once each in their respective constructors.
In a real life situation you would most likely immediately realize that’s inefficient, or you might not even come across this situation just yet. But in this scenario to demonstrate refactoring, you are in this situation but you move on to trying to make testToString() pass.
Although 56/58 = 28/29, it’s quite clear that the text “56/58” is not the same as the text “28/29” (the test strips out spaces, so we don’t worry about those). So, as it stands now in this scenario, testToString() will fail.
Maybe you could have gotten to this point in real life, but you would probably now realize that the Fraction constructor is where the lowest terms operation needs to happen.
But in the scenario, you go ahead and duplicate in Fraction.toString() the lowest terms operation from Fraction.equals(). Now testToString() should pass.
Which means that now you’re supposed to review what you’ve got so far to look for opportunities for refactoring.
It is at this point in the scenario that you would realize what you realized a lot sooner in real life, that, although these inefficiencies have no noticeable effect on performance, they are rather inelegant and you can do better with just a small tweak.
The whole point of making numer and denom private and final is that other classes can’t mess with them. A textbook example of encapsulation.
The constructor validates those two fields and then for everything else you can rely on them being the way you need them to be, meaning in lowest terms, and maybe also with a positive denominator.
It is also possible that instead of comparing this.numer , obj.numer, this.denom and obj.denom to determine equality, you decided to use the fractions’ values in floating point precision. After all, 5/8 = 0.625, and 10/16 = 0.625 also.
That should work just fine as long as the numerators and denominators are small. But it’s not too difficult to think of worrisome examples of potential false positives, as these outputs from the Scala REPL clearly demonstrate:
scala> 1.0 / 9223372036854775807.0
res6: Double = 1.0842021724855044E-19scala> 1.0 / 9223372036854775806.0
res7: Double = 1.0842021724855044E-19scala> res13 == res14
res8: Boolean = true
If you have the Java REPL on your system, it would perhaps make more sense to use that instead. But since Java and Scala have the same underlying primitives in the Java Virtual Machine (JVM), these results from the Scala REPL should cause us concern for our Java program.
This suggests a test to add to our test suite:
@Test
public void testNotEqualsCloseFraction() {
Fraction numberA = new Fraction(1, Long.MAX_VALUE);
Fraction numberB = new Fraction(1, Long.MAX_VALUE - 1);
assertNotEquals(numberA, numberB);
}Or, if you prefer to be less granular, you can stick this test into testEquals(). Despite the caveats about floating point precision, decimal approximations can be useful. This motivates the two exercises that would have been at the end, but which I’ve broken off to a separate article I will publish later.
Before we get to those, we should wrap up on refactoring. If at this point it’s still not clear that the Fraction constructor should put the fraction in lowest terms, we should add getNumerator() and getDenominator(), with corresponding tests that expect lowest terms.
@Test
public void testGetNumerator() {
System.out.println("getNumerator");
Fraction fract = new Fraction(75, 100);
long expected = 3L;
long actual = fract.getNumerator();
assertEquals(expected, actual);
}
@Test
public void testGetDenominator() {
System.out.println("getDenominator");
Fraction fract = new Fraction(75, 100);
long expected = 4L;
long actual = fract.getDenominator();
assertEquals(expected, actual);
}Again, this is a very contrived example of a need for refactoring. In real life, these tests would fail the first time on account of a stub that always returns a likely wrong value like 0 (and 0 is definitely the wrong number to return for every call to getDenominator()).
And then testGetNumerator() and testGetDenominator() would pass after being rewritten as bona fide getters. TDD makes examples like this so simple that hardly any refactoring is needed, if at all.
Hopefully, though, this contrived example gets across the point that refactoring is about streamlining. Refactoring can never take the place of a pertinent test, and writing a test can’t take the place of refactoring.
That includes timeout tests. Design inefficiencies that can be refactored away don’t hurt performance any more than anything else the JVM or the underlying operating system might have to do at any given moment.
So it would be difficult to know how much of a test duration is due to redundancies you can do anything about and how much is due to what else is going on in the JVM or the operating system.
With good object-oriented design, refactoring can be almost completely avoided, especially in the simpler designs of toy examples.
Still, it’s a good idea to really look for refactoring opportunities each time a previously failing test becomes a passing test.