TDD For Breaking Problems Apart 3: Finishing Up

Last time, we left off with a bowling score calculator that handled basic score calculation with the exception of double strikes and the tenth frame. Here is the code as of right now for both classes:

[TestClass]
public class BowlingTest
{
    [TestClass]
    public class Constructor
    {

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Score_To_Zero()
        {
            var scoreCalculator = new BowlingScoreCalculator();

            Assert.AreEqual(0, scoreCalculator.Score);
        }
    }

    [TestClass]
    public class BowlFrame
    {
        private static BowlingScoreCalculator Target { get; set; }

        [TestInitialize()]
        public void BeforeEachTest()
        {
            Target = new BowlingScoreCalculator();
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_0_And_1_Results_In_Score_1()
        {
            var frame = new Frame(0, 1);
            Target.BowlFrame(frame);

            Assert.AreEqual(frame.FirstThrow + frame.SecondThrow, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_2_And_3_Results_In_Score_5()
        {
            var frame = new Frame(2, 3);
            Target.BowlFrame(frame);

            Assert.AreEqual(frame.FirstThrow + frame.SecondThrow, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Sets_Score_To_2_After_2_Frames_With_Score_Of_1_Each()
        {
            var frame = new Frame(1, 0);
            Target.BowlFrame(frame);
            Target.BowlFrame(frame);

            Assert.AreEqual(frame.Total + frame.Total, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Sets_Score_To_Twenty_After_Spare_Then_Five_Then_Zero()
        {
            var firstFrame = new Frame(9, 1);
            var secondFrame = new Frame(5, 0);

            Target.BowlFrame(firstFrame);
            Target.BowlFrame(secondFrame);

            Assert.AreEqual(20, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Sets_Score_To_25_After_Strike_Then_Five_Five()
        {
            var firstFrame = new Frame(10, 0);
            var secondFrame = new Frame(6, 4);

            Target.BowlFrame(firstFrame);
            Target.BowlFrame(secondFrame);

            Assert.AreEqual(30, Target.Score);
        }
    }
}

public class BowlingScoreCalculator
{
    private readonly Frame[] _frames = new Frame[10];

    private int _currentFrame;

    private Frame LastFrame { get { return _frames[_currentFrame - 1]; } }

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        AddMarkBonuses(frame);

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private void AddMarkBonuses(Frame frame)
    {
        if (WasLastFrameAStrike()) Score += frame.Total;
        else if (WasLastFrameASpare()) Score += frame.FirstThrow;
    }

    private bool WasLastFrameAStrike()
    {
        return _currentFrame > 0 && LastFrame.IsStrike;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && LastFrame.IsSpare;
    }
}

[TestClass]
public class FrameTest
{
    [TestClass]
    public class Constructor
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_FirstThrow_To_Passed_In_Value()
        {
            var frame = new Frame(1, 0);

            Assert.AreEqual(1, frame.FirstThrow);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_SecondThrow_To_Passed_In_Value()
        {
            var frame = new Frame(0, 1);

            Assert.AreEqual(1, frame.SecondThrow);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Throws_Exception_On_Negative_Argument()
        {
            ExtendedAssert.Throws(() => new Frame(-1, 0));
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Throws_Exception_On_Score_Of_11()
        {
            ExtendedAssert.Throws(() => new Frame(Frame.Mark, 1));
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Total_To_FirstThrow_Plus_SecondThrow()
        {
            const int firstThrow = 4;
            const int secondThrow = 3;

            Assert.AreEqual(firstThrow + secondThrow, new Frame(firstThrow, secondThrow).Total);
        }
    }

    [TestClass]
    public class IsStrike
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_True_When_Frame_Has_10_For_First_Throw()
        {
            var frame = new Frame(10, 0);
            Assert.IsTrue(frame.IsStrike);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_False_When_Frame_Does_Not_Have_10_For_First_Throw()
        {
            var frame = new Frame(0, 2);
            Assert.IsFalse(frame.IsStrike);
        }
    }

    [TestClass]
    public class IsSpare
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_True_When_Frame_Totals_Mark()
        {
            var frame = new Frame(4, 6);

            Assert.IsTrue(frame.IsSpare);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_False_When_Frame_Does_Not_Total_10()
        {
            var frame = new Frame(0, 9);
            Assert.IsFalse(frame.IsSpare);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_False_When_Frame_Is_Strike()
        {
            var frame = new Frame(10, 0);
            Assert.IsFalse(frame.IsSpare);
        }
    }
}

public class Frame
{
    public class UnderflowException : Exception { }

    public class OverflowException : Exception { }

    public const int Mark = 10;

    public int FirstThrow { get; private set; }

    public int SecondThrow { get; private set; }

    public int Total { get { return FirstThrow + SecondThrow; } }

    public bool IsStrike { get { return FirstThrow == Frame.Mark; } }

    public bool IsSpare { get { return !IsStrike && Total == Frame.Mark; } }

    public Frame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new UnderflowException();
        if (firstThrow + secondThrow > Mark)
            throw new OverflowException();

        FirstThrow = firstThrow;
        SecondThrow = secondThrow;
    }
}

Without further ado, let’s get back to work. The first thing I’d like to do is actually a refactor. I think it would be more expressive when creating strikes to use a static property, Frame.Strike, rather than new Frame(10, 0). Since the strike is completely specific in nature and a named case, I think this approach makes sense. So the first thing that I’m going to do is test that it returns a frame where IsStrike is true:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Returns_Frame_With_Is_Strike_True()
{
    Assert.IsTrue(Frame.Strike.IsStrike);
}

(This is actually what the test looked like after two red-green-refactors, since the first one was just to define Frame.Strike). At this point, I now have a static property that I can use and I’m going to find everywhere in my score calculator and frame test classes that I queued up a strike and use that instead as part of this refactor cycle. While I’m at it, I also demote the visibility of Frame.Mark, since I realize I should have done that a while ago. The Mark constant isn’t needed outside of Frame since Frame is now expressive with IsStrike, IsSpare and Total. Strictly speaking, I should conceive of some test to write that will fail if Mark is visible outside of the class, but I try to be pragmatic, and that’s a screwy test to have and persist.

Now, let’s get down to brass tacks and fix the double strike issue. If I bowl a strike in the first frame, another in the second frame, and then a 9 in the third frame, my total score should be 57 in the third (29+19+9). Let’s write such a test:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_57_After_Two_Strikes_And_A_Nine()
{
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(new Frame(9, 0));

    Assert.AreEqual(57, Target.Score);
}

How to get this to pass… well, I’ll just tack on an extra frame.FirstThrow if the last two were strikes:

private void AddMarkBonuses(Frame frame)
{
    if (_currentFrame > 1 && LastFrame.IsStrike && _frames[_currentFrame - 2].IsStrike)
        Score += frame.Total;

    if (WasLastFrameAStrike()) 
        Score += frame.Total;
    else if (WasLastFrameASpare()) 
        Score += frame.FirstThrow;
}

… and NCrunch gives me green. Now, let’s make the class a little nicer to look at:

public class BowlingScoreCalculator
{
    private readonly Frame[] _frames = new Frame[10];

    private int _currentFrame;

    private Frame LastFrame { get { return _frames[_currentFrame - 1]; } }

    private Frame TwoFramesAgo { get { return _frames[_currentFrame - 2]; } }

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        AddMarkBonuses(frame);

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private void AddMarkBonuses(Frame frame)
    {
        if (WereLastTwoFramesStrikes())
            Score += 2 * frame.Total;
        else if (WasLastFrameAStrike()) 
            Score += frame.Total;
        else if (WasLastFrameASpare()) 
            Score += frame.FirstThrow;
    }

    private bool WereLastTwoFramesStrikes()
    {
        return WasLastFrameAStrike() && _currentFrame > 1 && TwoFramesAgo.IsStrike;
    }

    private bool WasLastFrameAStrike()
    {
        return _currentFrame > 0 && LastFrame.IsStrike;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && LastFrame.IsSpare;
    }
}

And, that’s that. Now we have to think about the 10th frame. This is going to be interesting because the 10th frame is completely different in concept than the other frames. The 10th frame’s total can range up to 30 instead of being capped at 10, and if you get a strike in the first frame or spare in the second frame, you get three throws instead of two. How to model this with what we have… add a new property to the frame class called “ThirdThrow”? That seems reasonable, but what if we populate the third throw when we’re not in the 10th frame? That’s no good — how can we know that a frame is a 10th frame? We’ll probably need a boolean property called IsTenthFrame… right?

Wrong! (At least in my opinion). That amounts to adding a flag that clients look at to know how to treat the object. If the flag is set to true, we treat it like one kind of object and if it’s set to false, we treat it like another kind. This is a code smell in my opinion — one that I think of as “polymorphism envy” or “poor man’s polymorphism”. This is a milder version of the kind you usually see which is some ObjectType enum that clients switch over. We don’t have that (yet) because we only have two values.

So if we’re contemplating a polymorphism envy approach, it stands to reason that maybe what we’re nibbling at is, well, actual polymorphism. Maybe we should have a TenthFrame class that derives from Frame and overrides important functionality. I don’t know that this is the solution, but TDD is about solving small problems incrementally, so let’s start down this path and see where it leads. We don’t need all of the answers this minute. The first thing to test is probably going to be that total is the sum of the three constructor arguments:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Initializes_Total_To_Sum_Of_Three_Throws()
{
    const int firstThrow = 1;
    const int secondThrow = 2;
    const int thirdThrow = 3;
    var frame = new TenthFrame(firstThrow, secondThrow, thirdThrow);

    Assert.AreEqual(firstThrow + secondThrow + thirdThrow, frame.Total);
}

As I wrote this test, two things didn’t compile. The first was the instantiation of the TenthFrame, which I solved by declaring it. The second was the Total property, which I solved by inheriting from Frame. That actually turned out to be an easier way to get a red test than declaring the property (and more productive toward our design). Then to get the test passing, the easiest thing to do was make Frame’s total virtual and override it in TenthFrame. So, pretty quickly we seem to be getting Total right:

public class TenthFrame : Frame
{
    public int ThirdThrow { get; private set; } 

    public override int Total { get { return base.Total + ThirdThrow; }  }

    public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) : base(firstThrow, secondThrow)
    {
        ThirdThrow = thirdThrow;
    }
}

Now we need to start tweaking the business rules. Parent is going to throw an exception if we initialize frame 1 and 2 each to a strike, but that’s fine in the 10th frame. Here’s a failing test:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Does_Not_Throw_Exception_On_Two_Strikes()
{
    ExtendedAssert.DoesNotThrow(() => new TenthFrame(10, 10, 9));
}

To get this passing, I declare a default constructor in the base (to make the compiler happy) and have the new class’s constructor implement its own assignment logic to avoid the checks in parent that cause this failure. But now that simple assignment is restored, we need to implement our own rules, which will include throwing exceptions for throws greater than ten or less than zero, but it will also include oddballs like this that I don’t yet know how to describe:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Throws_Exception_For_5_10_5()
{
    ExtendedAssert.Throws(() => new TenthFrame(5, 10, 5));
}

This is another one of the real draws of TDD. I don’t know what to call this or how to categorize it, but I have an example, and that’s all I need to get started. I just have to make this pass:

public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) 
{
    ValidateIndividualThrows(firstThrow, secondThrow, thirdThrow);

    if (firstThrow != Mark && secondThrow + firstThrow > Mark)
        throw new IllegalFrameException();

    FirstThrow = firstThrow;
    SecondThrow = secondThrow;
    ThirdThrow = thirdThrow;
}

I could have just tested for the specific literals in the test, but I didn’t feel the need to be that obtuse. You can really control your own destiny somewhat with “simplest thing to make the test pass”. IF you have no idea what direction the design should take, maybe you go that obtuse route. If you have some half-formed idea, as I do here, it’s fine to get a little more business-logic-y. I’m going to dial up another case that should fail because of the relationship between second and third frame and take if from there:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Throws_For_10_5_10()
{
    ExtendedAssert.Throws(() => new TenthFrame(10, 5, 10));
}

Now I have the following production code to get it to pass:

public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) 
{
    ValidateIndividualThrows(firstThrow, secondThrow, thirdThrow);

    if (firstThrow != Mark && secondThrow + firstThrow > Mark)
        throw new IllegalFrameException();
    if (secondThrow != Mark && thirdThrow + secondThrow > Mark)
        throw new IllegalFrameException();

    FirstThrow = firstThrow;
    SecondThrow = secondThrow;
    ThirdThrow = thirdThrow;
}

But, that’s getting a little fugly, so let’s refactor:

public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) 
{
    ValidateIndividualThrows(firstThrow, secondThrow, thirdThrow);
    CheckConsecutiveThrows(firstThrow, secondThrow);
    CheckConsecutiveThrows(secondThrow, thirdThrow);

    FirstThrow = firstThrow;
    SecondThrow = secondThrow;
    ThirdThrow = thirdThrow;
}

private static void CheckConsecutiveThrows(int first, int second)
{
    if (first != Mark && first + second > Mark)
        throw new IllegalFrameException();
}

Ah, a business rule is starting to emerge. In general, if a throw is not a mark, then it and the subsequent throw can’t be greater than 10. Hey, come to think of it, that sounds right from my bowling experience. They only reset the pins if you knock ’em all down. Of course, we have one final rule to implement, which is that if the first and second throws don’t knock all the pins down, there is no third throw. I’ll leave that out, since it’s pretty easy.

Now the time has arrived for some integration testing. I found a site that has some example bowling scores, and I’m going to code one up:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void SampleGame_For_Mary()
{
    Target.BowlFrame(new Frame(9, 0));
    Target.BowlFrame(new Frame(3, 7));
    Target.BowlFrame(new Frame(6, 1));
    Target.BowlFrame(new Frame(3, 7));
    Target.BowlFrame(new Frame(8, 1));
    Target.BowlFrame(new Frame(5, 5));
    Target.BowlFrame(new Frame(0, 10));
    Target.BowlFrame(new Frame(8, 0));
    Target.BowlFrame(new Frame(7, 3));
    Target.BowlFrame(new TenthFrame(8, 2, 8));

    Assert.AreEqual(131, Target.Score);
}

If you’re following along, you’ll see green in NCrunch. Let’s try one more:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void SampleGame_For_Kim()
{
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(new Frame(3, 7));
    Target.BowlFrame(new Frame(6, 1));
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(new Frame(2, 8));
    Target.BowlFrame(new Frame(9, 0));
    Target.BowlFrame(new Frame(7, 3));
    Target.BowlFrame(new TenthFrame(10, 10, 10));

    Assert.AreEqual(193, Target.Score);
}

Oops. Red. So, what happened? Well, I went back through game, setting temporary asserts until I found that things went off the rails following the third strike in a row. I then looked in my class at the logic following the two strikes and realized it wasn’t quite right:

private void AddMarkBonuses(Frame frame)
{
    if (WereLastTwoFramesStrikes())
        Score += frame.Total + frame.FirstThrow;
    //Score += 2 * frame.Total;
    else if (WasLastFrameAStrike())
        Score += frame.Total;
    else if (WasLastFrameASpare())
        Score += frame.FirstThrow;
}

I commented out the mistake and put in the correct code, and the entire test suite went green, including the integration test for Kim. I think this is a good note to close on because the tests are all passing and I believe the calculator is functioning (here it is on gist if you want to check out the final product) but also because I think there’s a valuable point here.

TDD is a design methodology — not a guarantee of bug free code/comprehensive testing strategy. I experienced and even blogged about writing code for days using TDD and assembling all the parts and having it work flawlessly the first time, and that did happen. But I realized that even with that, the happy and happy-ish paths were the ones that worked. Here, I had a bowling calculator that made it through all individual scoring tests and even an entire non-trivial game going green before we teased out a case in smoke testing where it went red.

TDD will help you break problems into small pieces to solve (the whole point of this series of posts), ensure that your code is testable and thus loosely coupled and modular, ensure that you don’t push dirty mop water around the floor by breaking functionality that you had working before, and generally promote good code. But think about this — those are all productive programming concerns rather than testing concerns. You still need testers, you still need edge case unit tests once you’re done with TDD, and you still need integration/smoke tests. The fact that TDD produces a lot of tests that you can check in and continue to use is really just a bonus.

And it’s a bonus that keeps on giving. Because let’s say that you don’t agree with my decision to use inheritance for tenth frame or you think strike would be a more appropriate descriptor of a throw than of a frame. With all of my TDD test artifacts (and the integration tests) in place, you can rip my internal design to pieces without worrying that you’re going to break the functionality that this provides to clients. And that’s incredibly powerful for allowing fearless refactoring and maintenance of this code. So do it to break your problems apart and keep yourself moving and productive, and keep the tests around to make sure the code stays clean and trends toward improvement rather than rot.

Full Code

• BowlingScoreCalculator.cs
• BowlingTest.cs
• Frame.cs
• FrameTest.cs
• TenthFrame.cs
• TenthFrameTest.cs
• IntegrationTest.cs
• ExtendedAssert.cs

TDD For Breaking Problems Apart 2: Magic Boxes

In the last post, I started a TDD exploration of the “bowling score” code kata. Today, I’m going to build a “magic box” in the course of TDD to solve the problem that arose at the end of the post with the scoring class doing too much.

Last time, we left off with production code that looked like this:

public class BowlingScoreCalculator
{
    public const int Mark = 10;

    public class FrameUnderflowException : Exception { }

    public class FrameOverflowException : Exception { }

    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new FrameUnderflowException();
        else if (firstThrow + secondThrow > Mark)
            throw new FrameOverflowException();

        Score += firstThrow + secondThrow;
    }
}

And the last test we had written looked like this:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_2_After_2_Frames_With_Score_Of_1_Each()
{
    Target.BowlFrame(1, 0);
    Target.BowlFrame(1, 0);

    Assert.AreEqual(2, Target.Score);
}

I had noticed that the amount of integer literals here seemed to be a bit of a smell and was starting to wrap my head around the idea of what to do about it from a design perspective. Specifically, the smell in question is “primitive obsession,” a tendency to overuse primitive types, often redundantly, due to reluctance to make an object. So what if we made an object — a frame object — what would that look like?

Looking at the BowlFrame() method, I think “what if I had a magic box that handled frame operations?” Well, the method would take a frame object and presumably that object would handle validating the individual throws, so the method would lose that exception handling logic (and the class would probably lose those exception definitions). It would probably also make sense for the frame to encapsulate some kind of totaling mechanism internally so that the score calculator didn’t need to inspect its properties to figure out what to add.

At this point, though, I’m going to stop and start coding up the frame class as I envision it. After the first test (including a couple of non-compile failures and passes prior to the test run failure), this is what the frame test and production code looks like:

[TestClass]
public class FrameTest
{
    [TestClass]
    public class Constructor
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_FirstThrow_To_Passed_In_Value()
        {
            var frame = new Frame(1, 0);

            Assert.AreEqual(1, frame.FirstThrow);
        }
    }
}

public class Frame
{
    public int FirstThrow { get; private set; }

    public Frame(int firstThrow, int secondThrow)
    {
        FirstThrow = firstThrow;
    }
}

The first thing I do at this point is refactor the test to eliminate the magic numbers and duplication. At this point, you might wonder why, having done this enough times, I don’t simply start out by leaving out the magic numbers and duplication. Am I obtuse? Well, sometimes, but the reason I often don’t bother with this is that I want the test I’m writing to be the simplest possible thing as I perceive it for problem solving purposes — not necessarily the quickest and certainly not the cleanest. I wouldn’t ever purposefully make a mess, but if calling the frame constructor with (1, 0) seems more intuitive to me, then that’s what I do. I encourage you to do the same. Simplicity as you perceive it is of the utmost importance writing the failing test. Whatever it is, make it pretty and elegant later.

The next thing I do is add a similar test and then code for the second throw, with similar green and refactor. With that in place, I move the exception handling tests from the ScoreCalculator test class to the Frame test class and change them to operate on Frame’s constructor. When I do this, I get a red test, which I make green by porting the exception handling code and constants over to the new Frame class. The production code is now:

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        Score += firstThrow + secondThrow;
    }
}

public class Frame
{
    public class UnderflowException : Exception { }

    public class OverflowException : Exception { }

    public const int Mark = 10;

    public int FirstThrow { get; private set; }

    public int SecondThrow { get; private set; }

    public Frame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new UnderflowException();
        if (firstThrow + secondThrow > Mark)
            throw new OverflowException();

        FirstThrow = firstThrow;
        SecondThrow = secondThrow;
    }
}

I’m happy that all of that logic about determining what constitutes a valid frame is now out of the BowlingScoreCalculator class, but it’s not yet sitting inside of a magic box — it’s just gone. So, let’s give the calculator the box that it’s looking for. We’re going to need a failing test, and the easiest way to do that is to change a test where we’re bowling a frame to look like this:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void With_Throws_0_And_1_Results_In_Score_1()
{
    var frame = new Frame(0, 1);
    Target.BowlFrame(frame);

    Assert.AreEqual(frame.FirstThrow + frame.SecondThrow, Target.Score);
}

That doesn’t compile, so we can fix the broken test by adding this method in production:

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        Score += firstThrow + secondThrow;
    }

