I was glancing at the delegates version of the code last night, and I have the impression that the frame status requirement will be more readily implemented there. So today we give that a try. The results are strikingly better than yesterday's!

Existing Code

If you’d like to review the existing code, see Delegates Code, pre-Status.

We began with a little rearranging of the code you just looked at – or didn’t look at – and we removed the AddRoll method, which isn’t used any more. (Thanks to Carl Manaster, the first to point this out!) Other than that, we moved the delegates down into their own region, and arranged the few real methods of the class into a more logical order. (I like to put the primary public methods first, then the ones they call after them. You’ll see what I mean as we display the code.)

Then, we decided just to bring over our existing FrameStatus tests and make them run, since we want all our experiments to be solving the same problem inasmuch as possible. That will save us a bit of time, but since the tests weren’t hard to write, it won’t save much. (With hindsight, I can tell you now that we refactored the tests enough before the session was over, that we surely spent at least as much time on them as yesterday.) Here they are:

  [TestFixture] public class FrameDisplayTestFixture {
    Frame frame;
    FrameStatus frameStatus;

    public FrameDisplayTestFixture() {
    }

    [SetUp] public void SetUp() {
      frame = new Frame();
    }

    [Test] public void Strike() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.IsNull(frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsFalse(frameStatus.Complete);
    }

    [Test] public void StrikeSecondRoll() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.IsNull(frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsFalse(frameStatus.Complete);
    }

    [Test] public void StrikeThirdRoll() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.AreEqual("19", frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsTrue(frameStatus.Complete);
    }

    [Test] public void Spare() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("/", frameStatus.Symbol);
      Assert.AreEqual("13", frameStatus.Score);
      Assert.AreEqual("6", frameStatus.FirstRoll);
      Assert.AreEqual("4", frameStatus.SecondRoll);
      Assert.IsTrue(frameStatus.Complete);
    }
  }

  [TestFixture] public class OpenFrameDisplayTestFixture {
    Frame frame;
    FrameStatus frameStatus;

    [SetUp] public void SetUp() {
      frame = new Frame();
      frame.RollConsumed(4);
      frame.RollConsumed(5);
      frameStatus = frame.FrameStatus();
    }

    [Test] public void SymbolIsDash() {
      Assert.AreEqual("-", frameStatus.Symbol);
    }

    [Test] public void ScoreIsNine() {
      Assert.AreEqual("9", frameStatus.Score);
    }

    [Test] public void FirstRollIsFour() {
      Assert.AreEqual("4", frameStatus.FirstRoll);
    }

    [Test] public void SecondRollIsFive() {
      Assert.AreEqual("5", frameStatus.SecondRoll);
    }

    [Test] public void StatusIsComplete() {
      Assert.IsTrue(frameStatus.Complete);
    }
  }

Naturally, these won’t even compile yet. They’re calling for a FrameStatus object and a bunch of properties on it. We’ll create that, though we could of course just use the existing one. We want to feel the pain of developing what we need, at least in the code itself, because we’re here to compare the various approaches.

We let Resharper add the FrameStatus() method to Frame, fill in a few blanks in Frame, and create the FrameStatus class manually. (Resharper always makes private classes and we don’t like that.) Now Resharper sees all those properties above, like Symbol and Score, and it will volunteer to help, giving, for example:

  public class FrameStatus {
    private string symbol;

    public string Symbol {
      get { return symbol; }
    }
  }

Repeat until done …

  public class FrameStatus {
    private string symbol;
    private string score;
    private string firstRoll;
    private string secondRoll;
    private bool complete;

    public string Symbol {
      get { return symbol; }
    }

    public string Score {
      get { return score; }
    }

    public string FirstRoll {
      get { return firstRoll; }
    }

    public string SecondRoll {
      get { return secondRoll; }
    }

    public bool Complete {
      get { return complete; }
    }
  }

That was very easy and mechanical. It speaks to the value of having the tests, to drive the code interface, and speaks to letting Resharper help out as much as possible. We just had to type in the data types, and the return statements, and a bunch of Alt+Enter to get Resharper to offer its next suggestion.

It looks like this will compile. Maybe it works too? We’ll see, but I’m betting not.

Darn. Not one. All the tests fail. Chet points out that with everything broken, we’ll be wishing all our tests were of the one-assert form. He’s thinking we should change them to that format. I’m not convinced. I’m resisting – it seems like work and I want to get the code working. We compromise, because it’s my computer. We’ll make the tests that are broken out run, and then see what to do with the ones that aren’t. So we’ll work on the open frame tests first: they’re up above for your review.

