Based on a suggestion, I'm going to build the BowlingGame in yet another way: based on a Stream concept. This will be more fun for me than what I should be doing: I hope it's the same for you.

Rolls as a Stream

A recurring misunderstanding of the bowling rules winds up assuming that you can treat each roll once and only once. This turns out not to be the case. For some discussion of the rules, see The Bowling Game or Extending the Procedural Bowling Game. The latter has a nice picture of the bowling scoresheet as well.

The reality is that a given roll can count to the score in up to three different frames. If you throw three strikes in a row, that constitutes three frames. Then suppose you throw an open frame, a 5 and a 4. We might write that as (10^1), (10^2), (10^3), (5^4 4^4). Your score by frame would be calculated this way:

(10^1+10^2+10^3), (10^2+10^3+5^4), (5^4+4^4), or

(30), (29), (9).

However, imagining that the rolls could be a sequence that we could just read leads to some interesting ideas for the program. A posting by Roberto Lupi triggered me to try this. Similar ideas have come in from others, including Alistair Cockburn1. Roberto’s is the one that triggered me to do this, probably because he provided most of the asnwer.

Our Cunning Plan

We’ll treat the rolls as a continuous stream of integers. Each frame will use as many of those numbers as it wants, and consume as many as it can dispose of. We’ve had similar ideas before – arguably all the ideas come down to this, but we’ll try to do it this time with a Stream metaphor.

My plan is not to create an actual Stream subclass, but instead to build an object, RollStream, that has enough Stream behavior to satisfy our needs and to match the metaphor. As a Stream subclass, it would have to have more behavior than we’d like to provide.

I’m going to use the existing tests, shown below for review, plugged into a new class I’m making up, called StreamBowlingGame. There’s been some discussion about the different ways that Smalltalkers use windows, compared to the conventional Java / C# IDEs, so I’ll show you my screen from time to time. Here’s how it looks as I save the test setUp method with a reference to StreamBowlingGame:

image

First I’m going to list the tests for you. I’m tired of doing this manually, so I browsed for implementors of print…, found printMethod:on:, and used it to print the methods of BowlingTest to the Transcript. Here, rearranged just slightly, is what the windows on my screen look like now. You can see the browser with the method in it, the workspace where I executed the method, and the tail end of the printout on the Transcript at top left.

image

Anyway, here’s the code, pretty much right out of the Transcript. Oops, no. The selectors (method names) printed in random order: that’ll be the order in the method hash table. Let’s sort them instead. That’s pretty tricky in Smalltalk: Here’s the modified workspace:

image

That, by the way, is how “Reflection” really ought to work. Show me the code for listing all your methods in Java. Oh … Java doesn’t know your source code, does it? Never mind. Anyway, the tests:

setUp
    game := StreamBowlingGame new

testAllGutters
    20 timesRepeat: [ game roll: 0].
    self should: [ game score = 0]

testAllOpen
    10 timesRepeat: [
        game roll: 5.
        game roll: 4].
    self should: [ game score = 90]

testAlternating
    5 timesRepeat: [
        game roll: 10.
        game roll: 6.
        game roll: 4].
    game roll: 10.
    self should: [ game score = 200 ]

testComplexOpen
    | rolls rollSum |
    rolls :=  #( 1 1   1 2   2 1   2 3   3 2   1 4   4 1   1 5   5 1   1 6).
    rollSum := rolls inject: 0 into: [ :sum :each | sum + each ].
    rolls do: [ :each |
        game roll: each].
    self should: [ game score = rollSum]

testPerfect
    12 timesRepeat: [ game roll: 10 ].
    self should: [ game score = 300 ]

testSpare
    game roll: 4.
    game roll: 6.
    game roll: 5.
    game roll: 4.
    8 timesRepeat: [
        game roll: 0.
        game roll: 0].
    self should: [ game score = 24]

testStrike
    game roll: 10.
    game roll: 5.
    game roll: 4.
    game roll: 3.
    game roll: 0.
    7 timesRepeat: [
        game roll: 0.
        game roll: 0].
    self should: [ game score = 31]

There. Now they’re in alpha order, which is better than random. I’d prefer chronological by first time entered, but that would be more work than I’m up for right now.

Let's Get To It

My plan is to build the new code partly in TDD order, partly by intention. I’m not going to write the tests over, but I will make them run a few at a time. First, I need to find out what needs to be in this new class, StreamBowlingGame. If I had forgotten that I called for it, I could just run the tests and debug one of the failing ones. That gives this:

image

Oh, right, we need the class StreamBowlingGame. There are two clues there. One, UndefinedObject does not understand new, and two, the code is still there in the browser. I’ll define the class and put it in a new browser, just for fun. Later on I’ll show the screen layout I’m using again. For now, let’s just do the code. I’m not sure what the class has to do, so I’ll run the tests again. Now they say StreamBowlingGame doesNotUnderstand: #roll:. Oh, right. we have to accumulate rolls. Let’s just make an OrderedCollection and accumulate into it, as we’ve been doing:

Smalltalk defineClass: #StreamBowlingGame
    superclass: #{Core.Object}
    indexedType: #none
    private: false
    instanceVariableNames: 'rolls '
    classInstanceVariableNames: ''
    imports: ''
    category: 'RonBowling'

initialize
    rolls := OrderedCollection new

roll: anInteger
    rolls add: anInteger

Maybe we’re done. I’ll run the tests. Oops, now it doesNotUnderstand: #score. I’ll implement that to return zero, because I think it’ll make the gutterBalls test run:

score
    ^0

And sure enough, it does. 7 run, one passed, 6 failed, 0 errors. Now let’s get to work in earnest on scoring. The idea, recall, is that we’ll have some kind of a RollStream that we can read from. I’ll code that by intention, until my memory runs out, then use the tests to see what I forgot:

score
    ^self scoreStream: (RollStream on: rolls)

Then, with RollStream undeclared, I’m going to sketch a little bit of scoreStream, enough to make open frames and gutter frames pass (I hope):

scoreStream: aRollStream
    | total |
    total := 0.
    10 timesRepeat:  [total := total + self scoreFrame: aRollStream].
    ^total

scoreFrame: aRollStream
    ^aRollStream next + aRollStream next

Now then. Streams in Smalltalk are linear collections of arbitrary objects. They respond, among other methods, to #next, which returns the next object in the stream, and consumes it, so that it isn’t still there. Therefore our code for #scoreFrame: should handle regular two-ball frames just fine. If only it worked. I’ll run the tests to discover that there is no class named RollStream, and I’ll build it:

Smalltalk defineClass: #RollStream
    superclass: #{Core.Object}
    indexedType: #none
    private: false
    instanceVariableNames: 'collection'
    classInstanceVariableNames: ''
    imports: ''
    category: 'RonBowling'

class>>on: aCollection
    ^self new setCollection: aCollection

setCollection: aCollection
    collection := aCollection

OK, this guy needs to handle #next. We’d better index the collection. I’ll add an instance variable #index and init it:

Smalltalk defineClass: #RollStream
    superclass: #{Core.Object}
    indexedType: #none
    private: false
    instanceVariableNames: 'collection index '
    classInstanceVariableNames: ''
    imports: ''
    category: 'RonBowling'

setCollection: aCollection
    collection := aCollection.
    index := 0

Now I can implement #next:

next
    index := index + 1.
    ^collection at: index

Notice that I’m starting the index at zero, while Smalltalk collections start at 1. That’s so I an increment it and then return the result, making the #next code a bit nicer. I didn’t really think of that as cleanly as it looks here. I wrote zero thinking of C# collections, then started writing the #next method, noticed how grubby it was with post-incrementing, then observed that I conveniently had the zero there. Anyway, I expect gutter balls and open frames to work now. Let’s see. Well, not quite. Here’s what happened when I clicked into the debugger to see why everything was still failing:

image

I left the parens out of the scoring method. So I’ll fix it right in the debugger:

image

Note that the debugger is telling me, via the highlighting, what is going to be executed next if I proceed. I won’t, though, I’ll just run all the tests again, because I think two will surely pass now. Whoa! Three passed. Either something is wrong, or I have forgotten the tests. Oh, wait, now I remember, I have two open frame tests, one that was written to be a bit more weird because of that bug I had where one 9 turned into 90. Let me check:

testComplexOpen
    | rolls rollSum |
    rolls :=  #( 1 1   1 2   2 1   2 3   3 2   1 4   4 1   1 5   5 1   1 6).
    rollSum := rolls inject: 0 into: [ :sum :each | sum + each ].
    rolls do: [ :each |
        game roll: each].
    self should: [ game score = rollSum]

Yes, right. That one should work also. The failing ones are alternating, perfect, strike, and spare. Let’s make spare work next. Programming by intention:

scoreFrame: aRollStream
    self isSpare ifTrue: [^aRollStream next + aRollStream next + aRollStream peek ].
    ^aRollStream next + aRollStream next

Peek is the conventional name for asking a Stream to show you its first element, but not remove it from the stream. We’ll have to implement that:

RollStream>>peek
    ^collection at: index + 1

I expect this to make the spare test work. And it might have had I implemented the isSpare method. The tests reminded me. Here it is … uh oh. isSpare wants to look at the stream. Best way right now is to pass it in, but that’s a bit ugly, we might want to revisit it.

StreamBowlingGame>>scoreFrame: aRollStream
    (self isSpare: aRollStream)
        ifTrue: [^aRollStream next + aRollStream next + aRollStream peek ].
    ^aRollStream next + aRollStream next