    public void BowlFrame(Frame frame)
    {
        BowlFrame(frame.FirstThrow, frame.SecondThrow);
    }
}

At this point, we can refactor the rest of the tests to use the second method and then refactor that method to inline the original method and now the score calculator only accepts frames. There’s our magic box. Suddenly, we’re just worrying about how to add the scores from the frame’s throws and not whether the frame we’re being passed is valid or not. And that makes sense — we shouldn’t care about the anatomy of a frame in a class responsible for adding up and tracking frames. We should be able to assume that if the frame exists and is being handed to us that it’s valid.

Now that this bigger refactoring is done, there’s still something a little fishy. Look at this test and production code for the calculator:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_2_After_2_Frames_With_Score_Of_1_Each()
{
    var frame = new Frame(1, 0);
    Target.BowlFrame(frame);
    Target.BowlFrame(frame);

    Assert.AreEqual(frame.FirstThrow + frame.SecondThrow + frame.FirstThrow + frame.SecondThrow, Target.Score);
}

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        Score += frame.FirstThrow + frame.SecondThrow;
    }
}

We’ve fixed our primitive obsession, but there sure seems to be a lot of redundancy. I mean I have to ask frame for two things in the production code and four things in the test: two for each frame. What if I had a magic box that turned those two things into one? What would that look like? Well, here’s a test I can write against frame that I think will tell me:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Initializes_Total_To_FirstThrow_Plus_SecondThrow()
{
    const int firstThrow = 4;
    const int secondThrow = 3;

    Assert.AreEqual(firstThrow + secondThrow, new Frame(firstThrow, secondThrow).Total);
}

