A response to the SWE101 attendees who tried to solve the TDD problem without TDD

This weekend we did some live TDD at the Software Engineering 101 event and broadcast it over LiveMeeting, which was a lot of fun. However, some people have posted solutions (here and here – see first comment) on how they solved the Greed scoring problem without using TDD.

I think you missed the point. The point was to help you learn the TDD thought process and how to put TDD into practice. There are lots of benefits to TDD, and I’ll use these non-TDD examples to illustrate.

How do you know your code works?

Those of you who solved the problem without TDD probably did some sort of manual testing in order to prove that your code is working. You could probably do that with a simple example like the one we had (there were only a few scoring rules). But what happens as we add rules (and in real life we’re always adding rules) and you have to manually test 16 rules? For the record, I took the solution posted here and ran my tests against it.

Tests are documentation

Tests document what the code is supposed to do. Since we wrote our test methods as sentences that read like English, you can figure out the rules just from reading our tests (as you can see in the image above). This is not documentation in Word format which becomes stale, this is living, breathing executable documentation of what the code is supposed to do.

Readability is important

I feel that the solution that we ended up with was very readable. Here is our implementation code:

public class GreedScorer
{
    public double Score(params Die[] dice)
    {
        var score = 0;

        score += ScoreASetOfThreeOnes(dice);
        score += ScoreASetOfThreeTwos(dice);
        score += ScoreASetOfThreeThrees(dice);
        score += ScoreASetOfThreeFours(dice);
        score += ScoreASetOfThreeFives(dice);
        score += ScoreASetOfThreeSixes(dice);
        score += ScoreEachOneThatIsNotAPartOfASetOfThree(dice);
        score += ScoreEachFiveThatIsNotAPartOfASetOfThree(dice);
        return score;
    }

    private int ScoreASetOfThreeOnes(Die[] dice)
    {
        if (dice.Count(die => die.Value == 1) >= 3)
            return 1000;
        return 0;
    }

    private int ScoreASetOfThreeTwos(Die[] dice)
    {
        if (dice.Count(die => die.Value == 2) >= 3)
            return 200;
        return 0;
    }

    private int ScoreASetOfThreeThrees(Die[] dice)
    {
        if (dice.Count(die => die.Value == 3) >= 3)
            return 300;
        return 0;
    }

    private int ScoreASetOfThreeFours(Die[] dice)
    {
        if (dice.Count(die => die.Value == 4) >= 3)
            return 400;
        return 0;
    }

    private int ScoreASetOfThreeFives(Die[] dice)
    {
        if (dice.Count(die => die.Value == 5) >= 3)
            return 500;
        return 0;
    }

    private int ScoreASetOfThreeSixes(Die[] dice)
    {
        if (dice.Count(die => die.Value == 6) >= 3)
            return 600;
        return 0;
    }

    private int ScoreEachOneThatIsNotAPartOfASetOfThree(Die[] dice)
    {
        if (dice.Count(die => die.Value == 1) < 3)
            return (dice.Count(die => die.Value == 1) * 100);
        if (dice.Count(die => die.Value == 1) > 3)
            return ((dice.Count(die => die.Value == 1) - 3) * 100);

        return 0;
    }

    private int ScoreEachFiveThatIsNotAPartOfASetOfThree(Die[] dice)
    {
        if (dice.Count(die => die.Value == 5) < 3)
            return (dice.Count(die => die.Value == 5) * 50);
        if (dice.Count(die => die.Value == 5) > 3)
            return ((dice.Count(die => die.Value == 5) - 3) * 50);

        return 0;
    }
}

Look how readable our code is. Read the score method. Notice how it tells you exactly what it’s doing. Contrast that with one of the other solutions:

static int Score(int[] numbers)
{
    int valueToReturn = 0;

    for (int numberIndex = 0; numberIndex < numbers.Length; numberIndex++)
    {
        switch (numbers[numberIndex])
        {
            case 1:
                if (
                    (numberIndex + 1 < numbers.Length && numbers[numberIndex + 1] == 1) &&
                    (numberIndex + 2 < numbers.Length && numbers[numberIndex + 2] == 1)
                   )
                {
                    valueToReturn += 1000;
                    numberIndex += 2;
                }
                else
                {
                    valueToReturn += 100;
                }
                break;

            case 5:
                if (
                    (numberIndex + 1 < numbers.Length && numbers[numberIndex + 1] == 5) &&
                    (numberIndex + 2 < numbers.Length && numbers[numberIndex + 2] == 5)
                   )
                {
                    valueToReturn += 500;
                    numberIndex += 2;
                }
                else
                {
                    valueToReturn += 50;
                }
                break;

            default:
                if (
                    (numberIndex + 1 < numbers.Length && numbers[numberIndex + 1] == numbers[numberIndex]) &&
                    (numberIndex + 2 < numbers.Length && numbers[numberIndex + 2] == numbers[numberIndex])
                   )
                {
                    valueToReturn += 100 * numbers[numberIndex];
                    numberIndex += 2;
                }
                break;

        }
    }

    return valueToReturn;
}

To me, this code is not very readable. If you had to implement a new rule in this method, it would be hard to do (plus you have no tests to tell you that you broke an existing rule).

The point of this exercise was not to find a clever way to solve a brain teaser. If you want to do a brain teaser, try and write code that will output a Fibonacci sequence using one LINQ expression. That's the kind of geek stuff that you might do at night for fun. But when you're writing real code, we care about things like readability and maintainability. When you're writing implementation code, use common language and write methods whose names tell you what they do.

TDD leads to well-designed code

As we were writing our first test, our test told us that we needed some class that would score the Greed game. So we named it GreedScorer, which does exactly what the name intends. This class will most likely end up following the single responsibility principle because we gave it a very specific name.

But you guys took so long!

We weren't trying to finish as fast as possible. We were not doing a coding competition. We were practicing and learning the TDD mindset and trying to explain things as we went. As you get better at TDD, it becomes more natural and you learn tricks that help you go faster (for example, sometimes writing a whole bunch of tests, watching them all fail, then making them all go pass is faster than just writing one test at a time and making them pass one at a time).

The bottom line is that we came out with some good, readable, maintainable code (with props to Sirena who helped write it). We were able to prove that our code is working. We don't have any code that we don't need. We could easily respond to change and implementing new scoring rules would be pretty easy. Implementing the Score method was fairly easy because we were building it incrementally (solving lots of little problems is easier than trying to solve a big problem all at once). This is why we do TDD!