Let's try some more incremental improvement to the objects. Meanwhile, some people think I'm taking bites that are too big. Let's think about that as well.

Looking Both Ways

A couple of people have written to me, suggesting that I’m taking bites that are too big here. It doesn’t feel that way to me. One of them, who for sake of anonymity I’ll just call Jeff Grigg, had the audacity to suggest that my steps were going beyond my tests, owing to familiarity with the problem and the language. Well, it’s possible, so let’s keep an eye out for it. I’m also wondering if it’s a time thing. Maybe I can do something in Smalltalk in ten minutes and be comfortable away from running tests that long, where it would take longer in C# and I’d be uncomfortable. Another possibility is that lately I’ve been refactoring – not in tiny steps, I’ll admit, unless four or five lines of code are tiny steps – and so it’s less exploratory and more directed.

I really don’t know. We’ll just have to watch and see what happens.

Now I mentioned last night that I had an idea, because I thought I’d get to it. But I didn’t. Today I’m sitting on four ideas, all somewhat related, and I’ll at least mention them before setting off to do something. No promises: what we wind up with might not be any of these ideas, but my guess is that it will be much like them. The last three ideas came up in a chat with Chet this morning. Here goes:

  1. My thought last night was this: the Frame objects are each being handed their own little array of just the rolls they should consider. That determination is being made in the BowlingGame, not in the Frame. So ... what if each Frame was instead handed all the rolls, plus an integer telling the Frame where its first ball is?
  2. What if we build the Frames in BowlingGame, but we linked them together instead of putting them in an array. Instead of adding up their scores, we could send #gameScore to the first one, and he would answer self score + next score. That might be pretty nifty.
  3. We might build the Frames one at a time. What if the BowlingGame just created one Frame at the beginning, and as rolls came in, sent them all to it. That Frame would check if it had enough rolls to do its job, and if so, send the rest to its next Frame (creating it if need be). The rolls would ripple down, creating new frames as they went. Then we'd do #score as above.
  4. Or, how about this? If we had it set up like above, then Frames could know their score as soon as they had all the rolls they needed. What if, instead of saving the result, or returning it on request, they sent it back to the BowlingGame? The BowlingGame would then have a running total, which would be correct at the end of the game. And each Frame would know its score as soon as possible, which is interesting in light of the fact that people often ask what we'd do if we had to do incremental frame-by-frame scoring.

So those are the ideas. The first one is a bit different from the others: the last three seem like one flows from the other, but all are focused on having just the right amount of memory in each Frame, while in the first one, each Frame knows all the rolls, and just ignores most of them.

There’s also a neat little variation in the latter ideas. One way is to let the same roll be in more than one Frame. A strike Frame, for example, would hold on to three rolls, but would start passing rolls on after the first (10) roll. But another way, simpler perhaps, would be to just parse the rolls into Frames in the standard way, and to implement #score something like this:

  self strike ifTrue: [10 + next firstRoll + next secondRoll]

In other words, the strike code would ask the next Frame for its first and second rolls, to add them into itself. Now of course the next Frame might be a strike, which means that the #secondRoll method would have to be smart enough to look to the next next Frame.

Wow, these are all very wild and speculative, aren’t they? Very unlike that nose to the grindstone thing I do in C#, where I would rarely speculate about a really good object-oriented design for something, much less a design that’s actually interesting. Have I gone nuts here? Well, perhaps. But I favor a different explanation:

Smalltalk is so flexible and so relentlessly object-oriented that it makes experimenting with new designs much faster than it would be in C# or Java. That being true, it also invites a bit more explicit thinking about what one might do, because one knows that it wouldn’t be that hard to do it. Am I right? Let’s find out by doing some of these examples to see how long it takes to move from one to the next.

Passing All Rolls to Frames

One code smell in the current implementation is that the BowlingGame is deciding what the Frame should know: BowlingGame has all the strike and spare logic in it. It would seem that a Frame should know that, not the Game. Then using that information, the BowlingGame parcels out the rolls to the Frames. What’s up with that? The Frames should know when they’re happy. Let’s fix that, moving in the direction of idea 1, where each Frame has all the rolls, and an index pointing to the first roll of interest.