Now I just need to define that property as the sum of my two throws to make it pass. I do that, and now I can refactor my score calculator and test. Here’s what it looks like now:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_2_After_2_Frames_With_Score_Of_1_Each()
{
    var frame = new Frame(1, 0);
    Target.BowlFrame(frame);
    Target.BowlFrame(frame);

    Assert.AreEqual(frame.Total + frame.Total, Target.Score);
}

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        Score += frame.Total;
    }
}

Now, all of the frame stuff is removed from score calculation and those concerns are separated. The “magic box,” Frame, handles evaluating frames for validity and totaling them up. This score calculator class is actually starting to smell like a lazy class that could be replaced with a list and a call to a Linq extension method, but I’m going to keep it around on a hunch.

Okay, now that we’ve done a good bit of cleanup and pulled out a magic box, time to get back to implementing features. Off the top of my head, I can think of two scenarios that we don’t currently handle: the 10th frame and marks carrying over from previous frames. I’m going to work on the latter for now and write a test that when I bowl a spare in the first frame and a 5, 0 in the second frame, my score should be 20 instead of 15.

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_Twenty_After_Spare_Then_Five_Then_Zero()
{
    var firstFrame = new Frame(9, 1);
    var secondFrame = new Frame(5, 0);

    Target.BowlFrame(firstFrame);
    Target.BowlFrame(secondFrame);

    Assert.AreEqual(20, Target.Score);
}

