A TDD Demonstration
In a Twitter conversation a week or so ago, I was asked to do a TDD video, to show how I do it. A video isn’t within scope just now, but here’s an article with real TDDed code in it.
For best results, this article should be read in conjunction with #What TDD is – and isn’t – like. The other article discusses some common [mis]apprehensions about TDD, together with some of my experienced general thoughts. Here we have a trace of my actual work and such thoughts as I could write down, while building a small application.
I hope you find them both thought-provoking, if not actually useful.
Scoring Bowling
The TDD demo I’m about to do uses the problem that Chet and I generally use when giving demonstrations of TDD and pair programming. I’ll be doing it alone, so all the mistakes are surely Chet’s fault.
The problem, succinctly stated, is “Given all the rolls of a legal game of 10-pin bowling, compute the final score”. We’ve chosen this example because it’s just about the right size for an hour or ninety minutes of demonstration with discussion.
Note that there are important omissions in this problem statement, including:
- We do not have to check for errors;
- We do not have to calculate individual frame scores;
I’ll try to remember to talk about the extent to which these simplifications bias the work. Bottom line, I don’t think they do.
As my testing framework, I use #Jake Sankey’s very nice CodeaUnit. You’ll see how that’s used shortly.
When we do this demonstration live, we usually invite the audience to speculate about the classes and objects that are likely to arise during the development. The ideas often include:
- game
- ball
- pins
- frame
- rolls
- strike
- spare
- open
However, the rules of TDD as we apply them do not allow us to pre-judge whether we’ll have any of those classes or objects, although we do strive to have those concepts in our code, and to make them explicit as needed.
Let’s get on with it:
TDD Example - Bowling Game in Lua
hookup
We begin, as always, with a hookup test to be sure we’re connected to the testing framework.
function testBowling()
CodeaUnit.detailed = true
_:describe("Bowling First Tests", function()
_:before(function()
-- Some setup
end)
_:after(function()
-- Some teardown
end)
_:test("Hookup", function()
_:expect( 2+2).is(3)
end)
end)
end
This test fails, saying
1: Hookup -- Actual: 4, Expected: 3
I’ll fix the implementation (in the test) to 2+1, and it comes back saying
1: Hookup -- OK. Life is good.
Outside-In
You’ll notice as we go forward, that I write no tests specifically aimed at the implementation of my objects. I only test their behavior. I allow incremental improvements to their behavior to draw out the implementation.
For example, at one point in what follows, during refactoring, I pull out some common calculations into a new method. I don’t write a test for that method: my existing tests test it, by continuing to get the right answer.
When people learn TDD, they often fall into a pattern of testing the implementation. Sometimes they’ll even test object creation or member variable accessors explicitly. We try to avoid this. We’ll test an accessor if it’s public, of course, because then it’s behavior. But private accessors will be driven out and tested by the testing of real object behavior.
We call them tests …
I noticed in the above that I refer to tests and testing. Well, it is called Test-Driven, so that’s natural. But as you’ll see, TDD is different.
In my old style of programming, I’d write the program. When I was confident that it worked, I’d write a few tests to make sure, usually stopping from boredom or the pressure of the schedule. Later on, of course, problems might arise, and I’d fix them. It always seemed like an oversight: “oh, I forgot that case”. I don’t recall going back and adding that test. After all, I had fixed the bug. No point testing something that works.
Sometimes a change to the requirements resulted in a change to my code, and sometimes old bugs would reappear. I don’t recall ever thinking that I should have saved that test.
TDD is different. We think of something simple the program doesn’t do. We write a test to see that in fact it doesn’t do it. We make minimal changes to the code so that it does do it. We run the tests all again, to be sure we haven’t broken anything in adding this new bit.
And … have you ever seen (or written) code that was intricate enough that you were afraid to change it, lest it stop working? I know I have. Thankfully it was a long time ago.
With TDD, each little increment of behavior gets its own test, showing first that it doesn’t work, then that it does. So when it is time to refactor or add new capabilities, we can make changes with the confidence that our tests are constraining the object, keeping it working.
Yes, we write things called tests, but it’s not “I wonder if this works”, it’s more like “let’s make this work now”. A very different outlook.
As you read what follows, see whether you can detect this attitude.
Gutter Balls
No, wait, a bit more philosophy …
One approach I try always to use when doing TDD is to write very simple tests, representing single improvements in behavior. I’d like to write as little code as possible to make each test run, so that the program just kind of ratchets forward, as a sum of tiny steps, all working.
It’s not always easy to do this, especially if one has one of those visions of a big chunk of code to solve some largeish problem. But I find that smaller steps work better for me, so I try to start small, and to stay alert for taking too big a bite later on.
I definitely recommend that if you’re just beginning to learn TDD, you try to take steps that seem stupidly tiny. Pay attention to what happens, and ramp things up slowly, to get a feeling for what the right size is for you.
And remember, we focus our tests on the external behavior of the program, not its internals.
OK, now, gutter balls.
In the spirit of simplicity, for this exercise I always start with a game consisting of all gutter balls, because it’s the easiest game to score. Since I don’t have any application code, my test will be the first user.
Our tests are our code’s first user.
This is an important aspect of TDD by the way. When we’re creating code that has some API, our TDD tests are the first user of that API. As such, we get early personal experience using our API. If it’s hard for us to test, that’s a good clue that the API ideas need work.
Let’s try this:
_:test("Gutter Balls", function()
local rolls = { 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
game = BowlingGame.new(rolls)
_:expect(game.score).is(0)
end)
Notice that I’ve made a couple of tentative design decisions in writing this first test. There will be a game, and a class named BowlingGame. Instances of that class will understand the method score()
. (Note also that I’m rusty in Lua, and there’s a mistake in that code. Note also that the tests will find it promptly.)1
The error message is “Attempt to index a null value: BowlingGame, which means that BowlingGame isn’t defined yet. No big surprise there. So I define the class:
BowlingGame = class()
Notice what tiny steps I’m taking. I could of course do more, for example setting up the response to the new message, and even returning a result. But the ritual, and I find it restful, is to write the minimum code needed to pass the test – or, in my case, to run the tests after every cohesive change to the code.
By “cohesive”, I mean not necessarily a single statement, but a single conceptual step in my mind, which may take a few statments. With TDD as I do it, it never takes very many statements. This step was “create the class”.
To no one’s surprise, now the error message is “Attempt to call a nil value: new”, which of course means that “new” isn’t defined.
And it shouldn’t be …
Wow, I’m rusty on Lua. You don’t call “new” in Lua, you just put the parameters on the class name when you instantiate. The code should be like this:
_:test("Gutter Balls", function()
local rolls = { 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
game = BowlingGame(rolls)
_:expect(game.score).is(0)
end)
Now I get a message that nil isn’t zero, which confuses me for a moment, until I real the expect
line again. Wow, I am really rusty! .score
is an access, not a message. It should be this:
_:test("Gutter Balls", function()
local rolls = { 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
game = BowlingGame(rolls)
_:expect(game:score()).is(0)
end)
This produces the message I expect, namely that the method “score” isn’t defined on BowlingGame. Whew. Time to do some real work, calculating the score:
BowlingGame = class()
function BowlingGame:score()
return 0
end
Now, at last, we get “2: Gutter Balls – OK”. Time for a bit of discussion.
What didn’t happen?
Notice what I didn’t test. I didn’t test whether the creation call BowlingGame(...)
returns a BowlingGame. If it had accessors, I wouldn’t test those. I’m testing real behavior, the useful API of the object, and I’m not going to write any code that’s not required by my tests. So if it has accessors or member variables, or any random items like that, they’ll appear because of tests of behavior, and they’ll be tested by tests of behavior.
Outside-in is the way, Not inside-out.
A common mistake of TDD beginners is to have some class in mind, and to drive out its inner design, as they imagine it should be, using tests. One can do that, but it is Not The Way. Outside in is the way, not inside out.
What just happened?
We’re testing the API, the outside-in behavior. But there are things worthy of further note.
There are (at least) two odd things that happened here. First, since I’m pretty rusty, I made a few mistakes that one would hope not to make every day. Interestingly enough, my simple test found them all. One of them, the use of a dot connector rather than a colon, is a mistake that I do frequently make. I haven’t figured out a way to avoid that yet. It bites me rather often, since other languages typically use dot rather than colon. Nonetheless, my tests always find it.
Second, you may have noticed that, although I created the new game by sending it an array of 20 gutter balls, I ignored that input entirely. I did that in the spirit of writing the simplest code that can pass my test. The simple thing is to ignore the input and just return zero.
Fake it til you make it.
This pattern is called “Fake it til you make it”, and is a standard early move in TDD. The first test is just barely enough to cause us to create the class and some of its protocol. Because we have thoughtfully chosen the simplest imaginable test, we can lay in the simplest imaginable solution.
You may wonder, if I was going to ignore the input, why I provided it? Well, first of all, if I didn’t provide it, then I’m adding a feature to the specification, about what happens if no input is provided. I don’t have that requirement. Second, the example given, of all gutter balls, has to work, and further tests will surely require access to the data, and the gutter ball test will need to continue to work.
This thinking will surely feel odd to you, but I commend to your attention, when learning TDD, to lean very far in the direction of doing things that are as simple as possible while still taking us forward. We trust that the next test will drive out more behavior. Why do we trust that? Because we’re going to write that test, and every test, with the exact purpose of driving out new behavior.
Open Frames
What’s the next test? Well, we need to drive out some actual scoring behavior, and the simplest test would be one or more open frames. Strikes and spares have special behavior, so we’ll save them for later.
We could do just one open frame, or more than one. My larger experience includes a common mistake building bowling systems: as I loop over frames, which must ultimately happen, I often forget to increment something and wind up scoring the same frame over and over. So if I create a game with all 5,4 in each of the ten frames, I might get the right answer without inspecting all the frames. I’m sure a later test would find that problem but I’d rather not have it.
Today, just for fun, lets roll two gutter balls, a 5,4 frame, then all gutter balls again:
_:test("Open Frame", function()
local rolls = { 0,0, 5,4, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(9)
end)
(Note that I added “local” on game. Another common mistake that I make in Lua is not to declare things local, and the default in Lua is global. I changed the other test as well, as you’ll see when I next print the whole test file.)
This test fails: “Actual: 0, Expected: 9”. No surprise there. We need to do two things: save the input, and then use it to get the answer. We’ll continue with the “Fake it til you make it” style. First, to save the data:
BowlingGame = class(rolls)
function BowlingGame:init(rolls)
self.rolls = rolls
end
function BowlingGame:score()
return 0
end
When you create a new instance, the parameter in the class definition is sent to the method init
, where you do what you wish. I wish to save the input array in the member variable rolls
.
This doesn’t break anything: I still get the same answer about zero not being nine. Now to add up the rolls to get the score:
BowlingGame = class(rolls)
function BowlingGame:init(rolls)
self.rolls = rolls
end
function BowlingGame:score()
local sum = 0
for i,roll in ipairs(self.rolls) do
sum = sum + roll
end
return sum
end
The for
loop there is how you iterate over an array in Lua. (It may be worth mentioning that array indices start at 1 in Lua. I mention it because it might bite me later, but I have high hopes that it won’t.)2
Anyway, that simple code sums the rolls in the array, and that’s the correct answer for a bowling game with no strikes or spares. Again, we’ve done the simplest thing that can do the job.
If you’re wondering why I don’t use map-reduce logic here, it’s because Lua doesn’t have it. On another day, maybe we’ll invent it, but today is not that day.
Refactor
Remember that the TDD mantra is RED/GREEN/REFACTOR, not just RED/GREEN. We need to look at this code and see if the design is good, and if not, improve it.
The main issue that comes to my mind is that the game of bowling defines the notion of a “frame”, which is one or two rolls (one if it’s a strike, otherwise two), and our code doesn’t have the frame notion.
We don’t need much here but we need something. I see some oddness coming, owing to Lua’s 1 origin, so I’m glad I just brought it up. I’ll put in some code and see if I like it. I suspect it may take some thinking and discussion. We’ll see:
function BowlingGame:score()
local sum = 0
for frame = 0,9 do
local start = 2*frame + 1
sum = sum + self.rolls[start] + self.rolls[start+1]
end
return sum
end
This works. As expected, I had to fiddle with the 1 origin of arrays, the “+ 1” in the definition of start
. Because I’m not very bright, it took me a time or two to get that right: my brain is very locked in on zero origins. Anyway, the tests are green again.
However, I don’t like the fact that the loop is zero to nine, frames are numbered from one to ten. So let’s refactor again:
function BowlingGame:score()
local sum = 0
for frame = 1,10 do
local start = 2*frame -1
sum = sum + self.rolls[start] + self.rolls[start+1]
end
return sum
end
OK, well, this better expresses the one to ten frames: at least better than zero through nine did.
Aren’t you being awfully picky here?
It may seem picky to be making tiny decisions such as 1-10 rather than 0-9, but changes like this easy to make, taking just a moment, and keeping the code a bit more clear pays off when we wind up with more and more of it, as one does. The best way I know is to keep the code as clean as I can. I go faster that way, and I also enjoy the pleasure of a job well done.
While we’re cleaning things up, I think I’d like to rename start
to frameStart
, so I’ll do that:
function BowlingGame:score()
local sum = 0
for frame = 1,10 do
local frameStart = 2*frame -1
sum = sum + self.rolls[frameStart] + self.rolls[frameStart+1]
end
return sum
end
That’s a bit better. What else? score
might be a better name than sum
. That has a bit of conflict with the name of the function, but maybe that’s OK. Let’s try it:
function BowlingGame:score()
local score = 0
for frame = 1,10 do
local frameStart = 2*frame -1
score = score + self.rolls[frameStart] + self.rolls[frameStart+1]
end
return score
end
OK, that’s a bit better, let’s move on. But first some commentary.
As I was saying …
It may seem a bit fiddly, worrying about these names and other such details. However, it takes less time to do these changes than it does to tell you about them, and it helps keep the code clean and expressing all my thoughts as I go along. I recommend that might want to try working this way for a while, until you get the rhythm of it, and see if you like it.
It’s also worth mentioning that if I weren’t writing this up, I’d possibly take bigger bites, because I’m trying here to be on my best behavior, and frankly I’d like to influence you to taking the smallest useful bites. That’s a key aspect of learning TDD in my view.
It’s also very much worth mentioning that when I take bigger bites, I often take too big a bite and need to step back and choose a simpler test, or a simpler change. Don’t hesitate to revert and start over with something simpler when you start dropping some of the idea balls you’re juggling.
Spare
Anyway, for now, let’s score a spare. I want at least one open frame – in fact let’s put one on each side of the spare, and one spare.
_:test("Spare", function()
local rolls = { 0,0, 5,4, 6,4, 1,3, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(24)
end)
I think 24 is right. 9, plus 10 + 1, plus 4. Yeah, sounds right. Of course the test will fail, and I expect it to fail with 23. Sure enough, it does. So now I have to implement the spare logic. Here’s one way:
function BowlingGame:score()
local score = 0
for frame = 1,10 do
local frameStart = 2*frame -1
local frameScore =
self.rolls[frameStart] + self.rolls[frameStart+1]
score = score + frameScore
if (frameScore == 10) then
score = score + self.rolls[frameStart+2]
end
end
return score
end
I save the frame sum in a temp (premature optimization)3 and after adding up the first two rolls, I check to see if they total 10, in which case I add in the next roll. The tests all run green. Woot!
Hey! A strike will break the heck out of that code!
Yes, a strike will indeed break that code. But we don’t have a test for that, yet, and all three of our tests run. We’ll do strike next. Don’t let the future impinge on your current implementation. We don’t implement speculatively:4 we rely on our tests to drive out necessary changes.
But first … it’s time to consider refactoring again, we’re at a “ Green Bar”. I don’t exactly love this code, but at the same time, I don’t see how to really improve it. In that case, it’s time to add another test, and see where the strike case leads us:
Strike
_:test("Strike", function()
local rolls = { 0,0, 5,4, 10, 1,3, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(27)
end)
That’s sure to fail in an interesting fashion, and it does. The message is “attempt to perform arithmetic on a nil value”. What’s going on here? Well, there are now only 19 rolls in the input, and we’re going through by 2’s, so the last iteration tries to add a nil into the score.
And that presents us with probably the most interesting problem in the entire application, the idea that we don’t always step through the rolls array by two. Sometimes – if it’s a strike – we only step by one, although we of course do add in three values, the strike roll and the two succeeding rolls. So that calls for us to initialize the frameStart
variable and increment it directly, in two different ways:
function BowlingGame:score()
local score = 0
local frameStart = 1
for frame = 1,10 do
if self.rolls[frameStart] == 10 then
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
+ self.rolls[frameStart+2]
frameStart = frameStart + 1
elseif self.rolls[frameStart] + self.rolls[frameStart+1] == 10 then
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
+ self.rolls[frameStart+2]
frameStart = frameStart + 2
else
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
frameStart = frameStart + 2
end
end
return score
end
Yes, Virginia, I got to this in only two tries. (The first time, I initialized frameStart to zero. Lua arrays start at one.) No, wait, I lied. It took three tries, because I preserved the old frameScore local by accident, and calculated the strike score into it and never used it.
If you can’t make it better yet, try making it a bit worse.
But I’ve done another trick here. I wasn’t really pleased with the code after the spare case, you’ll recall, so I let it get worse by writing everything out longhand, with some help from a bit of cutting and pasting. Now we have lots of duplication that we can get rid of.
Mind you, it’s rude to make the code worse and leave it, but often making it a bit more in-line or otherwise odd helps us identify improvements. Give it a try!
And there’s something interesting here! Notice that the score calculation for strike and spare are both the same: the sum of the three rolls starting at frameStart
. That’s interesting and we can of course remove that duplication. (There’s reason not to, and we’ll come back to that.)
Refactor
I think what I’ll do first is try to clean up all those self.rolls[blah]
things a bit, with a helper method.
local frameStart
function BowlingGame:roll(i)
return self.rolls[frameStart + i]
end
function BowlingGame:score()
local score = 0
frameStart = 1
for frame = 1,10 do
if self.rolls[frameStart] == 10 then
score = score
+ self:roll(0)
+ self:roll(1)
+ self:roll(2)
frameStart = frameStart + 1
elseif self.rolls[frameStart] + self.rolls[frameStart+1] == 10 then
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
+ self.rolls[frameStart+2]
frameStart = frameStart + 2
else
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
frameStart = frameStart + 2
end
end
return score
end
We had to promote frameStart to a member variable to make this even halfway decent. I only changed the strike case, just to see if I like it. I don’t like it much. How about a helper called rollSum()
that takes a range? Let’s try that …
local frameStart
function BowlingGame:rollSum(count)
local t = 0
for i = 0,count do
t = t + self.rolls[frameStart+i]
end
return t
end
function BowlingGame:score()
local score = 0
frameStart = 1
for frame = 1,10 do
if self.rolls[frameStart] == 10 then
score = score + self:rollSum(3)
frameStart = frameStart + 1
elseif self.rolls[frameStart] + self.rolls[frameStart+1] == 10 then
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
+ self.rolls[frameStart+2]
frameStart = frameStart + 2
else
score = score
+ self.rolls[frameStart]
+ self.rolls[frameStart+1]
frameStart = frameStart + 2
end
end
return score
end
Well, that’s kind of interesting, isn’t it? Let’s carry that all the way through and see what we think?
local frameStart
function BowlingGame:rollSum(count)
local t = 0
for i = 0,count-1 do
t = t + self.rolls[frameStart+i]
end
return t
end
function BowlingGame:score()
local score = 0
frameStart = 1
for frame = 1,10 do
if self.rolls[frameStart] == 10 then
score = score + self:rollSum(3)
frameStart = frameStart + 1
elseif self.rolls[frameStart] + self.rolls[frameStart+1] == 10 then
score = score + self:rollSum(3)
frameStart = frameStart + 2
else
score = score + self:rollSum(2)
frameStart = frameStart + 2
end
end
return score
end
There’s a change in there that you might not notice: the for loop goes from 0 to count-1, not 0 to count. Darn zero origins again! I’m honestly not sure why the tests passed with it the other way. Maybe I just didn’t notice the failure, but that seems unlikely. I’ll put it back and look more carefully.
Ha, vindicated. It did fail. So either I didn’t run the tests, or I didn’t look at the result. I suspect the former.
There’s a hot tip, by the way, even if the change can’t possibly break anything, run the tests. It takes no time and once in a while you’re not as smart as you think. Well, anyway I’m not.
We’re still at a green bar, so we can continue to refactor. The expressions in the if
and elseif
statements are a bit cryptic. Let’s extract strike
and spare
methods to do the checking:
BowlingGame = class(rolls)
function BowlingGame:init(rolls)
self.rolls = rolls
end
local frameStart
function BowlingGame:rollSum(count)
local t = 0
for i = 0,count-1 do
t = t + self.rolls[frameStart+i]
end
return t
end
function BowlingGame:spare()
return self.rolls[frameStart] + self.rolls[frameStart+1] == 10
end
function BowlingGame:strike()
return self.rolls[frameStart] == 10
end
function BowlingGame:score()
local score = 0
frameStart = 1
for frame = 1,10 do
if self:strike() then
score = score + self:rollSum(3)
frameStart = frameStart + 1
elseif self:spare() then
score = score + self:rollSum(3)
frameStart = frameStart + 2
else
score = score + self:rollSum(2)
frameStart = frameStart + 2
end
end
return score
end
So, what do we think? That’s pretty tight and clear. There are two lines each duplicated twice. We could fuss with that, maybe break the calculation of frameStart out separately? If we did that, then the score code might look like this …
if self:strike() then
score = score + self:rollSum(3)
elseif self:spare() then
score = score + self:rollSum(3)
else
score = score + self:rollSum(2)
end
… with the calculation of frameStart down below somewhere. Then we’d see that strike and spare are duplicated and we could combine into a single if strike or spare
. But that would be odd. Even though the calculation for a strike and spare frame accesses the same variables, it doesn’t feel that it should be the same. To keep those lines from needing to be collapsed, we could rely on one of two approaches.
We could just leave them separate, on the grounds that it makes the code more clear (or at least avoids making it more confusing). Or we could make the two lines actually different. I’ll go ahead and complete this separation of score from stepping and then we’ll look at that.
function BowlingGame:updateFrameStart()
if self:strike() then
frameStart = frameStart + 1
else
frameStart = frameStart + 2
end
end
function BowlingGame:score()
local score = 0
frameStart = 1
for frame = 1,10 do
if self:strike() then
score = score + self:rollSum(3)
elseif self:spare() then
score = score + self:rollSum(3)
else
score = score + self:rollSum(2)
end
self:updateFrameStart()
end
return score
end
I decided to embed the updating into a function, since frameStart is already a member value. Now we are faced with the duplication in strike and spare scoring. I’ve done bowling a million times, though I think never in Lua, so I can think of options, most of which I don’t like.
We could begin by calculating the sum of the first two rolls in a temp, then if we have a strike or spare, include the third. This masks the actual bowling rule, which is that a strike uses two bonus rolls, and a spare uses only one bonus roll. So I think that’s right out. What if we invented the notion of a bonus roll, something like this:
score = score + self:roll1() + self:bonus(1) + self:bonus(2)
No. That gets messy and the helper functions’ naming doesn’t help as much as I had hoped. (You can try it if you like, or maybe you have a better idea. That’s always an advantage of pairing, maybe your partner has a better idea.)
I’m still not satisfied with this solution, though I’m getting there. I’d like to remove that apparent duplication so that some later programmer isn’t temped to collapse those two clauses, because I feel they communicate something important.
I don’t see anything I like. But I do see something else that I don’t like. I made the decision to do the step update with a function call, updateFrameStart()
. Now look at the score()
code. It has that if
nest, and then a function call. The function is doing (at least) two things, and the Composed Method pattern suggests that we should make that more explicit. Let’s extract that if
nest. Here’s one way:
function BowlingGame:scoreIncrement()
if self:strike() then
return self:rollSum(3)
elseif self:spare() then
return self:rollSum(3)
else
return self:rollSum(2)
end
end
function BowlingGame:score()
local score = 0
frameStart = 1
for frame = 1,10 do
score = score + self:scoreIncrement()
self:updateFrameStart()
end
return score
end
Now, my original plan was to include the summing in a function updateScore()
but score isn’t visible to other functions and I was leery of promoting it. Then I remembered how one is really supposed to do these variables in Lua (no, seriously, I’m very stale on it). Let me try again:
BowlingGame = class(rolls)
function BowlingGame:init(rolls)
self.rolls = rolls
self.frameStart = 1
self.totalScore = 0
end
function BowlingGame:rollSum(count)
local t = 0
for i = 0,count-1 do
t = t + self.rolls[self.frameStart+i]
end
return t
end
function BowlingGame:scoreIncrement()
if self:strike() then
return self:rollSum(3)
elseif self:spare() then
return self:rollSum(3)
else
return self:rollSum(2)
end
end
function BowlingGame:spare()
return self.rolls[self.frameStart] + self.rolls[self.frameStart+1] == 10
end
function BowlingGame:strike()
return self.rolls[self.frameStart] == 10
end
function BowlingGame:updateFrameStart()
if self:strike() then
self.frameStart = self.frameStart + 1
else
self.frameStart = self.frameStart + 2
end
end
function BowlingGame:updateScore()
self.totalScore = self.totalScore + self:scoreIncrement()
end
function BowlingGame:score()
for frame = 1,10 do
self:updateScore()
self:updateFrameStart()
end
return self.totalScore
end
OK, this is rather highly factored, but that’s consistent with my personal coding standard. You might well prefer things a bit more in line. And you might have other ideas for making this better. In fact, I’m not out of ideas myself. I’m done typing for today, however, and will take another look tomorrow.
The main point at the moment, however, is that with just 8 tests including hookup()
, I’ve pretty much nailed bowling down, and I’ve tried almost a dozen design variations with no concern over breaking it. Not that I didn’t break it: I broke it often. But when I did, one or more tests failed, and told me what I had done wrong.
More Tests?
Now you might be concerned that these aren’t enough tests. I would bet that in fact they are sufficient to give us great confidence that the program now works.
We often find that observers will think they see a hole, and we always write a new test if someone proposes one. I’ll leave that to you at home. In our courses, as a bonus, Chet and I always do two more. First, the perfect game, twelve strikes in a row (ten strike frames plus two bonus rolls of ten each). That should score 300:
_:test("Perfect", function()
local rolls = { 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10}
local game = BowlingGame(rolls)
_:expect(game:score()).is(300)
end)
That runs correctly. We do one more test, and a story goes with it. Long ago, Jeff Langr and I were doing this demonstration at Qwest in Omaha, and after we did the perfect game, a class member told us that she was the Nebraska female bowling champion, and did we know that any game of alternating strikes and spares scores 200? We didn’t, so we implemented that test, and we always do in honor of that interesting bit of learning:
_:test("Alternating 200", function()
local rolls = { 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4}
local game = BowlingGame(rolls)
_:expect(game:score()).is(200)
end)
And that runs as well! That’ll do for today, I’ll sum up tomorrow. See you then!
Tuesday
Here we are, next day, and let’s take a look at the program and tests as they stand, and see what comes to mind.
function testBowling()
CodeaUnit.detailed = true
_:describe("Bowling First Tests", function()
_:before(function()
-- Some setup
end)
_:after(function()
-- Some teardown
end)
_:test("Hookup", function()
_:expect( 2+1).is(3)
end)
_:test("Gutter Balls", function()
local rolls = { 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(0)
end)
_:test("Open Frame", function()
local rolls = { 0,0, 5,4, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(9)
end)
_:test("Spare", function()
local rolls = { 0,0, 5,4, 6,4, 1,3, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(24)
end)
_:test("Strike", function()
local rolls = { 0,0, 5,4, 10, 1,3, 0,0, 0,0, 0,0, 0,0, 0,0, 0,0}
local game = BowlingGame(rolls)
_:expect(game:score()).is(27)
end)
_:test("Perfect", function()
local rolls = { 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 10}
local game = BowlingGame(rolls)
_:expect(game:score()).is(300)
end)
_:test("Alternating 200", function()
local rolls = { 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4}
local game = BowlingGame(rolls)
_:expect(game:score()).is(200)
end)
end)
end
Not much to see here. The tests seem pretty clear to me. What about the code?
BowlingGame = class(rolls)
function BowlingGame:init(rolls)
self.rolls = rolls
self.frameStart = 1
self.totalScore = 0
end
function BowlingGame:rollSum(count)
local t = 0
for i = 0,count-1 do
t = t + self.rolls[self.frameStart+i]
end
return t
end
function BowlingGame:scoreIncrement()
if self:strike() then
return self:rollSum(3)
elseif self:spare() then
return self:rollSum(3)
else
return self:rollSum(2)
end
end
function BowlingGame:spare()
return self.rolls[self.frameStart] + self.rolls[self.frameStart+1] == 10
end
function BowlingGame:strike()
return self.rolls[self.frameStart] == 10
end
function BowlingGame:updateFrameStart()
if self:strike() then
self.frameStart = self.frameStart + 1
else
self.frameStart = self.frameStart + 2
end
end
function BowlingGame:updateScore()
self.totalScore = self.totalScore + self:scoreIncrement()
end
function BowlingGame:score()
for frame = 1,10 do
self:updateScore()
self:updateFrameStart()
end
return self.totalScore
end
Well, I can imagine renaming strike
and spare
to isStrike
and isSpare
. Not much incentive felt there, but it might be better.
Looking at this:
function BowlingGame:scoreIncrement()
if self:strike() then
return self:rollSum(3)
elseif self:spare() then
return self:rollSum(3)
else
return self:rollSum(2)
end
end
We still have that duplication and, as we talked about, I’m motivated to remove all duplication but when we do that, it looks odd. Ah. Today I have a bit of a revelation.
I think the rules of the game actually are something like
if the first roll knocks down all the pins, a “strike”, the score shall be ten plus the sum of the next two rolls. otherwise, if the two rolls knock down all the pins, a “spare”, the score shall be ten plus the nexts roll. otherwise, an “open frame”, the score shall be the sum of the two rolls.
Our code does not embody the notion of the “ten”. If we’re going to try to preserve this spirit, maybe it should. I’ll try it and see what happens:
function BowlingGame:scoreIncrement()
local fs = self.frameStart
if self:strike() then
return 10 + self.rolls[fs + 1] + self.rolls[fs + 2]
elseif self:spare() then
return 10 + self.rolls[fs + 2]
else
return self.rolls[fs] + self.rolls[fs+1]
end
end
Hm. That says it better, gets rid of the duplication, and allows me to remove the rollSum
function entirely. A noticeable improvement in my opinion. What about this:
function BowlingGame:scoreIncrement()
local fs = self.frameStart
local rolls = self.rolls
if self:strike() then
return 10 + rolls[fs + 1] + rolls[fs + 2]
elseif self:spare() then
return 10 + rolls[fs + 2]
else
return rolls[fs] + rolls[fs+1]
end
end
Same thing but caches self.rolls. I think I like that even better. YMMV of course.
Questions and Topics
Looking at this code as it stands, I’m fairly happy with it.
More classes?
One might think it could be made simpler or more communicative if there was a Frame
class, so we had an array of frames and ask each one for its score. That would get amusing because of the need to look from one frame to the next, or next two for the bonus rolls. In our implementation, we finesse that by just pointing into the rolls array.
One might be able to do the same trick with a Frame
class, but I’m not seeing the advantage.
However, suppose we did decide to refactor to create an array of frames? Note that we would not need to change these tests at all. Each of them reflects behavior of the BowlingGame instance, and that desired behavior won’t change until the rules of bowling change.
We might wind up writing more tests, directly against Frame
or the code that creates the array, but these tests are good.
To me, the need to change a lot of tests tells me that the tests are testing implementation specifics, not object behavior. When that happens, as we say in the trade, “You’re doing it wrong”.
Too highly factored?
When I learned TDD, I learned it in the context of Smalltalk. In that language, the best code is highly factored into classes and lots of little methods. Often when you look to see how something is implemented in Smalltalk, it seems like you drill down and down and nothing much ever happens until someone at the bottom returns himself.
So a highly factored style is my preference, and it may not be yours. I think you can see that we could have left things more glommed together if we had wanted to, and we could even put things back in line if we prefer that. The tests only address two public elements, class creation and the score()
method. They’ll support any such changes we might undertake.
Validation
We define validation as outside our scope here. What if we had not done so? Well, the exercise would take even longer, and that’s not good: we’re over 5000 words already. We’d have the question of what to do if the array isn’t valid. Should we throw an exception? That always seems rude.
The style of development that I use, primarily learned when I was learning XP back 20+ years ago, is to write code that works when given good data, and protect it against bad data only near the edges of the application.
This works well when you do it: almost all your code is “happy path” only. It doesn’t work so well when you can’t trust a collaborating object. In that case, you have to do validation. And if you’ve ever done it, you know it’s rather a pain.
For validation in the case of bowling, I think I’d work toward a fairly simple state machine. It would start out knowing it had no frames and that it was at the beginning, so it would expect anything from zero to ten. If it got ten, it would accept a frame, and go back to the initial state.
If it didn’t get ten, it would expect 0 to 10 minus the previous roll. If the roll fell into that range, it would accept a two-roll frame, and go back to the initial state.
If it ran off the end of the array before getting to ten frames, that’s an error. If there were things left, that’s an error.
However, there would have to be some special logic for the tenth frame, to pick up the bonus balls if needed. So that would be a couple of additional states.
So … I’d try something like that. I don’t recall ever doing it, so I might well wind up somewhere else, but that’s my thinking right now.
For now, we’re not going there. If I decide to find out what happens, I’ll write another article.
Summing up
Don’t forget to read the companion article on what TDD feels like to me: I hope it gives an additional sense of what I think about as I’m TDDing.
The main reason why I do TDD is that, despite its even pace, it is the fastest way I know to create code that works and stays well designed going forward. I should also mention that I find that I feel far less tension when I TDD, because I am almost always confident that the program works.
If I take too big a bite, and start down a rat-hole, I begin to feel more tense, and that helps me to notice that I’m in trouble. The right thing is to revert out whatever test is too big, and start over. I try to do that, but sometimes I fall victim to the voice that tells me I’m a wimp if I give up now and surely just another minute or two and I’ll find the bug.
The results do not support that voice. When I revert and start a simpler test, I seem invariably to do it faster and better for having backed up a bit and started anew.
Maybe it’s a bit like when you’re stuck in mud or snow. It makes sense to back up and take another run at it.
All in all, TDD helps me keep my thoughts focused on fewer things at once, gives me confidence that the code is working and improving, and gets me to a finished decently-crafted result sooner.
I hope you’ll give it a try.
-
More than one mistake in fact. You can almost hear the rust flaking off my brain. ↩
-
His hopes were to be dashed. More than once. ↩
-
I expected to use
framescore
again, and then didn’t need it after all. It would have been better to let the duplication happen and then remove it, rather than speculate about what I was going to do a whole three lines from where I was. You’re probably smarter than I am, but keep your eyes open for whether these things happen to you. ↩ -
Except when we do, and apparently then we get it wrong. ↩