Chet and I get this version running all the way, by implementing the Strike functionality. We do some cleanup, and plan next steps. We can get a pretty good look at the OO-style version of bowling. Is it better? It's not clear.

Retrospective

Chet and I are now at Borders in Brighton: it’s Thursday morning. We’re catching up and seeing what needs to be done. I’m showing him the little bit of code I wrote, and we’re looking for what should be next. We agree that the true/false return from RollConsumed is confusing, obfuscatory, and uncommunicative.

We’re looking at Frame and noticing that it is keeping a list of rolls, which is no longer needed. It might be better to just accumulate the score. One issue that we “foresee” is that the Frame might be asked, for display purposes, whether it was a strike or spare or open, and it won’t know if we get rid of the list. On the other hand, we don’t have that requirement now, and if we did have it, it might be better to provide that information differently, using the Kickers. Let’s simplify, from:

  public class Frame {
    private ArrayList rolls;
    IKicker kicker;

    public Frame() {
      rolls = new ArrayList();
      kicker = new FirstRollKicker();
    }

    public void SetKicker(IKicker aKicker) {
      kicker = aKicker;
    }

    public void AddRoll(int pins) {
      rolls.Add(pins);
    }

    public bool RollConsumed(int pins) {
      return kicker.RollConsumed(this, pins);
    }

    public int Score() {
      int score = 0;
      foreach ( int pins in rolls ) {
        score += pins;
      }
      return score;
    }
  }

… to:

  public class Frame {
    private int score;
    private IKicker kicker;

    public Frame() {
      score = 0;
      kicker = new FirstRollKicker();
    }

    public void SetKicker(IKicker aKicker) {
      kicker = aKicker;
    }

    public void AddRoll(int pins) {
      score += pins;
    }

    public bool RollConsumed(int pins) {
      return kicker.RollConsumed(this, pins);
    }

    public int Score() {
      return score;
    }
  }

Tests run, and this is definitely simpler. We think that’s cool. What else do we see? We think we can clean up SecondRollKicker a bit by extracting a couple of methods, resulting in this:

  public class SecondRollKicker : IKicker {
    int firstRoll;

    public SecondRollKicker(int pins) {
      firstRoll = pins;
    }

    public bool RollConsumed(Frame frame, int secondRoll) {
      frame.AddRoll(secondRoll);
      SetKicker(frame, secondRoll);
      return true;
    }

    private void SetKicker(Frame frame, int secondRoll) {
      if (IsSpare(secondRoll))
        frame.SetKicker(new OneBonusKicker());
      else
        frame.SetKicker(new IgnoreKicker());
    }

    private bool IsSpare(int secondRoll) {
      return firstRoll + secondRoll == 10;
    }
  }

We talked about extracting the frame.SetKicker() calls from the other Kickers, but we don’t think that buys anything, though in this case we prefer it because of the logic in the Kicker. The same will probably happen in the Strike case.

We discussed briefly whether to make the kickers private classes of Frame. We don’t usually do that. We can certainly make them internal. Might do that later. As old Smalltalkers, we’re not as religious about public/private as some programmers are.

Chet observes that all the Kickers do much the same thing: they add in the roll (or not), change the kicker, and return the true/false indicator. It would be possible to give them a common superclass with a template method in it that did those three things, and leave the Kicker subclasses with just a few detail methods, one for each of those things.

This is in any case not the time to do that … we should wait until we have all the classes in. There’s just one more case to handle, the Strike. So we agree that we’ll put this off for a bit.

However, there’s another possibility. It might be possible to build a tabular data structure that represents all this. That might be tricky or difficult, since the selection of a new Kicker is dependent on the input. It might be a good way to build a state machine that isn’t so dependent on data but seems a bit tricky for what we’re doing. On the other hand, it would make for a more compact notation: some readers will find that looking at five or six classes is “too hard”. That feeling depends, I think, on what you’re used to. We think the code is more expressive than trying to put a little language into a table.

We removed the default constructors from all the classes that have it, just to make less code to read. Enough speculation and chit-chat. Let’s finish up and then assess how it all went.

Strike

Begin with a test. We’ll put a test in FrameTestFixture:

    [Test] public void Strike() {
      Assert.IsTrue(frame.RollConsumed(10), "Strike");
      Assert.IsFalse(frame.RollConsumed(4), "First Bonus");
      Assert.IsFalse(frame.RollConsumed(5), "Second Bonus");
      Assert.IsFalse(frame.RollConsumed(2), "Ignored next roll");
      Assert.AreEqual(19, frame.Score());
    }