This test fails and I need to get it to pass. There are no trivial tricks I can do, but I will do the simplest thing I can think of. I’ll store an array, write frames to it, and check last frame to know when to handle this case. Here is the simplest thing I can think of that makes this pass:

public class BowlingScoreCalculator
{
    private Frame[] _frames = new Frame[10];

    private int _currentFrame;

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        if (_currentFrame > 0 && _frames[_currentFrame - 1].Total == Frame.Mark)
            Score += frame.FirstThrow;

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }
}

That’s pretty ugly though, so let’s clean it up at least a little before we move on. Boy Scout Rule and all that:

public class BowlingScoreCalculator
{
    private Frame[] _frames = new Frame[10];

    private int _currentFrame;

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        if (WasLastFrameAMark())
            Score += frame.FirstThrow;

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private bool WasLastFrameAMark()
    {
        return _currentFrame > 0 && _frames[_currentFrame - 1].Total == Frame.Mark;
    }
}

Now the method is semantically clearer than it was with all of that crap in the guard condition. If the last frame was a mark, do something different with this frame, and then do our normal score augmentation and frame recording. We might decide to do some reasoning at this point about whether score should be computed as frames are added or only on demand, but that’s a little nitpicky, so let’s instead focus on the fact that this doesn’t score strikes correctly. So we’ll write a test in which we bowl a strike in the first frame and then a (6, 4) in the second frame and assert that the score should be 30 (unlike the current implementation, which would make it 26):

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_25_After_Strike_Then_Five_Five()
{
    var firstFrame = new Frame(10, 0);
    var secondFrame = new Frame(6, 4);

    Target.BowlFrame(firstFrame);
    Target.BowlFrame(secondFrame);

    Assert.AreEqual(30, Target.Score);
}

