Test Driven Refactoring

One of the best uses of tests is to allow you to change a single thing about some code and to be sure that there has been no unexpected changes. If a test fails unexpectedly then you know you have a bug to fix.

Refactoring code is a necessary part of software maintenance. We often change the codebase when a new requirement has been added to the application or an old, unused feature needs to be removed. Refactoring can also be done to make the code more readable or to remove code smells identified from static scanning. Some engineering teams devote time to clear up technical debt and this can lead to restructuring and refactoring classes and functions.

Test Driven Development (TDD) is not just for writing new functionality, it should also be applied when refactoring code. The principle is the same: write the test then change the code to fix the test until the desired changes have been made. The difference is that there may already be a test covering the functionality. Sometimes changing the expectation of the existing test is required rather than adding a brand new test.

Refactoring Example

When introducing TDD in How to Start Test Driven Development, I used the example of adding two Roman Numerals. The function was an extension function on the String class and allowed us to write "IV".addRomanNumeral("IX") to add two such numbers. Kotlin has a nice feature that allows functions that are operators to be called inline. If we had two variables representing Roman Numerals, say four and nine, wouldn’t it be better to add them with code like this: four + nine?

The problem is that the String class already uses the plus symbol for string concatenation, which is useful and we don’t want to try to replace that with our own operator function. So to get an inline operator to work with Roman Numerals we are going to refactor the existing code. Instead of basing the operator on a String, let’s create a class to represent Roman Numerals. The inline operator function can be added to this class.

Change a Test

The code has three types of test already: testing the sum, testing conversion from Roman Numeral to Integer, and testing conversion from Integer to Roman Numeral. Looking at the existing test to add two numbers, what needs to be changed here?

@Test
fun `should add two Roman Numerals together showing the result as a Roman Numeral`() {
	val four = "IV"
	val eleven = "XI"
	val expected = "XV"
	val result = four.addRomanNumeral(eleven)
	assertEquals(expected, result)
}

First is that we need to construct an object of our new type rather than a String for the two inputs and the expectation. Let's assume for now that we construct a Roman Numeral by passing in a String object. Here are the three new variables:

val four = RomanNumeral("IV")
val eleven = RomanNumeral("XI")
val expected = RomanNumeral("XV")

This code will not compile as there is no class called RomanNumeral so let’s add that next.

class RomanNumeral(stringRep: String)

The code is still giving out but now at the line where we execute the test because there is no function addRomanNumerals in the class yet. But we don’t want to add that method, rather we want to use an inline operator function. Let’s complete the changes to the test.

@Test
fun `should add two Roman Numerals together showing the result as a Roman Numeral`() {
    val four = RomanNumeral("IV")
    val eleven = RomanNumeral("XI")
    val expected = RomanNumeral("XV")
	val result = four + eleven
	assertEquals(expected, result)
}

Back to the RomanNumeral class next to add an inline operator function as follows, using Kotlin’s TODO() function to defer writing the implementation for now.

inline operator fun plus(another: RomanNumeral): RomanNumeral = TODO()

The test should now be in a runnable state, but as usual with TDD, it is going to fail if it is run as the implementation is not done yet.

More Test Changes

Arguably one could proceed with making the changes to the inline operator function now, but as we know that further tests are going to have to change, it may make more sense to change all the tests first. We may learn more about how to structure the actual code by learning about the other changes.

If we implement the function first then we may have to change it again later. It doesn’t matter which approach you take if you don’t mind refactoring regularly. With TDD, making small, incremental changes becomes second nature.

Let’s think about the extension function on String to convert a Roman Numeral to an Int. Here is an example of one of the tests for that function:

@Test
fun `should convert a Roman Numeral to an integer`() {
	val input = "IV"
	val expected = 4
	val result = input.fromRomanNumeral()
	assertEquals(expected, result)
}

We will make a similar change to the input variable to construct a RomanNumeral instead of a String. Note also that the function under test, fromRomanNumeral, does not really make sense if we were to add it to the RomanNumeral class. A better signature for it would be fun toInt(): Int. Here are both changes:

@Test
fun `should convert a Roman Numeral to an integer`() {
	val input = RomanNumeral("IV")
	val expected = 4
	val result = input.toInt()
	assertEquals(expected, result)
}

We have a compilation error, but that’s easy to fix by adding the toInt function to the RomanNumeral class.

fun toInt(): Int = TODO()

Assuming that’s done, the final tests that need changing are to convert an Int to a Roman Numeral. Here’s the changed test:

@Test
fun `should convert an Int to an Roman Numeral`() {
	val input = RomanNumeral(4)
	val expected = RomanNumeral("IV")
	val result = RomanNumeral(input)
	assertEquals(expected, result)
}

There was a choice here. The above code has added a secondary constructor to the class that takes an Int and should construct the object. Should the object store the integer value or the string representation? Should it store both?

The point I’m trying to make is that by refactoring the all the tests before committing to too many implementation details we can uncover some further questions to think about before changing too much code.

I don’t want users of the RomanNumeral class to have to think about the internal state of the object, nor do I want them the rely on that internal state. I may want to change the implementation in the future. Can the class be designed so that future changes do not have knock on effect on other code?

An alternate approach to having two constructors, is to use a companion object (Kotlin’s equivalent to static functions) that can construct a RomanNumeral from either a String or an Int. We could keep the constructor private, hiding the internal state of the object. This will make the class easier to change later because object creation would happen through the companion object functions. Later changes will only impact the RomanNumeral class and not those depending on it.

I like the approach of using a private constructor with static factory functions over the alternatives (multiple constructors or using a data class). By hiding how the class stores the value internally, it allows changing how the implementation works later without impacting too much other code.

The solution we are going to build will store both the integer value and the string representation. Later we may find this is not a good approach – perhaps converting at object creation is too slow, or the code needs to be more memory efficient. No code outside the class itself should need to change as the static functions will still provide a RomanNumeral object.

The code in the tests to create a RomanNumeral from either an Int or a String will now become:

val four = RomanNumeral.fromString("IV")
val eleven = RomanNumeral.fromInt(11)

Further changes are required to the RomanNumeral class to get the tests to compile, but we will return to that next time.

Conclusion

I did not discover my preferred way to refactor the code immediately. The approach only became apparent after refactoring all the tests first. Before refactoring, ensure that there is complete or wide test coverage for every class or function that might need to change. This allows refactoring the tests first and this will help drive better implementation details when doing the actual code refactoring.