Here’s my cunning plan, such as it is. The BowlingGame is now consuming the rolls list as it goes. That has to stop. Instead, it will create each Frame with all the rolls, and giving it the starting index. But where will it get the starting index? The starting index for the first Frame, of course, is 1. After that, let’s have the BowlingGame ask the most recent Frame where the next Frame should start. That should push all much of the strike/spare logic into the Frame. Scoring will push the rest.

For the first Frame we don’t have a previous frame to ask. So my plan is to create the first one directly, then loop over the next 9. Here’s the current code:

frames
  ^(1 to: 10) collect: [ :frameIndex | Frame new: self frameArray ]

By intention, here’s what I want:

frames
  | frames |
  frames := OrderedCollection with:
    (Frame 
      rolls: rolls
      startAt: 1).
  9 timesRepeat: [ 
    frames add: (Frame
      rolls: rolls
      startAt: frames last nextFrameStart) ].
  ^frames

OK. We create an ordered collection with the first frame in it. I’m assuming a new constructor for Frame, #rolls:startAt:. Then, nine more times, we add a Frame with all the rolls, and with its start set to the recommended #nextFrameStart from the last frame in the list.

Smalltalk told me when I saved that that #rolls:startAt: and #nextFrameStart were new messages. I nodded. Now let’s run the tests and see if maybe it works. Mysteriously, it does not. All the tests get errors. (My guess is #rolls:startAt:. I’ll run debug to find out.) By golly, yes. Entering the debugger, I’ll define the new method, get the halt, and see what happens next.

rolls: arg1 startAt: arg2
  "Dummy method template.  This was installed by the debugger
  define method command."

  ^self halt

Well, now. The method really wants to look like this now:

rolls: aCollection startAt: anInteger
  ^self new
    setRolls: aCollection
    startAt: anInteger

This is rote application of a pattern from Mr Beck’s book. A constructor calls the constructor parameter method. This method doesn’t exist either, but we can proceed, get the new error, define the method and get to the halt in it:

setRolls: arg1 startAt: arg2
  "Dummy method template.  This was installed by the debugger
  define method command."

  ^self halt

Here, we want to set the instance variables of the object. This is a bit of a problem, because the object doesn’t happen to have these variables yet. Its definition is this:

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