And, let’s fix the code:

public void BowlFrame(Frame frame)
{
    if (_currentFrame > 0 && _frames[_currentFrame - 1].FirstThrow == Frame.Mark)
        Score += frame.Total;
    else if (WasLastFrameAMark())
        Score += frame.FirstThrow;

    Score += frame.Total;
    _frames[_currentFrame++] = frame;
}

That makes everyone green, but it’s ugly, so let’s refactor it to this:

public class BowlingScoreCalculator
{
    private readonly Frame[] _frames = new Frame[10];

    private int _currentFrame;

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        AddMarkBonuses(frame);

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private void AddMarkBonuses(Frame frame)
    {
        if (WasLastFrameAStrike())
            Score += frame.Total;
        else if (WasLastFrameASpare())
            Score += frame.FirstThrow;
    }

    private bool WasLastFrameAStrike()
    {
        return _currentFrame > 0 && _frames[_currentFrame - 1].FirstThrow == Frame.Mark;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && _frames[_currentFrame - 1].Total == Frame.Mark;
    }
}

Now BowlFrame() is starting to read like the actual scoring rules of bowling. “If the last frame was a strike, add current frame’s total points to the score as a bonus. Otherwise, if it was a spare, add the first throw’s points to the score as a bonus. No matter what, add this frame’s pins.” Not too shabby. At this point, before proceeding any further with the score calculator (such as handling double strikes and the 10th frame), I’d like to clean up something I don’t like: the fact that the score class is responsible for figuring out whether a frame is a strike or spare. I’m eliding the details of the tests I wrote for the Frame class to make this happen, but they’re pretty straightforward and I’ll post everything at the end of the series. Here is the new look calculator, after some refactoring:

public class BowlingScoreCalculator
{
    private readonly Frame[] _frames = new Frame[10];

    private int _currentFrame;

    private Frame LastFrame { get { return _frames[_currentFrame - 1]; } }

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        AddMarkBonuses(frame);

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private void AddMarkBonuses(Frame frame)
    {
        if (WasLastFrameAStrike())
            Score += frame.Total;
        else if (WasLastFrameASpare())
            Score += frame.FirstThrow;
    }

    private bool WasLastFrameAStrike()
    {
        return _currentFrame > 0 && LastFrame.IsStrike;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && LastFrame.IsSpare;
    }
}

Next time, I’ll cover the addition of double strike logic and the tenth frame, and I’ll explore some different alternatives that are easily achievable with a nice suite of unit tests backing things up.

TDD For Breaking Problems Apart

If I have to write some code that’s going to do something rather complex, it’s easy for my mind to start swimming with possibilities, as mentioned in the magic boxes post. In that post, I talked about thinking in abstractions and breaking problems apart and alluded to TDD being a natural fit for this paradigm without going into much detail. Today and in my next post about this I’m going to go into that detail.

Let’s say I’m tasked with coding some class or classes that keep track of bowling scores. I don’t think “okay, what is my score at the start of a bowling game?” I think things like “I’ll need to have some iteration logic that remembers back at least two frames, since that’s how far back we can go, and I’ll probably need something to look ahead to handle the 10th frame, and maybe there’s a design pattern for that, and…” Like I said, swimming. These are the insane ramblings of an over-caffeinated maniac more than they’re a calm and methodical solution to a somewhat, but not terribly complex problem. In a way, this is like premature optimization. I’m solving problems that I anticipate having rather than problems that I actually have right at this moment.

TDD is thus like a deep breath, a relaxing cup of some hot beverage, and perhaps a bit of meditation or zoning out. All of this noise fades out of my head and I can actually start figuring out how things work. The reason for this is that the essence of TDD is breaking the task into small, manageable problems and solving those while simultaneously ensuring that they stay solved. It’s like a check-list. Forget all of this crap about iterators and design patterns — let’s establish a base case: “Score at the beginning of a game is zero.” That’s pretty easy to solve and pretty hard to get frantic over.