First one is the dash test, which is now returning null. Chet plans to fake it. I can tell that this is going to leave the code broken so badly that I’ll have to give in and break out the other tests. He can’t trick me but I’m going to go along with it. Fake the dash.

Ha! Note that we already created the FrameStatus class with properties that are returned by the getters. Therefore it’s suitable to init the fields in the constructor, rather than breaking the getters to return literals:

    public FrameStatus() {
      symbol = "-";
      score = null;
      firstRoll = null;
      secondRoll = null;
      complete = false;
    }

I put in the one I knew, symbol = “-“, and nulled the rest. The tests will help. Now we iterate fixing red bars – but that’s not helping much. Instead, let’s actually make the dash work, by putting appropriate code into the delegate methods.

The method that knows whether a frame is open is this one:

    private bool ProcessSecondRoll( int pins) {
      score += pins;
      if (score == 10) 
        processor = new ProcessRoll(OneBonusRoll);
      else
        processor = new ProcessRoll(IgnoreRoll);
      return true;
    }

Let’s extend the method this way:

    private bool ProcessSecondRoll( int pins) {
      score += pins;
      if (score == 10) {
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.Symbol = "-";
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

We let Resharper create the write property, and we changed the constructor to null everything:

  public class FrameStatus {
    private string symbol;
    private string score;
    private string firstRoll;
    private string secondRoll;
    private bool complete;

    public FrameStatus() {
      symbol = null;
      score = null;
      firstRoll = null;
      secondRoll = null;
      complete = false;
    }

    public string Symbol {
      get { return symbol; }
      set { symbol = value; }
    }

    ...

Now the next test is ScoreIsNine. Turns out that goes in the same method:

    private bool ProcessSecondRoll( int pins) {
      score += pins;
      if (score == 10) {
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.Symbol = "-";
        frameStatus.Score = score.ToString();
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

We foresee that there will be some duplication setting score every time we set the processor to IgnoreRoll. We’ll deal with that if it happens, even though we’re sure.

Have you noticed already that this is a hundred times easier than what we did yesterday? We sure have. We could do this left-handed. What’s that telling us?

This at least confirms yesterday’s theory that the Kicker classes were not the right solution, and that refactoring to the delegates was a better idea. This code may turn out to look a little procedural, but we’ll see what we think about that when we get there. Moving on, the test to fix is FirstRollIsFour, and the code is:

    private bool ProcessFirstRoll(int pins) {
      frameStatus.SetFirstRoll(pins);
      score += pins;
      if (score == 10)
        processor = new ProcessRoll(TwoBonusRoll);
      else
        processor = new ProcessRoll(ProcessSecondRoll);
      return true;
    }

    private bool ProcessSecondRoll( int pins) {
      score += pins;
      if (score == 10) {
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.Symbol = "-";
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

We didn’t like having to convert the pins and score to string on the caller side, so we built a setter method for each, and converted in there:

  public class FrameStatus {
    ...
    public string Score {
      get { return score; }
    }

    public void SetScore(int pins) {
      score = pins.ToString();
    }

    public string FirstRoll {
      get { return firstRoll; }
    }

    public void SetFirstRoll(int pins) {
      firstRoll = pins.ToString();
    }

    ...
  }

We may revisit that as time goes on but it feels like it keeps the data types of FrameStatus internal to it, while accepting the natural data types of Frame from the outside. We could change symbol similarly, though it isn’t necessary since it’s a string. Chet suggests that it’s more consistent to set them all the same way – and I think he’s actually leaning toward explicit get methods as well. Let’s not go that far yet, but let’s do change the symbol setting. I’ll delete the setter and let the compiler help.

    private bool ProcessSecondRoll( int pins) {
      score += pins;
      if (score == 10) {
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

    public string Symbol {
      get { return symbol; }
    }

    public void SetSymbol(string newSymbol) {
      symbol = newSymbol;
    }

Now second roll the same way.

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

And in FrameStatus:

    public void SetFirstRoll(int pins) {
      firstRoll = pins.ToString();
    }

    public void SetSecondRoll(int pins) {
      secondRoll = pins.ToString();
    }

Next one may be slightly more interesting, but I have my doubts. The test is StatusIsComplete. We could build that two ways. One is here:

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetComplete();
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

There are going to be other places where we need to set complete – everywhere we move to Ignore, just as we will need for setting score from the strikes and spares. We could do something about it now, but I’m calling YAGNI until the duplication appears. This should make the tests run:

    public bool Complete {
      get { return complete; }
    }

    public void SetComplete() {
      complete = true;
    }

And it does. All the OpenFrameDisplayTestFixture tests run. How about the others? All the FrameTestDisplayFixture tests are red. Chet says that some of them might be green but we don’t have good visibility into how done we are. I fear that he is right. This single assert idea may be better than I thought. Let’s review the tests:

  [TestFixture] public class FrameDisplayTestFixture {
    Frame frame;
    FrameStatus frameStatus;

    public FrameDisplayTestFixture() {
    }

    [SetUp] public void SetUp() {
      frame = new Frame();
    }

    [Test] public void StrikeFirstRoll() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.IsNull(frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsFalse(frameStatus.Complete);
    }

    [Test] public void StrikeSecondRoll() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.IsNull(frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsFalse(frameStatus.Complete);
    }

    [Test] public void StrikeThirdRoll() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.AreEqual("19", frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsTrue(frameStatus.Complete);
    }

    [Test] public void Spare() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("/", frameStatus.Symbol);
      Assert.AreEqual("13", frameStatus.Score);
      Assert.AreEqual("6", frameStatus.FirstRoll);
      Assert.AreEqual("4", frameStatus.SecondRoll);
      Assert.IsTrue(frameStatus.Complete);
    }
  }

We renamed that first test to be more clear. If we try to make things run from here, we’ll be guessing about what works and what doesn’t. For example, it’s clear that we should set the strike status in ProcessFirstRoll, as soon as we realize it’s a 10. But no test will go green, and we’ll have to read the details of the failure to see how we did. NUnit will let us do that, but we won’t get that nice green bar jolt. Chet is sitting on my shoulder with little wings and a halo, saying we should break them out. It can’t take long, we’ll just duplicate the tests five times, rename them, delete some lines. Time me: it’s 10:35.

    [Test] public void StrikeFirstRollSymbolIsX() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
    }

    [Test] public void StrikeFirstRollScoreIsNull() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.IsNull(frameStatus.Score);
    }

    [Test] public void StrikeFirstRollFirstRollIsTen() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("10", frameStatus.FirstRoll);
    }

    [Test] public void StrikeFirstRollSecondRollIsNull() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.IsNull(frameStatus.SecondRoll);
    }

    [Test] public void StrikeFirstRollFrameIsNotComplete() {
      frame.RollConsumed(10);
      frameStatus = frame.FrameStatus();
      Assert.IsFalse(frameStatus.Complete);
    }

It’s 10:37. Took less than three minutes. What was I whining about. Now to make them work.

Wow. Four out of those five work already! The only one that doesn’t work is the X. Now of course if I had known that, I could have just made the X work and the original would have gone green … but I didn’t know that and it would have taken reasoning to figure it out. This way I just push the button. OK, make X work. This is our first touch on ProcessFirstRoll, which looks like this:

    private bool ProcessFirstRoll(int pins) {
      frameStatus.SetFirstRoll(pins);
      score += pins;
      if (score == 10)
        processor = new ProcessRoll(TwoBonusRoll);
      else
        processor = new ProcessRoll(ProcessSecondRoll);
      return true;
    }

And needs to look like this:

    private bool ProcessFirstRoll(int pins) {
      frameStatus.SetFirstRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetSymbol("X");
        processor = new ProcessRoll(TwoBonusRoll);
      }
      else {
        processor = new ProcessRoll(ProcessSecondRoll);
      }
      return true;
    }

Surprisingly, now StrikeSecondRoll is working! It’s a redundant test, which we never realized before. StrikeThirdRoll, and Spare, need work. Let’s look at those:

    [Test] public void StrikeThirdRoll() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
      Assert.AreEqual("19", frameStatus.Score);
      Assert.AreEqual("10", frameStatus.FirstRoll);
      Assert.IsNull(frameStatus.SecondRoll);
      Assert.IsTrue(frameStatus.Complete);
    }

    [Test] public void Spare() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("/", frameStatus.Symbol);
      Assert.AreEqual("13", frameStatus.Score);
      Assert.AreEqual("6", frameStatus.FirstRoll);
      Assert.AreEqual("4", frameStatus.SecondRoll);
      Assert.IsTrue(frameStatus.Complete);
    }

I guess I haven’t a leg to stand on if I don’t want to break these out. So here goes, we’ll just do them all:

    [Test] public void StrikeThirdRollSymbolIsX() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("X", frameStatus.Symbol);
    }

    [Test] public void StrikeThirdRollScoreIsNineteen() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("19", frameStatus.Score);
    }

    [Test] public void StrikeThirdRollFirstRollStillTen() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("10", frameStatus.FirstRoll);
    }

    [Test] public void StrikeThirdRollSecondRollStillNull() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.IsNull(frameStatus.SecondRoll);
    }

    [Test] public void StrikeThirdRollFrameIsComplete() {
      frame.RollConsumed(10);
      frame.RollConsumed(5);
      frame.RollConsumed(4);
      frameStatus = frame.FrameStatus();
      Assert.IsTrue(frameStatus.Complete);
    }