Should be easy enough. Maybe it even works now … no such luck. Fails on the “First Bonus”, because as written, the Frame always gets two rolls. We need to beef up the FirstRollKicker. His code is:

    public bool RollConsumed(Frame frame, int pins) {
      frame.AddRoll(pins);
      frame.SetKicker(new SecondRollKicker(pins));
      return true;
    }

We need to give him a more sophisticated SetKicker method, as we did in SecondRollKicker. We’ll extract the existing SetKicker call and then expand, thus:

  public class FirstRollKicker : IKicker {

    public bool RollConsumed(Frame frame, int pins) {
      frame.AddRoll(pins);
      SetKicker(frame, pins);
      return true;
    }

    private void SetKicker(Frame frame, int pins) {
      if ( IsStrike(pins) )
        frame.SetKicker(new TwoBonusKicker());
      else
        frame.SetKicker(new SecondRollKicker(pins));
    }

    private bool IsStrike(int pins) {
      return pins == 10;
    }
  }

  public class TwoBonusKicker: IKicker {

    public bool RollConsumed(Frame frame, int pins) {
      frame.AddRoll(pins);
      frame.SetKicker(new OneBonusKicker());
      return false;
    }
  }

We have the Frame working as intended … let’s check the game with a few new tests. I think that the alternating strike/spare test might suffice:

    [Test] public void Alternating() {
      RollAll( new int[] { 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10});
      Assert.AreEqual(200, game.Score());
    }

The test does not run – it runs off the end of the Frame array. The issue is that the tenth frame accumulates the final bonus ball (the last 10), but returns false, saying that it didn’t consume the ball. So our RollConsumed loop looks for a frame who wants to consume the ball. Either we need a Sentinel Frame in the 11th position, to ignore everything but say that it ate it, or we need a specialized tenth Frame to consume the bonuses.

Chet objects to the 11th Frame idea, because that’s when you go for beer, not something you score. He thinks if we add an 11th Frame, the US Bowling Association will shoot out my front porch light. He has a simple idea for a TenthFrame. We’ll subclass Frame and change RollConsumed to always return true, saying that the ball was consumed. We’ll have to TDD that class, I guess, and it will make initializing the frames array a bit trickier. Should be no problem …

Well, it was, primarily because it took us eleven tries to remember how to convince C# that you want to override a method of the same name in a subclass. Thanks, Anders. Anyway …

  public class TenthFrame : Frame {

    public override bool RollConsumed(int pins) {
      kicker.RollConsumed(this, pins);
      return true;
    }
  }

All the tests now run. I really don’t think we need any more, since Alternating checks just about everything, but just for fun I’ll do an AlternatingSpareFirst:

    [Test] public void AlternatingSpareFirst() {
      RollAll( new int[] { 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6,4, 10, 6, 4});
      Assert.AreEqual(200, game.Score());
    }

That runs as well. We’ll see what Chet thinks, but I think we’re done.

Chet agrees: we’ll break for lunch.

Session Retrospective

Over lunch, we discussed what we’ve learned so far and what to do next. By removing redundant constructors and the like, the code is now down to about 160 lines, which is still well over twice the size of the procedural version. And is it better? We’re not convinced.

It might be argued that the procedural version will be harder to enhance than the OO version. We’re also not convinced of that, and we plan to try it in a while. The experiment will be to set each version up so that the frames could be displayed in a GUI, a bit like a bowling scoring machine would do. Our discussion makes us think it’ll be easy in either case, but actually slightly easier in the procedural case. We’ll see.

We also want to try two different ways of reducing the code. One will be Chet’s suggestion that I mentioned earlier: build a hierarchy of Kicker objects, and remove duplication up to the superclass. That will surely work, and will probably be fun. We’ll learn something.

The other idea is to use C# delegates. This idea was something we talked about before we did the Kicker classes, but we didn’t see how to do it. A recent e-mail from Doug Swartz of First Data’s XP team, with a Smalltalk implementation, got us thinking. Doug used the Smalltalk #become: method to change the actual class of the Frame object at run time. (Yes, you can do that in Smalltalk.) “Kicking” that around made us think that the same thing could probably have been done with a #perform: operation, which is Smalltalk’s neat and easy way of doing what C# delegates do with difficulty. But the delegate approach should still be simple, and it should collapse our number of classes back down.

So in upcoming articles we’ll try most of these things:

  1. Remove duplication in the Kickers by building a class hierarchy;
  2. Remove Kicker classes using delegates;
  3. Sketch a GUI interface for both procedural and OO versions of the game.

Should be fun … stay tuned. Keep those cards and letters coming!