With TDD, the first that I’m going to do is write a test and I’m going to write only enough of a test to fail (which includes writing something that doesn’t compile. So, let’s do that:

[TestClass]
public class Constructor
{
    [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
    public void Initializes_Score_To_Zero()
    {
        var scoreCalculator = new BowlingScoreCalculator();
    }
}

Alright, since there is no such type as “BowlingScoreCalculator”, this test fails by virtue of non-compiling. I then define the type and the test goes green (I’m using NCrunch, so I’m never actually building or running anything — I just see dots change colors). Next, I want to assert that the score property is equal to zero after the constructor executes:

[TestClass]
public class BowlingTest
{
    [TestClass]
    public class Constructor
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Score_To_Zero()
        {
            var scoreCalculator = new BowlingScoreCalculator();

            Assert.AreEqual(0, scoreCalculator.Score);
        }
    }
}

public class BowlingScoreCalculator
{
    public BowlingScoreCalculator()
    {

    }
}

This also fails, since that property doesn’t exist, and I solve this problem by declaring an auto-implemented property, which actually makes the whole test pass. And just like that, I’ve notched my first passing test and step toward a functional bowling calculator. The game starts with a score of zero.

What’s next? Well, I suppose the simplest thing that we should say is that if I bowl a frame with a score of zero for the first throw and one on the second throw, my score is now 1. Okay, so what is a frame, and what kind of data structure should I use to represent it, and what would be the ideal API for that, and is score really best represented as a property, and — BZZT!! Stop it. We’re solving small, simple problems one at a time. And the only problem I have at the moment is “lack of failing test”. I’m going to quickly decide that the mechanism for bowling a frame is going to be BowlFrame(int, int) so that I know the name of my next sub-test class for naming purposes. Writing code until something fails, I get:

[TestClass]
public class BowlingTest
{
    [TestClass]
    public class Constructor
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Score_To_Zero()
        {
            var scoreCalculator = new BowlingScoreCalculator();

            Assert.AreEqual(0, scoreCalculator.Score);
        }
    }

    [TestClass]
    public class BowlFrame
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_0_And_1_Results_In_Score_1()
        {
            var scoreCalculator = new BowlingScoreCalculator();
            scoreCalculator.BowlFrame(0, 1);
        }
    }
}

public class BowlingScoreCalculator
{
    public int Score { get; set; }

    public BowlingScoreCalculator()
    {    

    }
}

This fails because there is no “bowl frame” method, so I add it. CodeRush’s “declare method” refactoring does the trick nicely, populating the new method with a thrown NotImplementedException, which gives me a red test instead of a non-compile. I have to make the test pass before moving on, so I delete it by deleting the throw, and then I add an assert that score is equal to 1. This fails, and I make it pass:

[TestClass]
public class Constructor
{
    [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
    public void Initializes_Score_To_Zero()
    {
        var scoreCalculator = new BowlingScoreCalculator();

        Assert.AreEqual(0, scoreCalculator.Score);
    }
}

[TestClass]
public class BowlFrame
{
    [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
    public void With_Throws_0_And_1_Results_In_Score_1()
    {
        var scoreCalculator = new BowlingScoreCalculator();
        scoreCalculator.BowlFrame(0, 1);

        Assert.AreEqual(1, scoreCalculator.Score);
    }
}

public class BowlingScoreCalculator
{
    public int Score { get; set; }

    public BowlingScoreCalculator()
    {

    }

    public void BowlFrame(int throw1, int throw2)
    {
        Score = 1;
    }
}

Now, with a green test, I have the option to solve problems in existing code (i.e. “refactor”). For instance, I don’t like that I have a useless, empty constructor, so I delete it and note that my tests still pass. This is another easy problem that I can solve with confidence.

From here, I think I’d like a non-obtuse implementation of scoring that first frame. I think I’ll test that if I bowl a 2 and then a 3, the score is equal to five. For the first time, I’m not going to have any non-compiling failings, so I write some code and see red, obviously, which I get rid of by making the whole thing look like this:

[TestClass]
public class BowlingTest
{
    [TestClass]
    public class Constructor
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Score_To_Zero()
        {
            var scoreCalculator = new BowlingScoreCalculator();

            Assert.AreEqual(0, scoreCalculator.Score);
        }
    }

    [TestClass]
    public class BowlFrame
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_0_And_1_Results_In_Score_1()
        {
            var scoreCalculator = new BowlingScoreCalculator();
            scoreCalculator.BowlFrame(0, 1);

            Assert.AreEqual(1, scoreCalculator.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_2_And_3_Results_In_Score_5()
        {
            var scoreCalculator = new BowlingScoreCalculator();
            scoreCalculator.BowlFrame(2, 3);

            Assert.AreEqual(5, scoreCalculator.Score);
        }
    }
}

public class BowlingScoreCalculator
{
    public int Score { get; set; }