It only has one instance variable, rolls. (And that one isn’t used the same way as we have in mind. In the current implementation, that variable holds just the few rolls of interest; in the new one, it will hold them all. And we have no startAt variable at all. Well, right in the middle of the debugger, I think I’ll just add that variable:

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

Smalltalk doesn’t complain. Now let’s define that method:

setRolls: aCollection startAt: anInteger
  rolls := aCollection.
  startAt := anInteger

Now this gives me a slightly troubling message. The compiler hasn’t picked up the definition of startAt. I’m not sure what happened there. I go over to the class browser and save the definition again. Anyway, now I’m on a message saying that #nextFrameStart isn’t defined. My plan for that is to get into the debugger, add the method, and change it to return 2. I expect that will make the gutter ball and open frame tests run. Let’s see. No, the debugger isn’t quite that smart. I’ll need to start again in the TestRunner, just pulling down the debug list again. That works, and lets me add the method:

nextFrameStart
  ^2

I expect two tests to run now, so I’ll drop the debugger and rerun the tests. Bummer, only one passed. Gutter balls worked, but test all open didn’t. The reason, of course, is that it got some huge score, because the scoring method in Frame adds up all the rolls it has. Let’s verify that by looking at the test failing:

image

Sure enough, the open frames game has been scored 900 (see the highlight where I did printIt) instead of 90. Each Frame counted all 90 points. I can make that work by changing the score method, which is inconveniently not in the stack. I could push a bit and use the debugger to get there, but instead I’ll just change the method in the Browser. The old code is:

score
  ^rolls
    inject: 0
    into: [ :sum :each | sum + each ]

and we want:

score
  ^(rolls at: startAt) + (rolls at: startAt + 1)

That should handle the open frame test. Let’s see … sure enough, two pass.

Isn't This a Lot of Refactoring?

We have redefined the Frame class to have different instance variables, and a different constructor. We have changed its score method. And we’re not done yet. Is this getting to be too big a bite? Well, I think not. We haven’t been particularly confused, though there was that moment when I expected to tests to run and only one did. But the cause and the change was obvious.

If I had had to do this in C#, frankly I would not have. It would be too much. Here, I’ve just typed a couple of methods and saved them. And I think … I just might be nearly done. I think all I need to do now to make the spare test run is to improve the score method to detect the spare and add in one more roll. And to make the strike test and the alternating tests run, I need to enhance the #nextStartAt: and score just a bit more. The spare is easier, because the frame size is still two in that case, so our fake value doesn’t have to be improved, just #score. Thusly:

score
  | score |
  score := (rolls at: startAt) + (rolls at: startAt + 1).
  self spare ifTrue: [ score := score + (rolls at: startAt + 2) ].
  ^score

I expect that to make the spare game run. Let’s see. Oops. Forgot to define #spare. That looks like this:

spare
  ^(rolls at: startAt) + (rolls at: startAt+1) == 10

But it doesn’t work! Bad sign: I’m surprised. This could be a signal that the bite I’ve taken was too big.

I took a quick look at the Frames being created, and the answer is obvious. #nextFrameStart shouldn’t be returning 2: that makes all frames start at 2, except the first. It should be startAt+2. Fixing that:

nextFrameStart
  ^startAt + 2

makes the spare code run, as I thought. Time for a pause for reflection.

Reflection

OK. I got a couple of surprises there. That’s enough to suggest that the bite I’m taking may be too big. On the other hand, I have three of the six tests running now, and I’m feeling confident rather than confused. I’m thinking that I’ll go ahead. But first, dinner. Zukey Lake Tavern again, I think. We’ll see if I change my mind when I get back. Wait right there …

… OK, I’m back. Zukey Lake Tavern has, quite fortunately, hired the chef from the Blue Martini, which went out of business this summer. This guy is good! He has a number of Cajun specialties that ZLT is adding to their menu from time to time. Tonight the soup was Gumbo, so I had that instead of fries with my burger. And some slaw, as well as the lettuce and tomato, so if anyone asks, I had some vegetables.

What about the program? Well, the big fool says to press on. We do need to remember to write another test, though, because our open frame test wasn’t complex enough and we didn’t catch that “2” defect. I’ll start with that test, even though I’m on a red bar. Because I’m not really on a red bar, I’m half-way through a refactoring.

Whoa. That would be bad, changing hats like that. I better not. Never mind, we’ll add the test later. Your job is to remind me. We’ll add it before moving to step 2 in the list.

Finishing Up

We have the gutterball test, the open frame test, and the spare test all working. They all rely on the frame size being 2, and we have beefed up the #score method to look for the spare and add in the bonus ball. The code isn’t very pretty but let’s get green first and then clean it up. I believe that as soon as we make any strike test work, they will all work. But there are two steps to that. We have to change the #nextFrameStart method to increment by 1 if it’s a strike (otherwise 2), and we have to change the scoring to detect that it’s a strike and add up the right number of balls. Two changes, very tricky. Here goes. First the #nextFrameStart. It looks like this now:

nextFrameStart
  ^startAt + 2

and we want it to add only one if the frame is a strike. Should be easy enough. I’ll extract that 2 as a separate method, #frameStep, then enhance it. Here’s the extract:

nextFrameStart
  ^startAt + self frameStep

frameStep
  ^2

Naturally, Smalltalk has the Extract Method refactoring built in. I just selected the 2, and called for the refactoring. Now to beef it up:

frameStep
  ^self strike
    ifTrue: [1]
    ifFalse: [2].

Note here that we have just one return (^), at the beginning of the statement. The #ifTrue:ifFalse: method is a conditional expression, returning 1 or 2 to the return statement. This is idiomatic Smalltalk.

Now #strike isn’t implemented. We could implement it to return false and see if our three tests still run. Let’s do that. The same three pass. Now for the complete implementation of #strike. (By the way: would you prefer the name #isStrike? I’m tempted to change it. Hold that thought. For now, we’re making things work.)

strike
  ^(rolls at: startAt) == 10

Easy enough. It’s a strike if the first roll is 10, otherwise not. (And this repeated use of (rolls at: 1) isn’t going to escape our attention much longer either.) Now, though, we need to make the #score method smarter. It needs to handle the strike. Right now it looks like this:

score
  | score |
  score := (rolls at: startAt) + (rolls at: startAt + 1).
  self spare ifTrue: [ score := score + (rolls at: startAt + 2) ].
  ^score

Now I’m tempted to just add a test for strike. But notice that I’ve initialized score to be the sum of two rolls, which isn’t quite the right algorithm for strike. In addition, we might want not to add up the rolls to get 10, as we do now, but to refer t0 a literal ten the way the rules do. For now, I’m going to make it work, then clean it up.

score
  | score |
  score := (rolls at: startAt) + (rolls at: startAt + 1).
  self spare ifTrue: [ score := score + (rolls at: startAt + 2) ].
  self spare ifTrue: [ score := score + (rolls at: startAt + 2) ].
  ^score

I think that’s actually right, despite the perhaps surprising duplication. We’ll talk about that, but let’s see if we’re green. Oops! I typed spare again. That duplication probably surprised you more than the second part. I meant, of course:

score
  | score |
  score := (rolls at: startAt) + (rolls at: startAt + 1).
  self spare ifTrue: [ score := score + (rolls at: startAt + 2) ].
  self strike ifTrue: [ score := score + (rolls at: startAt + 2) ].
  ^score

That works! The tests are green. I’m going to remove some unnecessary methods from BowlingGame, show you the complete listings, then we’ll reflect.

As We Stand

  
BowlingGame
-----------
initialize
  rolls := OrderedCollection new.

roll: anInteger
  rolls add: anInteger

frames
  | frames |
  frames := OrderedCollection with:
    (Frame 
      rolls: rolls
      startAt: 1).
  9 timesRepeat: [ 
    frames add: (Frame
      rolls: rolls
      startAt: frames last nextFrameStart) ].
  ^frames

score
  ^self frames 
    inject: 0
    into: [ :sum :each | sum + each score ].

Frame
-----
setRolls: aCollection startAt: anInteger
  rolls := aCollection.
  startAt := anInteger

nextFrameStart
  ^startAt + self frameStep

frameStep
  ^self isStrike
    ifTrue: [1]
    ifFalse: [ 2].

isStrike
  ^(rolls at: startAt) == 10

score
  | score |
  score := (rolls at: startAt) + (rolls at: startAt + 1).
  self isSpare ifTrue: [ score := score + (rolls at: startAt + 2) ].
  self isStrike ifTrue: [ score := score + (rolls at: startAt + 2) ].
  ^score

isSpare
  ^(rolls at: startAt) + (rolls at: startAt+1) == 10

Note that I did change #strike to #isStrike and #spare to #isSpare. All the weird methods are now gone from BowlingGame, and all the work is done in Frame. We only have a couple of complicated methods: #score in Frame, and #frames in BowlingGame.

Let’s reflect.

Bite-Sized, or Not?

So, how did that go? The steps were bigger than I’d take in C# or Java, and there were a couple of surprises. On the other hand, there were no big debugging sessions or long periods of confusion. Each surprise was followed immediately by “Oh, it’s that.” Now in C#, I’d go in smaller steps, but there again I would make small mistakes and follow them pretty quickly with “Oh, it’s that.” More of those mistakes might have been found by the compiler, while here they were found by my tests.

Which reminds me. A correspondent asked why I wouldn’t prefer the type checking that I get in C#. This would seem to be the perfect example of why I should want it. The answer is that to get those compiler messages in C#, I’d have to compile the whole program, which takes a noticeable time even on a program this small. In Smalltalk, you save the method and you’re done. And you do get a number of useful messages, such as the one that tells you something is completely undefined. So I get to my tests faster, at the cost of not getting some type checking messages. Well, here, we haven’t had any type-checking problems, so waiting for the compiles wouldn’t have helped me. My long experience with Smalltalk makes me confident that I’ll always prefer the tradeoff that way. Not to say that I never make type errors: I do. It’s just that I don’t want to pay long delays in every compile to get them.

So I’m not sure. I’ll bet that to the readers, it will seem like the bites were too big, and in fact I’ll pay a bit more attention myself to trying to go smaller. But since there was no confusion for long periods, or unexplained weirdness, I suspect that the bites seem larger to you because you’re thinking of how long they would take in Java or C#. In Smalltalk, the delays are shorter, so you can keep thoughts in your head long enough to get things done … even if you’re writing an article at the same time!

Bottom line, in the next phase I’ll see about smaller bites, but I’m not feeling like I messed up here. It felt about right to me.

Refactoring

Let’s address some needed refactoring. First of all, I see a bunch of duplicated (rolls at: x). I’m going to extract those to named methods firstRoll, secondRoll, and thirdRoll. I did it in #score, which winds up like this:

score
  | score |
  score := self firstRoll + self secondRoll.
  self isSpare ifTrue: [score := score + self thirdRoll].
  self isStrike ifTrue: [score := score + self thirdRoll].
  ^score

Definitions of the …Roll methods are left to the reader. They’ll show up in a complete listing later on.

Now, there are two issues with this method as it now stands. First, we notice significant duplication in the last couple of lines. Second, it initializes score oddly. I think I’ll just rework it to do three different things, and see if I like that better:

score
  self isStrike ifTrue: [ ^self firstRoll + self secondRoll + self thirdRoll].
  self isSpare ifTrue: [ ^self firstRoll + self secondRoll + self thirdRoll].
  ^self firstRoll + self secondRoll.

Well that’s certainly shorter. How about that remaining duplication? We could get rid of it in two different ways. One is to create a method #isMark that answers #isStrike||#isSpare. Or, we could reflect the difference between the situations this way:

score
  self isStrike ifTrue: [ ^10 + self secondRoll + self thirdRoll].
  self isSpare ifTrue: [ ^10 + self thirdRoll].
  ^self firstRoll + self secondRoll.

In the interest of expressing intent, and the rules of bowling, let’s do that. People always get nervous when I point out that strike and spare really score the same three balls. This isn’t the most compact code we can imagine, but it’s pretty clear.

Now, what about that frames method in BowlingGame:

frames
  | frames |
  frames := OrderedCollection with:
    (Frame 
      rolls: rolls
      startAt: 1).
  9 timesRepeat: [ 
    frames add: (Frame
      rolls: rolls
      startAt: frames last nextFrameStart) ].
  ^frames

That’s really pretty simple, but still a bit odd. How about this. First we’ll extract a method:

frames
  | frames |
  frames := OrderedCollection with: self firstFrame.
  9 timesRepeat: 
      [frames add: (Frame rolls: rolls startAt: frames last nextFrameStart)].
  ^frames

firstFrame
  ^Frame rolls: rolls startAt: 1

Then, to clean up that second creation, let’s add a new constructor to the Frame class, so that we can write this:

frames
  | frames |
  frames := OrderedCollection with: self firstFrame.
  9 timesRepeat: 
      [frames add: (Frame fromFrame: frames last)].
  ^frames

The #fromFrame: method will just use the existing constructor. We’ll have to provide an accessor for rolls, however:

fromFrame: aFrame
  ^self
    rolls: aFrame rolls
    startAt: aFrame nextFrameStart

rolls
  ^rolls

That should work. Let’s find out. Indeed, it does!

But the #frames method is still a bit ugly. It has that accumulating variable in it. This is Smalltalk: we should be able to do this in a single statement. How about this one:

frames
  ^(2 to: 10)
    inject: (OrderedCollection with: self firstFrame)
    into: [ :sum :each | sum add: (Frame fromFrame: sum last); yourself ]

Whoa! Was that one too weird for you? I shall explain. No, it is too much. I shall sum up.

Remember that in the original method, we started an ordered collection with firstFrame, then added in 2 through 10 more, each one constructed from the last. This code just does exactly that, using the #inject:into: method. We start the :sum variable off with the same starting collection. Each time through the loop, we add the same frame as before, Frame fromFrame: sum last. We refer to sum last, of course, because now the result is in :sum, not the frames temp. We might make that code a bit less opaque this way:

frames
  ^(2 to: 10)
    inject: (OrderedCollection with: self firstFrame)
    into: [ :frames :ignored | frames add: (Frame fromFrame: frames last); yourself ]

Now you’re probably just wondering about that #yourself. Let’s expand that code onto separate lines:

  frames
    add: (Frame fromFrame: frames last);
    yourself

The semicolon means that we are sending two consecutive messages to the frames variable. The first one adds, and the second one … answers the frames collection. That’s right: if you send #yourself to any object, you get the object back. Why? Because the #add: method answers the thing added, not the whole collection. That’s useful in other contexts, so the use of #yourself is common to do what we just did. We could have written this, instead:

frames
  ^(2 to: 10)
    inject: (OrderedCollection with: self firstFrame)
    into: [ :frames :ignored | frames add: (Frame fromFrame: frames last). frames ]

That would have the exact same effect. The first statement in the final brackets adds the new frame, then the frames variable reference makes it the new sum (:frames). But the way we have it is better idiomatic Smalltalk (if we assume that I know what good Smalltalk style is).

Either way, we have reduced that complex method with a temp and three statements down to one statement. I think I’d leave things where they stand. I’ll include a final listing, but first let’s reflect.

Reflection

We still owe a test, a more complex open frame example. Better do that while we’re thinking of it:

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]