I ran into a little more trouble this time, because I started changing the tests in the middle. Still just took a couple of minutes. Now let’s make them run, and worry about the Spare test next.

Three of the tests already work! This single-assert stuff is cool. ScoreIsNineteen, and FrameIsComplete don’t work. Both of these have to be done in the OneBonusRoll method:

    private bool OneBonusRoll(int pins) {
      score += pins;
      frameStatus.SetScore(score);
      frameStatus.SetComplete();
      processor = new ProcessRoll(IgnoreRoll);
      return false;
    }

Everything works now but Spare. Chet is going to make me break out the tests again:

    [Test] public void SpareSymbolIsSlash() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("/", frameStatus.Symbol);
    }

    [Test] public void SpareScoreIsThirteen() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("13", frameStatus.Score);
    }

    [Test] public void SpareFirstRollIsSix() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("6", frameStatus.FirstRoll);
    }

    [Test] public void SpareSecondRollIsFour() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.AreEqual("4", frameStatus.SecondRoll);
    }

    [Test] public void SpareFrameIsComplete() {
      frame.RollConsumed(6);
      frame.RollConsumed(4);
      frame.RollConsumed(3);
      frameStatus = frame.FrameStatus();
      Assert.IsTrue(frameStatus.Complete);
    }

It went better that time because I started editing with the first one instead of in the middle. Must make a note of that. Now what works? Wow, all of them work except Symbol. That goes in SecondRoll:

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetSymbol("/");
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetComplete();
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