    public void BowlFrame(int throw1, int throw2)
    {
        Score = throw1 + throw2;
    }
}

All right, the calculator is now doing a good job of handling a low scoring first frame. So, let’s correct a little thing or two with the refactor cycle, making sure that whatever we do is a problem that’s easily solvable, isolated, and small in scope. I’m thinking I don’t like the magic numbers 2, 3 and 5 in that last test. Let’s try making it look like this:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void With_Throws_2_And_3_Results_In_Score_5()
{
    var scoreCalculator = new BowlingScoreCalculator();
    const int firstThrow = 2;
    const int secondThrow = 3;
    scoreCalculator.BowlFrame(firstThrow, secondThrow);

    Assert.AreEqual(firstThrow + secondThrow, scoreCalculator.Score);
}

Yes, it’s perfectly acceptable to refactor your unit tests during the “refactor” phase. Treat these guys as first class code and keep them clean or nobody will want them around. I have another bone to pick with this code, which is the duplication of the constructor logic in each test. Let’s fix that too:

[TestClass]
public class BowlFrame
{
    private static BowlingScoreCalculator Target { get; set; }

    [TestInitialize()]
    public static void BeforeEachTest()
    {
        Target = new BowlingScoreCalculator();
    }

    [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
    public void With_Throws_0_And_1_Results_In_Score_1()
    {
        Target.BowlFrame(0, 1);

        Assert.AreEqual(1, Target.Score);
    }

    [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
    public void With_Throws_2_And_3_Results_In_Score_5()
    {
        const int firstThrow = 2;
        const int secondThrow = 3;
        Target.BowlFrame(firstThrow, secondThrow);

        Assert.AreEqual(firstThrow + secondThrow, Target.Score);
    }
}

There, that looks better to me now. Now, what’s wrong with the production code? What problem can we solve? I can think of a few things: we can bowl more than 10 per frame, we can bowl negative numbers, and this thing will only keep track of the most recent frame. There are many other issues as well, but we’re already trending toward overload here, so let’s get back on Easy Street and figure out what to do if the user gives us goofy input.

I write a test that fails:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Throws_Exception_On_Negative_Argument()
{
    ExtendedAssert.Throws(() => Target.BowlFrame(-1, 0));
}

And then make it pass (first by declaring the nested exception and then by throwing it when the first frame is negative). I also take a shortcut in terms of obtuseness of my TDD here and make it throw the exception if either parameter is negative. I consider this acceptable, personally, since I believe that within the red-green-refactor discipline it’s a matter of some discretion how simple “the simplest thing” is. I’m not going to be accepting negative parameters for one and not the other and I know it. At any rate, after getting this to pass, I now refactor my method parameter names to be more descriptive and set the Score setter to private (which I should have done at first anyway) and the production class looks like this:

public class BowlingScoreCalculator
{
    public class FrameUnderflowException : Exception { }

    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new FrameUnderflowException();
        Score = firstThrow + secondThrow;
    }
}

Now I’m going to tackle the similar problem of allowing too much scoring for a frame. I want to solve the problem that scores of greater than 10 are currently allowed and I’m going to do this with the test case for 11. TDD is not a smoke testing approach nor is it an exhaustive approach, so I’m not going to write tests for 11, 12, 13… 200 or anything like that. As such, I try to hit edge cases whenever possible and that’s what I’ll do here:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Throws_Exception_On_Score_Of_11()
{
    ExtendedAssert.Throws(() => Target.BowlFrame(10, 1));
}

I make this pass, and then I go back and then decide I’m not a huge fan of that “10” in both my test and in the production class. Zero I can let slide because of it’s sort of universal value in scoring of competitions, but 10 deserves a descriptive name. I’m going to call it “Mark” since that’s what a frame of 10 is known as in bowling.

Now that we’ve sufficiently guarded against bad inputs, I’d say it’s time to start thinking about aggregation. The easiest way to do that is to have a frame of 1 and then another frame of one, and make sure that we have a score of 2:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_2_After_2_Frames_With_Score_Of_1_Each()
{
    Target.BowlFrame(1, 0);
    Target.BowlFrame(1, 0);

    Assert.AreEqual(2, Target.Score);
}

How cool is it that I fix this by changing “Score = firstThrow + secondThrow;” to “Score += firstThrow + secondThrow;” Talk about the simplest solution possible!

Now, I don’t like the way that test is written with 5 int literals in there. This is telling me that it might be time to rethink the way I’m handling frame. I don’t really want to think too much about it now, but I know that there’s going to be a hard-fail when I get to the 10th frame, which can have three throws so I’m already not married to this implementation. And now it’s already starting to feel awkward.

So what if I created a simple frame object? How would that look? Well, I’ll cover that next time as I flesh out this exercise. I’m hoping I’ve at least sold you somewhat on the notion that TDD forces you to chunk problems into manageable pieces and solve them that way without getting carried away. Next time I’ll circle back a bit to the idea of “magic boxes” as we decide how to divide up the concept of “frame” and “game” while adhering to the practice of TDD.