OK, that was kind of fun. Instead of doing the math, I created an array of rolls, spaced so we can see the frames, then added them up. Done once, especially for a beginner, this might be a little tricky. If we created a lot of tests this way, less so. Anyway, we have another complex test. The most interesting thing about it is that we don’t know the answer, we just know that it works. What else?

On the xp list, Alistair Cockburn commented:

I confess I am one of those for whom recursion has never made any sense. It looks simple, but I always feel that someone has pulled the rug out from under me when I wasn't looking (I get this feeling also from some Smalltalk code when the Law of Demeter is applied very heavily and each method only calls the next method and the final method just returns itself --- I can never figure out where the computation is getting done!).

That really is a common feeling with Smalltalk. Each method only does some simple thing, mostly calling another method and the final method does something dumb like return either 1 or 2. Yet the job gets done. If you’re feeling a little bit that way now, be reassured that you are normal. Good idiomatic Smalltalk has tiny methods. And for a while, as they work, most newcomers find it hard to find their way around. Every method sends another message or two: doesn’t anyone ever do anything? It takes a while to get used to it.

That said, the program is looking to me pretty much like it should. There are two classes, BowlingGame and Frame, and neither one does much of anything, and neither one knows much about the other. I think it’s pretty neat. And it wasn’t hard, and I didn’t get confused or go down a rathole. I think we’ll ship it, then do something more with it in the next article, just for fun.