That makes all the tests run! The only problem now is that it seems too soon. It doesn’t feel like enough work. Reviewing the code, though, it looks right to us. There don’t seem to be any status changes missing, and all our tests are running. We didn’t get as much duplication as I was expecting, either. Another triumph for waiting.

This really was a lot easier than the Kicker version. There’s a bit of cleanup that we can do – and it seems clear now that everyone who sets score also sets complete, so we should imply complete from score != null.

It’s lunch time and we are going to try a new place. We’ll retrospect while we eat and I’ll report back shortly.

After Lunch ...

We tried the Carlyle Bar and Grill, newly opened over by the Quality 16 Theater. Kind of your high end burger place, nice building, decent food.

Reviewing the session, we noticed that moving the tests over helped a bit, but that the refactoring we did on them probably cancelled out the benefit of that. The changes we made were all in one class, so we only have a few classes open instead of the nine or ten we had yesterday. There was no passthru in this implementation: the Frame talked directly to the FrameStatus instead of the Kicker talking to the Frame talking to the FrameStatus.

Yesterday we had a bunch of override hassles, trying to convince C# to do what we wanted, and putting the code in the right classes. Nothing like that happened today. We had to pass the Frame to the Kickers, while today, of course, our code was in the Frame, so no additional passing of parameters was needed.

Wednesday, we felt like the objects weren’t helping us: today everything was right in front of us and everything was straightforward.

There wasn’t much duplication added, we felt (I’ll check that in a few minutes) but what was added feels like the kind that can be fixed with simple Extract Method refactorings, rather than more cmplex operations, pushing things around between classes as we had to do with the Kickers.

All in all, it’s clear that the delegates solution, in addition to being much smaller, is both simpler and more amenable to change. A case can be made, of course, that we just made a bad choice with the Kickers, but however you cut it, the new requirements went in much more easily today.

A Little Review and Refactoring

I’ll take a quick scan through the code now, clean a few things up, and let you take a look.

One thing we noticed is that setting score and setting complete always happen at the same time. So we can remove the SetComplete method, and implement the Complete property to check score for null. That will change things like this:

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetSymbol("/");
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetScore(score);
        frameStatus.SetComplete();
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

To this:

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetSymbol("/");
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

With this little tweak in FrameStatus, as well as removing the SetComplete: and the field “complete”.

    public bool Complete {
      get { return score != null; }
    }

Tests are green, of course. Let’s see if there’s anything else to improve.

One thing Chet and I discussed is the fact that the FrameStatus has properties for value fetching, and method-style setters for setting the values. That’s a little odd, and points up the difficulty we have knowing when to use properties and when not. I’m saying we should leave the getters as properties, as they will be used by our alleged GUI, which will just interrogate values, and never set them. The setters, on the other hand, are used only by the operational code. So it makes a fair amount of sense.

