A more realistic example of refactoring in TDD
The previous time I wrote about test-driven development (TDD), I stated that most refactoring scenarios in tutorials are contrived, and not really representative of refactoring in an actual program (meaning a program meant for a purpose other than to give examples in a tutorial).
This is in part because the examples of tutorials by necessity need to be easier to program, and so they are less likely to need refactoring.
Authors of tutorials could use examples drawn from their own actual programs, but then there is the problem that they might have to explain things that distract from refactoring.
For example, if I give an example of refactoring from my Algebraic Integer Calculator project, which I’m writing mostly in Java, I would have to explain algebraic integers and complex numbers.
They’re not difficult mathematical concepts, but they are unfamiliar and take time to explain. A couple of days ago, I found myself doing some actual refactoring and I thought it would be an excellent example, except for the simple but unfamiliar mathematical concepts.
And then I thought maybe I could simplify things by explaining complex numbers as Cartesian coordinates.
For example, the complex number −3 + 7i can be thought of as the Cartesian coordinate pair (x, y), where x = −3 and y = 7. We can call x the “real part” and y the “imaginary part.”
It’s an oversimplification, but hopefully one that helps people understand the example I’m going to present here.
To further simplify things, in this article I will limit discussion to Gaussian integers, which are numbers of the form a + bi, with a and b both being arbitrary integers of the kind we’re familiar with.
So instead of the class ImaginaryQuadraticInteger that you would see in my GitHub repository, you will see the class GaussianInteger here.
Let’s say that to construct a GaussianInteger object, the syntax is new GaussianInteger(a, b), with both a and b being of type int.
The GaussianInteger class provides the four basic arithmetic operations: addition, subtraction, multiplication and division. However, if one Gaussian integer is not divisible by another, a NotDivisibleException will occur.
In the scenario of the example, the four basic arithmetic operations have all been tested, written, and proven to work correctly, though with caveats for overflows.
The example: a function to create ranges of Gaussian integers
So far the whole project is in Java, though the project it was spun off from included a couple of Scala classes.
Regardless, I like to take ideas from Scala. In Scala, most if not all the standard numeric types come with the function to(), which produces an immutable collection, a range, to be specific.
For example, assigning 1 to a and 10 to b, a.to(b) would create a Range containing the integers 1, 2, 3, 4, 5, 6, 7, 8, 9, 10. I deliberately avoided a couple of Scala niceties so as to not get sidetracked with those.
The example then is that I would like to enable a similar capability in GaussianInteger, though on the complex plane, not limited to the real number line.
This will also be written in Java, but interoperability with Scala would be a nice, added bonus.
Then, in our example, with a = new GaussianInteger(1, 0) and b = new GaussianInteger(10, 0), the command a.to(b) would create a list (like maybe an ArrayList) consisting of the numbers 1 + 0i, 2 + 0i, 3 + 0i, …, 10 + 0i.
Or if a = new GaussianInteger(0, 1) and b = new GaussianInteger(0, 10), the command a.to(b) would create a list consisting of the numbers i, 2i, 3i, 4i, 5i, 6i, 7i, 8i, 9i, 10i.
And just one more so that everyone gets the idea: with a = new GaussianInteger(10, 0) and b = new GaussianInteger(0, 10), the command a.to(b) would create a list consisting of the numbers 10, 9 + i, 8 + 2i, 7 + 3i, 6 + 4i, 5 + 5i, 4 + 6i, 3 + 7i, 2 + 8i, 1 + 9i, 10i. The real part decreases, the imaginary part increases.
The function should return whatever Gaussian integers are in a straight line on the complex plane between a and b. If there are none besides a and b, the resulting list consists of just a and b.
Also, I expect the function to always give a result regardless of what direction a and b are relative to each other; one may be straight above or below the other, or in a diagonal, etc.
Writing the stub
If you prefer, you may skip ahead and write the tests first and then skip back here to write the stub.
// STUB TO FAIL THE FIRST TEST
public List<GaussianInteger> to(GaussianInteger endPoint) {
List<GaussianInteger> range = new ArrayList<>();
range.add(new GaussianInteger(0, 0));
return range;
}That should fail the first test. Just as long as we don’t ask for 0 to 0.
Writing the tests
In my actual project, I wrote four tests. One of those tests was checking that AlgebraicDegreeOverflowException occurs under certain circumstances. But since we’re limiting ourselves to Gaussian integers here, we don’t have to worry or even know what an algebraic degree is, much less worry that it could be overflown.
The other three tests still apply:
aandbhave different real and imaginary (x and y) parts, so the line connecting them is a diagonal, e.g., 10 to 10i (the real part of one of them is 0 and the imaginary part of the other is 0)aandbhave the same real part but different imaginary parts, so the line connecting them is vertical, e.g., i to 10i (real part is 0 for all of these)aandbhave the same imaginary part but different real parts, so the line connecting them is horizontal, e.g., 1 to 10 (imaginary part is 0 for all of these)
The stub should fail all three tests because these all require a ≠ b, but the stub always returns the list with just 0 in it.
Hmm… I think we should add a test for a.to(a) or b.to(b), which I’ll call the “single point test.”
Getting a couple of the tests to pass
It seems to me like the horizontal and vertical tests should be very easy to to take from failing to passing. Of course the single point test should be the easiest one to make pass.
public List<GaussianInteger> to(GaussianInteger endPoint) {
List<GaussianInteger> range = new ArrayList<>();
if (this.equals(endPoint)) {
range.add(this);
return range;
}
if (this.real == endPoint.real) {
range.add(this);
GaussianInteger inBetweener = this;
GaussianInteger incr = new GaussianInteger(0, 1);
if (this.imag > endPoint.imag) {
incr = incr.times(-1);
}
do {
inBetweener = inBetweener.plus(incr);
range.add(inBetweener);
} while (!inBetweener.equals(endPoint));
return range;
}
if (this.imag == endPoint.imag) {
range.add(this);
GaussianInteger inBetweener = this;
int incr = 1;
if (this.real > endPoint.real) {
incr = -1;
}
do {
inBetweener = inBetweener.plus(incr);
range.add(inBetweener);
} while (!inBetweener.equals(endPoint));
return range;
}
range.add(this);
// TODO: Logic to pass diagonal test
return range;
}Notice how many times “range.add(this);” is repeated above? I noticed that before getting to a formal refactoring step, and took care of it before running the tests again.
I think that you too would realize right away that that line is something that needs to happen almost every time in the actual program, and indeed every time in the version simplified for this article.
Some refactoring turns out to be of the “blink and you’ll miss it” variety. However, in this example there is still one subtler and more interesting opportunity for refactoring left.
Before we get to that, though, run the tests again. Single-point, horizontal and vertical should all pass now, but diagonal should still fail.
Some more refactoring almost skipped over
I neglected to mention that GaussianInteger.plus() is overloaded so that it can take as an addend either another GaussianInteger or just a plain old int.
It should be clear at this point that a GaussianInteger can have imag = 0 and thus be numerically equal to an int. It’s nice being able to add an int to a GaussianInteger when you need to, but in this case, do we need to?
Except for incr being a GaussianInteger in the this.real == endPoint.real branch and an int in the this.imag == endPoint.imag branch, the two branches of execution are very similar, and both branches have the same do-while loop.
So that’s definitely an opportunity for refactoring. As I turned over the whole thing in my head, I realized that the same do-while loop can also be used to pass the diagonal test. It’s just a matter of figuring out what incr needs to be.
Also, I’ve grown to dislike “incr” as an identifier. Something easier to pronounce would be better, like maybe “step.”
And so, in refactoring, I wound up kinda skipping ahead to the fail-pass-refactor cycle for the diagonal test. Not the most dogmatic form of TDD, but if it works, I’m willing to sacrifice a little purity.
public List<GaussianInteger> to(GaussianInteger endPoint) {
List<GaussianInteger> range = new ArrayList<>();
range.add(this);
if (this.equals(endPoint)) {
return range;
}
GaussianInteger step;
if (this.real == endPoint.real) {
step = new GaussianInteger(0, 1);
if (this.imag > endPoint.imag) {
step = step.times(-1);
}
} else if (this.imag == endPoint.imag) {
step = new GaussianInteger(1, 0);
if (this.real > endPoint.real) {
step = step.times(-1);
}
} else {
step = endPoint.minus(this);
boolean keepGoing = true;
int divisor = 2;
while (keepGoing) {
try {
step = step.divides(divisor);
keepGoing = step.abs() > 1.0;
divisor = 1;
} catch (NotDivisibleException nde) {
keepGoing = nde.getAbs() > 1.0;
}
divisor++;
}
}
GaussianInteger inBetweener = this;
do {
inBetweener = inBetweener.plus(step);
range.add(inBetweener);
} while (!inBetweener.equals(endPoint));
return range;
}In the actual project, I had to add the getAbs() function to NotDivisibleException. It basically does the same thing abs() does for GaussianInteger, only that the value should be a Gaussian number but not a Gaussian integer.
I had some additional problems in the actual project, but those all stemmed from the actual to() not being limited to Gaussian integers.
This example, in theory, should now pass all the tests. I hope you have found this example of refactoring a lot more informative than the previous one.