isSpare: aRollStream
    ^aRollStream peek + aRollStream peekSecond = 10

I’m positing a new method on RollStream, #peekSecond, viz:

peekSecond
    ^collection at: index + 2

Now what about my tests? Yes, four pass. The failing ones are all about strikes. The solution is now obvious:

scoreFrame: aRollStream
    (self isStrike: aRollStream)
        ifTrue: [^aRollStream next + aRollStream peek + aRollStream peekSecond ].
    (self isSpare: aRollStream)
        ifTrue: [^aRollStream next + aRollStream next + aRollStream peek ].
    ^aRollStream next + aRollStream next

isStrike: aRollStream
    ^aRollStream peek = 10

I really expect all my tests to work now. And they do!! Here’s the code as it stands. First StreamBowlingGame:

initialize
    rolls := OrderedCollection new

isSpare: aRollStream
    ^aRollStream peek + aRollStream peekSecond = 10

isStrike: aRollStream
    ^aRollStream peek = 10

roll: anInteger
    rolls add: anInteger

score
    ^self scoreStream: (RollStream on: rolls)

scoreFrame: aRollStream
    (self isStrike: aRollStream)
        ifTrue: [^aRollStream next + aRollStream peek + aRollStream peekSecond ].
    (self isSpare: aRollStream)
        ifTrue: [^aRollStream next + aRollStream next + aRollStream peek ].
    ^aRollStream next + aRollStream next

scoreStream: aRollStream
    | total |
    total := 0.
    10 timesRepeat:  [total := total + (self scoreFrame: aRollStream) ].
    ^total

And now RollStream:

next
    index := index + 1.
    ^collection at: index

peek
    ^collection at: index + 1

peekSecond
    ^collection at: index + 2

setCollection: aCollection
    collection := aCollection.
    index := 0

Reflection

That went pretty smoothly, and very much like Smalltalk usually goes for me. I’m a bit casual, not thinking too deeply, even though working fairly far ahead. I run the tests frequently, and they catch me up on what I’ve missed. Now a really great programmer (I’m thinking of a fellow called CTips) might argue that I should be paying more attention, and that I shouldn’t need all that help from the computer, and that I should have the complete picture in my head before going on. Well, could be. But this works for me.

What about the overall design? Well, I like the #scoreStream: method a lot. It says what it means. And I like #scoreFrame: a fair amount, but I don’t like the need to pass the stream as a parameter to the isXXX: methods. That could be resolved by making the stream a member variable, and allowing the methods to look at it. But, as Mr Beck once asked me: “What do we call an object that is an input to a method?” I stared dumbly, then tried “Parameter?”. “Right,” he said. It’s always a judgment call. I do think this would be more clear:

scoreFrame: aRollStream
    self isStrike
        ifTrue: [^aRollStream next + aRollStream peek + aRollStream peekSecond ].
    self isSpare
        ifTrue: [^aRollStream next + aRollStream next + aRollStream peek ].
    ^aRollStream next + aRollStream next

But I’m not willing to change what we’ve got to get that improvement.

Another way to do it would be to let RollStream implement some of those methods. But that’s not where this design idea was meant to lead, in my opinion. What we’re exploring here is whether Stream behavior, extended to allow double peeking, would be a good way to build our system. If we decided that it was, we’d extend the PeekableStream class to support #peekSecond, and use a real Stream, getting rid of our RollStream class.

It may be worth mentioning the possibility of building specialized subclasses of base classes, such as making RollStream a real kind of Stream. It’s not hard to do this: we’d only have to implement “a few” methods to make it work. But it’s not always a good idea to make your personalized collections and streams be subclasses of the real collection and stream classes. This has at least these issues:

  1. Your new class winds up implementing and inheriting behavior that you don't want and that may not work, such as #atEnd or #rewind or #inject:into:.
  2. When the Smalltalk version changes, the details of implementation of your base classes may change, breaking your classes.
  3. It's usually more complex to do it that way.

I used to really groove on building specialized collection classes. I’ve come to believe that that’s sort of “Yellow Belt” behavior, and that someone more masterful wouldn’t do that. If there’s someone more masterful out there, I’d love to hear from them on this or anything at all really. In fact, I’d love to hear, and will try to learn from, all opinions.

Anyway, for now, we’re done. This little experiment results in what I think is a fairly neat design, combining a clear scoring algorithm with well known primitive ideas right underneath. On a given day, I could imagine building the bowling system that way. What do you think?

Just One More Question, Sir ...

I feel like Columbo. But still, we’re not done. Two classes kind of feel like too much, the RollStream is an interesting idea but things are perhaps too split out. I’m going to put them back together. When I do, my guess is that we’ll get a solution much like ones that have gone before, but just a bit better. Here’s the screen layout I’m starting with:

image

