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.
[…] use the example of a method that identifies numbers as prime or not, and in a series of posts I did last fall on TDD, I use the example of something that calculates a bowling score. I’ve also done other […]
I keep rereading this series to more closely examine your process. During one of my rereads I came up with a question. The question is about an alternate design for the score calculator. It would be too difficult to explain, so I modified your code to show you what I’m thinking. I put the changed files up on gist if you’d like to see: https://gist.github.com/TheSecretSquad. I’m not in any way advocating my design over yours. In fact, I have the usual feeling that there is something blatantly bad about my design. I’m trying to get a better “design sense”, so… Read more »
I’m thinking that I might write a post about this concept, but I’ll see how involved the comment gets 🙂 Also, as kind of disclaimer, this is code I wrote a year and a half ago and haven’t really looked at or thought about much since. On the subject of how I divide class responsibilities, what I try to do is let myself be guided by the “Principle of Least Surprise.” Often, this is achieved by writing classes that mimic real world objects along with their behaviors and dependencies. A “Car” class has an “Engine” it starts when a client… Read more »
I couldn’t ask for a more coherent answer. I’m considering framing this and hanging it on the wall. Thank you.
Glad it helped!