Keep those cards and letters coming!

The Code As It Stands

  
BowlingGame
-----------
initialize
  rolls := OrderedCollection new.

roll: anInteger
  rolls add: anInteger

score
  ^self frames 
    inject: 0
    into: [ :sum :each | sum + each score ].

frames
  ^(2 to: 10)
    inject: (OrderedCollection with: self firstFrame)
    into: [ :frames :ignored | frames add: (Frame fromFrame: frames last); yourself ]

firstFrame
  ^Frame rolls: rolls startAt: 1

Frame
-----
class>>rolls: aCollection startAt: anInteger
  ^self new
    setRolls: aCollection
    startAt: anInteger

class>>fromFrame: aFrame
  ^self
    rolls: aFrame rolls
    startAt: aFrame nextFrameStart    

setRolls: aCollection startAt: anInteger
  rolls := aCollection.
  startAt := anInteger

rolls
  ^rolls

nextFrameStart
  ^startAt + self frameStep

frameStep
  ^self isStrike
    ifTrue: [1]
    ifFalse: [ 2]

score
  self isStrike ifTrue: [ ^10 + self secondRoll + self thirdRoll].
  self isSpare ifTrue: [ ^10 + self thirdRoll].
  ^self firstRoll + self secondRoll.

firstRoll
  ^rolls at: startAt

secondRoll
  ^rolls at: startAt + 1

thirdRoll
  ^rolls at: startAt + 2

isSpare
  ^(rolls at: startAt) + (rolls at: startAt+1) = 10

isStrike
  ^(rolls at: startAt) = 10

Hey, look! Duplication! Change those last two:

isSpare
  ^self firstRoll + self secondRoll = 10

isStrike
  ^self firstRoll = 10

And the tests, of course, all run.