My plan is this: I’m going to change the scoreFrame method to be the way I want it, without the parameters. To make that work, I’ll move the RollStream inside and remove it as a parameter to either of the scoring methods. Then I’ll put the various next and peek methods into the StreamBowlingGame class, forwarding them to the RollStream. Then I’ll figure out how to remove the RollStream altogether, by using an indexer inside the StreamBowlingGame class. It will be kind of an inverse of Extract Class. Does that have a name? If it does, I can’t find it with a quick look. Anyway …

Smalltalk defineClass: #StreamBowlingGame
    superclass: #{Core.Object}
    indexedType: #none
    private: false
    instanceVariableNames: 'rolls rollStream '
    classInstanceVariableNames: ''
    imports: ''
    category: 'RonBowling'

score
    rollStream := RollStream on: rolls.
    ^self scoreStream    

scoreStream
    | total |
    total := 0.
    10 timesRepeat:  [total := total + self scoreFrame ].
    ^total    

scoreFrame
    self isStrike ifTrue: [^rollStream next + rollStream peek + rollStream peekSecond ].
    self isSpare ifTrue: [^rollStream next + rollStream next + rollStream peek ].
    ^rollStream next + rollStream next

There’s more to do, but the above is all I have the nerve to do in one go. Let’s check the tests … well, of course, they all error on isStrike and isSpare. Those should be:

isSpare
    ^rollStream peek + rollStream peekSecond = 10

isStrike
    ^rollStream peek = 10

Now how about those tests? Ha, they’re green. See what I mean about the tests reminding me what I have to do? Now to remove most of the references to rollStream. We’ll browse references:

image

My plan is to change all the rollStream xxx to self xxx, and implement the various xxx methods, like next, peek, and peekSecond. (I think that’ll be all.) The changed methods are:

isSpare
    ^self peek + self peekSecond = 10

isStrike
    ^self peek = 10

scoreFrame
    self isStrike ifTrue: [^self next + self peek + self peekSecond ].
    self isSpare ifTrue: [^self next + self next + self peek ].
    ^self next + self next

And the new methods are:

next
    ^rollStream next

peek
    ^rollStream peek

peekSecond
    ^rollStream peekSecond

The tests should run … and they do. Now we should be able to remove the RollStream class by adding an index, and properly implementing #next and the #peek sisters:

Smalltalk defineClass: #StreamBowlingGame
    superclass: #{Core.Object}
    indexedType: #none
    private: false
    instanceVariableNames: 'rolls index '
    classInstanceVariableNames: ''
    imports: ''
    category: 'RonBowling'

score
    index := 0.
    ^self scoreStream

next
    index := index + 1.
    ^rolls at: index

peek
    ^rolls at: index + 1    

peekSecond
    ^rolls at: index + 2

I expect this to work again. And it does! Now I can remove the RollStream class. Tests still run. Here’s the final version:

Smalltalk defineClass: #StreamBowlingGame
    superclass: #{Core.Object}
    indexedType: #none
    private: false
    instanceVariableNames: 'rolls index '
    classInstanceVariableNames: ''
    imports: ''
    category: 'RonBowling'

initialize
    rolls := OrderedCollection new

isSpare
    ^self peek + self peekSecond = 10

isStrike
    ^self peek = 10

next
    index := index + 1.
    ^rolls at: index

peek
    ^rolls at: index + 1

peekSecond
    ^rolls at: index + 2

roll: anInteger
    rolls add: anInteger

score
    index := 0.
    ^self scoreStream

scoreFrame
    self isStrike ifTrue: [^self next + self peek + self peekSecond ].
    self isSpare ifTrue: [^self next + self next + self peek ].
    ^self next + self next

scoreStream
    | total |
    total := 0.
    10 timesRepeat:  [total := total + self scoreFrame ].
    ^total

Let’s see what we think.

Reflection

Actually, I think this is my favorite implementation so far, but of course it’s also my most recent. I might do some things … move the #scoreStream code up to score, because #score now has no useful content. Rename #scoreFrame to #frameScore. I’m tempted to move the index init to #initialize, but leaving it where it is lets us call #score more than once.

What lessons can we draw here? I can think of a few: you might have more.

  1. Having a pair is always helpful. Many of the ideas in this series of articles have come from other folks.
  2. There's almost always a better solution, waiting to be found. A little refreshing pause will often turn up a better idea.
  3. Be aware when it's time to stop polishing. In a situation where we had a bowling game to ship, we would have stopped many articles ago. But in a situation where we continue to deal with and maintain the game, improvements like some of what we've done here would probably be worth making.
  4. Don't be afraid to experiment. Each of these articles represents less than an hour or programming. As such, we can afford to try out a lot of ideas. Perhaps not every idea, but many.

This has been fun. I feel that I’m done with Bowling for now at least. But there are lots of other things to talk about in Smalltalk, and in this series, Discovering Better Code. Stay tuned.