I’ve seen TDD practiced a number of different ways. All of them use the basic “red, green, refactor” approach (i.e. write a failing test, pass it, refactor to clean up the code), but differ in the way the tests are written and in the focus of each test. Each way seems to lead the design differently, pushing design decisions at different times, requiring different amounts of refactoring and also focussing on quite different elements of design and implementation. I have a feeling that when practiced by masters each approach would converge to similar levels of awesomeness, but how far the rest of us get with each approach seems to vary greatly by how we naturally tend to approach problems.
To investigate this I thought I’d have a go at a coding exercise, and attempt it several different ways. You can’t draw too many conclusions from this as the knowledge I get each time I go through the exercise will impact the later attempts, but I thought it could be an interesting exercise regardless.
The exercise I’ve picked is Roy Osherove’s String Calculator kata. You can read the details at his site, but as it helps to avoid reading ahead I’ll just describe it as we go. The overall gist is a calculator that has an Add() method. This method takes a string input, adds the numbers in the string together, then returns the result.
For this first attempt I’ll be using a traditional Test Driven Development approach (or at least, traditional TDD as I understand it from reading some early-ish work on the topic by Kent Beck et al.). We’ll pick up from the point where I have created a new solution containing a single C# project called CalculatorKata. I have two files: Calculator and CalculatorTests.
First test: empty strings
The first requirement for Add() is that it should return zero when given an empty string. Let’s do that:
[TestFixture]
public class CalculatorTests {
private Calculator calculator;
[SetUp]
public void SetUp() {
calculator = new Calculator();
}
[Test]
public void Takes_empty_string_and_returns_zero() {
Assert.That(calculator.Add(""), Is.EqualTo(0));
}
}
I’ve cheated a bit here by extracting a SetUp method in advance, but I’m pretty sure I’m going to need a new calculator for each test. Maybe this is evil. Feel free to point out the problems with this in the comments. This test doesn’t pass, so let’s pass it:
public class Calculator {
public int Add(string expression) {
return 0;
}
}
It passes. Stay with me, things should start getting more interesting later (I hope…).
Test 2: Adding a single number
The next requirement is for when the string contains a single number. The test and passing implementation is below:
//CalculatorTests.cs
[Test]
public void Takes_a_single_number_and_returns_it() {
Assert.That(calculator.Add("2"), Is.EqualTo(2));
}
//Calculator.cs
public int Add(string expression) {
if (expression == "") return 0;
return int.Parse(expression);
}
Summing multiple numbers
The next requirement is to actually sum multiple numbers separated by commas (‘,’). We’ll start of with the easy case: 2 numbers.
//CalculatorTests.cs
[Test]
public void Takes_two_numbers_delimited_by_commas_and_returns_the_sum() {
Assert.That(calculator.Add("1,2"), Is.EqualTo(3));
}
//Calculator.cs
public int Add(string expression) {
if (expression == "") return 0;
return expression.Split(',').Sum(x => int.Parse(x));
}
Now you’re quite welcome to debate me on whether this is actually the simplest thing that could possibly work. I could assume from my test that I’ll only be dealing with two single digit numbers separated by a comma, get each digit via array access (expression[0] and expression[2]), parse each as an int and return the sum, but that hardly sounds any simpler to me. The current implementation will also pass our next test:
[Test]
public void Takes_multiple_numbers_separated_by_commas_and_returns_the_sum() {
Assert.That(calculator.Add("1,2,3,4,5,6,7,8,9"), Is.EqualTo(45));
}
The only refactoring I can see is extracting the delimiter (‘,’) into a constant. Once that’s done we can go to the next step.
Allow commas and newlines as delimiters
//CalculatorTests.cs
[Test]
public void Takes_numbers_delimited_by_a_newline_and_returns_the_sum() {
Assert.That(calculator.Add("1\n2\n3"), Is.EqualTo(6));
}
//Calculator.cs
public class Calculator {
private string[] delimiters = new[] {",", "\n"};
public int Add(string expression) {
if (expression == "") return 0;
return expression.Split(delimiters, StringSplitOptions.None).Sum(x => int.Parse(x));
}
}
Allow a custom delimiter
The next requirement in the kata is more interesting. As input we can optionally specify a delimiter by using the following format: //[delimiter]\n[numbers]. The test is fairly simple, but the code got very messy:
//CalculatorTest.cs
[Test]
public void Can_set_the_delimiter_at_the_start_of_the_expression() {
Assert.That(calculator.Add("//;\n1;2;3;4"), Is.EqualTo(10));
}
//Calculator.cs
public class Calculator {
private const string CustomDelimiterToken = "//";
private string[] DefaultDelimiters = new[] {",", "\n"};
public int Add(string expression) {
if (expression == "") return 0;
var delimiters = DefaultDelimiters;
if (expression.StartsWith(CustomDelimiterToken)) {
var indexOfStartOfCustomDelimiter = CustomDelimiterToken.Length;
var indexAfterCustomDelimiter = expression.IndexOf("\n");
var customDelimiter = expression.Substring(indexOfStartOfCustomDelimiter, indexAfterCustomDelimiter - indexOfStartOfCustomDelimiter);
delimiters = new[] {customDelimiter};
expression = expression.Substring(indexAfterCustomDelimiter + 1);
//Console.WriteLine(expression);
}
return expression.Split(delimiters, StringSplitOptions.None).Sum(x => int.Parse(x));
}
}
That’s beginning to look pretty intolerable. Our test passes, but this is in desperate need of refactoring. Now I’m pretty sure I’ve stuffed up the traditional TDD process here: this seems like much too big a step to get this test to pass. This could be an indication that I should have refactored first. Another indication is, as you may have noticed, I left a commented out Console.WriteLine() in there. That was because I had initially stuffed up the string escaping of the newline character, and couldn’t find a good way to get inside my implementation to test what the value was at that point. This just feels plain dirty when you resort to that, but pragmatism won that battle and it helped me fix my error. All of this is really screaming out for a nice big refactoring, and I did refactor, but it was mainly tinkering around the edges using Extract Method to make the mess more understandable, rather than cutting through the mess itself.
public class Calculator {
private const string CustomDelimiterToken = "//";
private const string NewLine = "\n";
private string[] DefaultDelimiters = new[] {",", NewLine};
public int Add(string expression) {
if (expression == "") return 0;
var delimiters = DefaultDelimiters;
if (HasCustomDelimiterSpecified(expression)) {
delimiters = new[] {GetCustomDelimiter(expression)};
expression = GetExpressionWithoutCustomDelimiterSpecification(expression);
}
return expression.Split(delimiters, StringSplitOptions.None).Sum(x => int.Parse(x));
}
private string GetExpressionWithoutCustomDelimiterSpecification(string expression) {
return expression.Substring(IndexOfFirstNewLine(expression) + 1);
}
private string GetCustomDelimiter(string expression) {
var indexOfStartOfCustomDelimiter = CustomDelimiterToken.Length;
var indexAfterCustomDelimiter = IndexOfFirstNewLine(expression);
var lengthOfCustomDelimiter = indexAfterCustomDelimiter - indexOfStartOfCustomDelimiter;
var customDelimiter = expression.Substring(indexOfStartOfCustomDelimiter, lengthOfCustomDelimiter);
return customDelimiter;
}
private int IndexOfFirstNewLine(string expression) {
return expression.IndexOf(NewLine);
}
private bool HasCustomDelimiterSpecified(string expression) {
return expression.StartsWith(CustomDelimiterToken);
}
}
Not horribly impressive, is it? Perhaps a little easier to understand the Add() method itself, but this implementation hardly fills me with joy. The kata is not fully finished yet, but I think this is a decent place to stop and take stock.
Where did I go wrong?
Well, first up I missed the cue for a bigger refactoring. My tests weren’t really screaming out a nice direction for me to go though. If you know your SOLID principles you’ll have noticed right away that I am violating the Single Responsibility Principle (SRP). My class is doing lots of things: interpretting custom delimiters specified in a string, parsing numbers from the string, and adding the numbers.
Later on I went through and factored out a NumberParser class which took care of all the delimiter stuff and returning the numbers in the string, but that class still needed to be broken down further as well. Then I also needed to update the tests, so the calculator tests only test that it adds the numbers that come back from a mock NumberParser, and then adjust most of the current tests to test that the number parser is doing the parsing properly. Sure, I could leave the current tests as integration tests and forget about each unit, but I’ve this approach has given me troubles in the past.
Now I’m sure that if I were a better TDDer, refactorer, OOer etc that this would have turned out better. But my main complaint is that my tests aren’t really helping me drive out my design. They help me to drive out an implementation that works (for which I am very grateful – so much better than old school hack and slash :)), but by the time I am getting design feedback and test smells I’ve already dug myself in a bit of a hole. Luckily I’ve got tests to haul myself out, but is there a better way? Or do I need to just resign myself to the fact that I’m going to have to run through the gauntlet of SOLID principles and GRASP patterns etc. after each test to see when I need to refactor? But even then, refactor to what? I can start writing tests for the NumberParser, then join the bits back together, but this seems a bit hit-and-miss to me.
I’d love to get your thoughts on this. Anyone that wants to give the first part of the kata a try I’d love to read a blog post or have a chat with you and see how things ended up for you.
Next up I’ll try a different flavour of TDD on the same problem.