OK, what else? We said things like frameStatus.SetSymbol(“X”). That’s not cool, it means that the Frame knows how the FrameStatus represents things. That should be covered by making methods on FrameStatus named SetStrike() and such. I’ll do that. Here are a couple of examples:

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetSymbol("/");
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetSymbol("-");
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

It seems to me that Resharper should be willing to help me on this, but if it knows how I can’t find it. No biggie, the changes are obvious. I think I’ll change SetSymbol to private, so the compiler will find them all for me.

That was easy enough, except when I changed SetSymbol(“/”) to SetStrike() for some reason. Fortunately Resharper let me rename the method when I stumbled over SetSymbol(“X”) a moment or two later. I think we’re good to go: here’s the Frame and FrameStatus as they stand. Take at least a glance at them, and then meet me on the other side:

  public class Frame {
    private int score;
    delegate bool ProcessRoll(int pins);
    private ProcessRoll processor;
    private FrameStatus frameStatus;

    public Frame() {
      score = 0;
      processor = new ProcessRoll(ProcessFirstRoll);
      frameStatus = new FrameStatus();
    }

    public virtual bool RollConsumed(int pins) {
      return processor(pins);
    }

    public int Score() {
      return score;
    }

    public FrameStatus FrameStatus() {
      return frameStatus;
    }

    #region delegates

    private bool ProcessFirstRoll(int pins) {
      frameStatus.SetFirstRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetStrike();
        processor = new ProcessRoll(TwoBonusRoll);
      }
      else {
        processor = new ProcessRoll(ProcessSecondRoll);
      }
      return true;
    }

    private bool ProcessSecondRoll( int pins) {
      frameStatus.SetSecondRoll(pins);
      score += pins;
      if (score == 10) {
        frameStatus.SetSpare();
        processor = new ProcessRoll(OneBonusRoll);
      }
      else {
        frameStatus.SetOpen();
        frameStatus.SetScore(score);
        processor = new ProcessRoll(IgnoreRoll);
        }
      return true;
    }

    private bool TwoBonusRoll(int pins) {
      score += pins;
      processor = new ProcessRoll(OneBonusRoll);
      return false;
    }

    private bool OneBonusRoll(int pins) {
      score += pins;
      frameStatus.SetScore(score);
      processor = new ProcessRoll(IgnoreRoll);
      return false;
    }

    private bool IgnoreRoll(int pins) {
      return false;
    }

    #endregion
  }

  public class FrameStatus {
    private string symbol;
    private string score;
    private string firstRoll;
    private string secondRoll;

    public FrameStatus() {
      symbol = null;
      score = null;
      firstRoll = null;
      secondRoll = null;
    }

    public string Symbol {
      get { return symbol; }
    }

    public string Score {
      get { return score; }
    }

    public void SetScore(int pins) {
      score = pins.ToString();
    }

    public string FirstRoll {
      get { return firstRoll; }
    }

    public void SetFirstRoll(int pins) {
      firstRoll = pins.ToString();
    }

    public void SetSecondRoll(int pins) {
      secondRoll = pins.ToString();
    }

    public string SecondRoll {
      get { return secondRoll; }
    }

    public bool Complete {
      get { return score != null; }
    }

    public void SetSpare() {
      SetSymbol("/");
    }

    public void SetOpen() {
      SetSymbol("-");
    }

    public void SetStrike() {
      SetSymbol("X");
    }

    private void SetSymbol(string newSymbol) {
      symbol = newSymbol;
    }
  }

Summing Up

That’s all there is to it. About 130 lines of code. The Kicker solution was about 360, nearly three times larger.

I don’t want to over-generalize here, but clearly the Kicker solution isn’t as good as the delegate one. And clearly, at least with this separation of concerns, putting in our new feature was much harder – probably at least three times harder and it feels like more.

Simpler was better in this case, and in my opinion, in most every case. But it has to be actually simpler, not cryptically compact.

That’s enough for now. Tune in again soon to see what we do tomorrow!

No, wait! I was just writing a note to Joseph Little on the XP list, and said something I think is important:

Today Chet and I broke some tests down into one assertion per test format, a thing that we don't usually do. It discernibly made things feel more comfortable, gave our work a bit more direction, and I think even made it go faster. So that experience, in a live context, teaches me that I like the one assert trick better than I thought I did. That learning will inform what I do in the future.

I wanted to underline that for my own advantage as much as yours. We need to try things that are a bit different, and leave ourselves open to hearing what they tell us. The one-assert experiment was a good example.