Bowling Again

It’s been a while since I’ve done the bowling game scoring exercise. For those of you who don’t know it, the bowling game (ten-pin) is almost the Hello, world! of TDD. I want to learn Groovy, but I’m going to re-do it in Java first, so that I have recent code and feelings to compare when I sink my teeth in Groovy.

Here’s the goal for this exercise: write a program that can score a bowling game. Inputs are the rolls, output the score.

The first thing to note is that the inputs are rolls (number of pins knocked down), while the score depends on frames. A frame consists of up to two rolls: if the first roll knocks all 10 pins down, then we’re talking about a strike and the frame consist of just the one roll. Otherwise, the frame consists of two rolls. If the second roll knocks down all pins, the frame is called a spare, else it’s a regular frame. The only exception is the last frame. If that is a strike or a spare, it will consist of three rolls. We call those bonus frames.

Frames

So our first task is to distinguish the several types of frames. Let’s start simple, with a regular frame:

public class RegularFrameTest {

  @Test
  public void frame() {
    final Frame frame = new RegularFrame(2, 5);
    Assert.assertEquals("# rolls", 2, frame.numRolls());
    Assert.assertEquals("First", 2, frame.pins(0));
    Assert.assertEquals("Second", 5, frame.pins(1));
  }

}

This forces me to write the Frame interface:

public interface Frame {

  int numRolls();
  int pins(int index);

}

The implementation is straightforward:

public class RegularFrame implements Frame {

  private final int first;
  private final int second;

  public RegularFrame(final int first, final int second) {
    this.first = first;
    this.second = second;
  }

  @Override
  public int numRolls() {
    return 2;
  }

  @Override
  public int pins(final int index) {
    return index == 0 ? first : second;
  }

}

Now, let’s do a spare:

public class SpareTest {

  @Test
  public void frame() {
    final Frame frame = new Spare(3);
    Assert.assertEquals("# rolls", 2, frame.numRolls());
    Assert.assertEquals("First", 3, frame.pins(0));
    Assert.assertEquals("Second", 7, frame.pins(1));
  }

}

Again, pretty easy:

public class Spare implements Frame {

  private final int first;

  public Spare(final int first) {
    this.first = first;
  }

  @Override
  public int numRolls() {
    return 2;
  }

  @Override
  public int pins(final int index) {
    return index == 0 ? first : 10 - first;
  }

}

OK, so by now, strike should be familiar territory:

public class StrikeTest {

  @Test
  public void frame() {
    final Frame frame = new Strike();
    Assert.assertEquals("# rolls", 1, frame.numRolls());
    Assert.assertEquals("first", 10, frame.pins(0));
  }

}
public class Strike implements Frame {

  @Override
  public int numRolls() {
    return 1;
  }

  @Override
  public int pins(final int index) {
    return 10;
  }

}

And finally, the bonus frame:

public class BonusFrameTest {

  @Test
  public void frame() {
    final Frame frame = new BonusFrame(10, 3, 5);
    Assert.assertEquals("# rolls", 3, frame.numRolls());
    Assert.assertEquals("first", 10, frame.pins(0));
    Assert.assertEquals("second", 3, frame.pins(1));
    Assert.assertEquals("third", 5, frame.pins(2));
  }

}
public class BonusFrame implements Frame {

  private final int first;
  private final int second;
  private final int third;

  public BonusFrame(final int first, final int second, final int third) {
    this.first = first;
    this.second = second;
    this.third = third;
  }

  @Override
  public int numRolls() {
    return 3;
  }

  @Override
  public int pins(final int index) {
    switch (index) {
      case 0: return first;
      case 1: return second;
      default: return third;
    }
  }

}

Note that I never bothered to specify what pins should return for an invalid index.

Now, there is a lot of duplication in these classes, so let’s extract a common base class. First, I’m going to focus on BonusFrame, since that one has the most code.

I’m going to take baby steps, keeping the code green as much as possible. First, I introduce a list to hold the three separate values:

public class BonusFrame implements Frame {

  private final List<Integer> pins = new ArrayList<Integer>();

  public BonusFrame(final int first, final int second, final int third) {
    this.first = first;
    this.second = second;
    this.third = third;
    pins.add(first);
    pins.add(second);
    pins.add(third);
  }

  // ...
}

Next, I implement numRolls using the new list:

public class BonusFrame implements Frame {

  @Override
  public int numRolls() {
    return pins.size();
  }

  // ...
}

Then the pins method:

public class BonusFrame implements Frame {

  @Override
  public int pins(final int index) {
    return pins.get(index);
  }

  // ...
}

Now the original fields are unused, and I can delete them safely. Never in this refactoring did I break the tests, not even for a second.

The second stage of this refactoring is to extract a base class that will hold the list of pins. Since the constructor is using the fields, I need to change it slightly, so that I can safely move the list. First I’ll introduce a new constructor that accepts a list of pins:

public class BonusFrame implements Frame {

  protected BonusFrame(final List<Integer> pins) {
    this.pins.addAll(pins);
  }

  // ..
}

Then I change the original constructor to call the new one:

public class BonusFrame implements Frame {

  public BonusFrame(final int first, final int second, final int third) {
    this(Arrays.asList(first, second, third));
  }

  // ...
}

Now I can do an automated Extract Superclass refactoring to get my coveted base class:

public class BonusFrame extends BaseFrame implements Frame {

  public BonusFrame(final int first, final int second, final int third) {
    this(Arrays.asList(first, second, third));
  }

  protected BonusFrame(final List<Integer> pins) {
    this.pins.addAll(pins);
  }

}

Unfortunately, there is no way to have Eclipse also move the constructor that I had prepared 😦 Also, the BaseFrame class doesn’t implement Frame yet:

public class BaseFrame {

  protected final List<Integer> pins = new ArrayList<Integer>();

  public BaseFrame() {
    super();
  }

  @Override
  public int numRolls() {
    return pins.size();
  }

  @Override
  public int pins(final int index) {
    return pins.get(index);
  }

}

This is bad, since now the code doesn’t even compile anymore, thanks to the @Override! Let’s add the missing implements, so we’re back to green. The public no-arg constructor can also go, it will be generated for us.

Now to clean up the mess. Eclipse doesn’t offer to Pull Up constructors, which is a shame. So let’s move it by hand:

public class BonusFrame extends BaseFrame implements Frame {

  public BonusFrame(final int first, final int second, final int third) {
    super(Arrays.asList(first, second, third));
  }

}
public class BaseFrame implements Frame {

  protected BaseFrame(final List<Integer> pins) {
    this.pins.addAll(pins);
  }

  // ...
}

The list should be private, and the implements Frame on BonusFrame can go.

We can now use BaseFrame as a base class for the other classes as wel:

public class Strike extends BaseFrame {

  public Strike() {
    super(Arrays.asList(10));
  }

}
public class Spare extends BaseFrame {

  public Spare(final int first) {
    super(Arrays.asList(first, 10 - first));
  }

}
public class RegularFrame extends BaseFrame {

  public RegularFrame(final int first, final int second) {
    super(Arrays.asList(first, second));
  }

}

I’m happy with the result, so let’s move on.

From Rolls To Frames

Now that we have the different kinds of frames in place, we need a way to create them from the rolls that are our input. We basically need to convert a series of rolls into a series of frames. This series idea we can capture in an Iterator:

public class FrameIteratorTest {

  @Test
  public void regular() {
    final Iterator<Frame> frames = new FrameIterator(Arrays.asList(1, 2));
    Assert.assertTrue("Missing frame", frames.hasNext());

    final Frame frame = frames.next();
    Assert.assertEquals("Frame", new RegularFrame(1, 2), frame);
    Assert.assertFalse("Extra frame", frames.hasNext());
  }

}

But for this to work, we need to implement equals on BaseFrame:

public class BaseFrameTest {

  @Test
  public void equality() {
    final Frame frame1 = new BaseFrame(Arrays.asList(4, 2));
    Assert.assertTrue("Same", frame1.equals(new BaseFrame(Arrays.asList(4, 2))));
    Assert.assertFalse("Different", frame1.equals(new BaseFrame(Arrays.asList(2, 4))));
  }

}

This is easy to implement, since Eclipse can generate the equals and hashCode for us:

public class BaseFrame implements Frame {

  @Override
  public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + ((pins == null) ? 0 : pins.hashCode());
    return result;
  }

  @Override
  public boolean equals(final Object obj) {
    if (this == obj) {
      return true;
    }
    if (obj == null) {
      return false;
    }
    if (getClass() != obj.getClass()) {
      return false;
    }
    final BaseFrame other = (BaseFrame) obj;
    if (pins == null) {
      if (other.pins != null) {
        return false;
      }
    } else if (!pins.equals(other.pins)) {
      return false;
    }
    return true;
  }

  // ...
}

Now we can return to our FrameIterator:

public class FrameIterator implements Iterator<Frame> {

  private final Iterator<Integer> rolls;

  public FrameIterator(final List<Integer> rolls) {
    this.rolls = rolls.iterator();
  }

  @Override
  public boolean hasNext() {
    return rolls.hasNext();
  }

  @Override
  public Frame next() {
    final int first = rolls.next();
    final int second = rolls.next();
    return new RegularFrame(first, second);
  }

  @Override
  public void remove() {
    throw new UnsupportedOperationException();
  }

}

OK, so how about a spare?

public class FrameIteratorTest {

  @Test
  public void spare() {
    final Iterator<Frame> frames = new FrameIterator(Arrays.asList(8, 2));
    Assert.assertTrue("Missing frame", frames.hasNext());

    final Frame frame = frames.next();
    Assert.assertEquals("Frame", new Spare(8), frame);
  }

  // ...
}
public class FrameIterator implements Iterator<Frame> {

  @Override
  public Frame next() {
    final int first = rolls.next();
    final int second = rolls.next();
    final Frame result;
    if (first + second == 10) {
      result = new Spare(first);
    } else {
      result = new RegularFrame(first, second);
    }
    return result;
  }

  // ...
}

OK, then a strike shouldn’t be too hard:

public class FrameIteratorTest {

  @Test
  public void strike() {
    final Iterator<Frame> frames = new FrameIterator(Arrays.asList(10));
    Assert.assertTrue("Missing frame", frames.hasNext());

    final Frame frame = frames.next();
    Assert.assertEquals("Frame", new Strike(), frame);
  }

  // ...
}

This test doesn’t just fail, but it throws a NoSuchElementException, since we only provide one roll. Not too hard to fix, though:

public class FrameIterator implements Iterator<Frame> {

  @Override
  public Frame next() {
    final Frame result;
    final int first = rolls.next();
    if (first == 10) {
      result = new Strike();
    } else {
      final int second = rolls.next();
      if (first + second == 10) {
        result = new Spare(first);
      } else {
        result = new RegularFrame(first, second);
      }
    }
    return result;
  }

  // ...
}

OK, that works, but the code is a bit messy. Not only do we have a magic constant, we now have it in two places! So let’s get rid of that:

public class FrameIterator implements Iterator<Frame> {

  private static final int PINS_PER_LANE = 10;

  @Override
  public Frame next() {
    final Frame result;
    final int first = rolls.next();
    if (isAllDown(first)) {
      result = new Strike();
    } else {
      final int second = rolls.next();
      if (isAllDown(first + second)) {
        result = new Spare(first);
      } else {
        result = new RegularFrame(first, second);
      }
    }
    return result;
  }

  private boolean isAllDown(final int pins) {
    return pins == PINS_PER_LANE;
  }

  // ...
}

I’m still not all that happy with this code, although I guess it does state the rules quite clearly now. Maybe the messyness of the code follows the messyness of the rules? I dont know, so let’s let our background processor think that over some more while I continue.

We now have all the normal frames in place, but we still lack the bonus frames. First a spare:

public class FrameIteratorTest {

  @Test
  public void bonusSpare() {
    final Iterator<Frame> frames = new FrameIterator(Arrays.asList(10, 10, 10, 10, 10, 10, 10, 10, 10, 2, 8, 3));
    for (int i = 0; i < 9; i++) {
      Assert.assertTrue("Missing frame " + i, frames.hasNext());

      final Frame frame = frames.next();
      Assert.assertEquals("Frame " + i, new Strike(), frame);
    }

    Assert.assertTrue("Missing bonus frame", frames.hasNext());

    final Frame frame = frames.next();
    Assert.assertEquals("Bonus frame", new BonusFrame(2, 8, 3), frame);
  }

  // ...
}
public class FrameIterator implements Iterator<Frame> {

  private static final int NUM_FRAMES_PER_GAME = 10;

  private int numFrames;

  @Override
  public Frame next() {
    numFrames++;
    final Frame result;
    final int first = rolls.next();
    if (isAllDown(first)) {
      result = new Strike();
    } else {
      final int second = rolls.next();
      if (isAllDown(first + second)) {
        if (isFinalFrame()) {
          result = new BonusFrame(first, second, rolls.next());
        } else {
          result = new Spare(first);
        }
      } else {
        result = new RegularFrame(first, second);
      }
    }
    return result;
  }

  private boolean isFinalFrame() {
    return numFrames == NUM_FRAMES_PER_GAME;
  }

  // ...
}

And then the strike:

public class FrameIteratorTest {

  @Test
  public void bonusStrike() {
    final Iterator<Frame> frames = new FrameIterator(Arrays.asList(10, 10, 10, 10, 10, 10, 10, 10, 10, 10, 7, 2));
    for (int i = 0; i < 9; i++) {
      Assert.assertTrue("Missing frame " + i, frames.hasNext());

      final Frame frame = frames.next();
      Assert.assertEquals("Frame " + i, new Strike(), frame);
    }

    Assert.assertTrue("Missing bonus frame", frames.hasNext());

    final Frame frame = frames.next();
    Assert.assertEquals("Bonus frame", new BonusFrame(10, 7, 2), frame);
  }

  // ...
}
public class FrameIterator implements Iterator<Frame> {

  @Override
  public Frame next() {
    numFrames++;
    final Frame result;
    final int first = rolls.next();
    if (isAllDown(first)) {
      if (isFinalFrame()) {
        final int second = rolls.next();
        result = new BonusFrame(first, second, rolls.next());
      } else {
        result = new Strike();
      }
    } else {
      final int second = rolls.next();
      if (isAllDown(first + second)) {
        if (isFinalFrame()) {
          result = new BonusFrame(first, second, rolls.next());
        } else {
          result = new Spare(first);
        }
      } else {
        result = new RegularFrame(first, second);
      }
    }
    return result;
  }

  // ...
}

OK, now we really have to simplify the code! Let’s extract all the strike and spare stuff:

public class FrameIterator implements Iterator<Frame> {

  @Override
  public Frame next() {
    numFrames++;
    final Frame result;
    final int first = rolls.next();
    if (isAllDown(first)) {
      result = newStrike(first);
    } else {
      final int second = rolls.next();
      if (isAllDown(first + second)) {
        result = newSpare(first, second);
      } else {
        result = new RegularFrame(first, second);
      }
    }
    return result;
  }

  private Frame newStrike(final int first) {
    final Frame result;
    if (isFinalFrame()) {
      final int second = rolls.next();
      result = new BonusFrame(first, second, rolls.next());
    } else {
      result = new Strike();
    }
    return result;
  }

  private Frame newSpare(final int first, final int second) {
    final Frame result;
    if (isFinalFrame()) {
      result = new BonusFrame(first, second, rolls.next());
    } else {
      result = new Spare(first);
    }
    return result;
  }

  // ...
}

I think I could further simplify the code by refactoring to a Strategy pattern and then do a Replace Conditional With Polymorphism to get rid of all those pesky if statements. But although that would make the individual methods easier to read, we would lose the overview, and it may just make the rules harder to retrieve from the code. So, I’m inclined to leave it as it is.

On to scoring then!

Scoring

This is where the fun begins.

The basic score for a frame is just the number of pins that were knocked down:

public class BaseFrameTest {

  @Test
  public void baseScore() {
    final Frame frame = new BaseFrame(Arrays.asList(3, 6));
    Assert.assertEquals("Base score", 9, frame.getBaseScore());
  }

  // ...
}
public interface Frame {

  int getBaseScore();

  // ...
}
public class BaseFrame implements Frame {

  @Override
  public int getBaseScore() {
    int result = 0;
    for (final int pins : this.pins) {
      result += pins;
    }
    return result;
  }

  // ...
}

Writing that only now makes me realize that I named the list of rolls pins. But it’s not the pins knocked down, but the list of pins per roll that were knocked down:

public class BaseFrame implements Frame {

  private final List<Integer> rolls = new ArrayList<Integer>();

  protected BaseFrame(final List<Integer> rolls) {
    this.rolls.addAll(rolls);
  }

  @Override
  public int numRolls() {
    return rolls.size();
  }

  @Override
  public int pins(final int index) {
    return rolls.get(index);
  }

  @Override
  public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + ((rolls == null) ? 0 : rolls.hashCode());
    return result;
  }

  @Override
  public boolean equals(final Object obj) {
    if (this == obj) {
      return true;
    }
    if (obj == null) {
      return false;
    }
    if (getClass() != obj.getClass()) {
      return false;
    }
    final BaseFrame other = (BaseFrame) obj;
    if (rolls == null) {
      if (other.rolls != null) {
        return false;
      }
    } else if (!rolls.equals(other.rolls)) {
      return false;
    }
    return true;
  }

  @Override
  public int getBaseScore() {
    int result = 0;
    for (final int pins : rolls) {
      result += pins;
    }
    return result;
  }

}

Now, that was the easy part of scoring. Apart from the base score, strikes and spares also have extra scores. We don’t want to deal with exceptions, though. That will just lead to code littered with if statements. Instead, we can use polymorphism and the Null Object pattern:

public class BaseFrameTest {

  @Test
  public void extraScore() {
    final Frame frame = new BaseFrame(Arrays.asList(3, 4));
    Assert.assertTrue("Extra score", frame.getExtraScore() instanceof NoExtraScore);
  }

  // ...
}
public interface Frame {

  ExtraScore getExtraScore();

  // ...
}
public interface ExtraScore {

}
public class NoExtraScore implements ExtraScore {

}

Note that the ExtraScore interface is empty for now. We will get to that in a minute. First let’s make the test pass:

public class BaseFrame implements Frame {

  @Override
  public ExtraScore getExtraScore() {
    return new NoExtraScore();
  }

  // ...
}

For spares, the extra score is one roll:

public class SpareTest {

  @Test
  public void extraScore() {
    final Frame frame = new Spare(4);
    Assert.assertTrue("Extra score", frame.getExtraScore() instanceof OneRollExtraScore);
  }

  // ...
}
public class OneRollExtraScore implements ExtraScore {

}
public class Spare extends BaseFrame {

  @Override
  public ExtraScore getExtraScore() {
    return new OneRollExtraScore();
  }

  // ...
}

For strikes, the extra score is two rolls:

public class StrikeTest {

  @Test
  public void extraScore() {
    final Frame frame = new Strike();
    Assert.assertTrue("Extra score", frame.getExtraScore() instanceof TwoRollsExtraScore);
  }

  // ...
}
public class TwoRollsExtraScore implements ExtraScore {

}
public class Strike extends BaseFrame {

  @Override
  public ExtraScore getExtraScore() {
    return new TwoRollsExtraScore();
  }

  // ...
}

Allright, what about that ExtraScore interface? We don’t want to go back to dealing with rolls, since we have our lovely FrameIterator that gives us frames. So the extra score must deal with frames. First, it must tells us whether it needs any at all:

public class NoExtraScoreTest {

  @Test
  public void needFrame() {
    final ExtraScore extraScore = new NoExtraScore();
    Assert.assertFalse("Need more", extraScore.needFrame());
  }

}
public interface ExtraScore {

  boolean needFrame();

}
public class NoExtraScore implements ExtraScore {

  @Override
  public boolean needFrame() {
    return false;
  }

}

We have a little bit more work for the one roll extra score:

public class OneRollExtraScoreTest {

  @Test
  public void extraScore() {
    final ExtraScore extraScore = new OneRollExtraScore();
    Assert.assertTrue("Need more", extraScore.needFrame());

    final Frame frame = new RegularFrame(3, 4);
    final int score = extraScore.getScore(frame);
    Assert.assertEquals("Extra score", 3, score);
    Assert.assertFalse("Need still more", extraScore.needFrame());
  }

}
public interface ExtraScore {

  int getScore(Frame frame);

  // ...
}
public class OneRollExtraScore implements ExtraScore {

  private boolean needMore = true;

  @Override
  public boolean needFrame() {
    return needMore;
  }

  @Override
  public int getScore(final Frame frame) {
    needMore = false;
    return frame.pins(0);
  }

}

And the extra score for a strike is the most complex:

public class TwoRollsExtraScoreTest {

  @Test
  public void regular() {
    final ExtraScore extraScore = new TwoRollsExtraScore();
    Assert.assertTrue("Need more", extraScore.needFrame());

    final Frame frame = new RegularFrame(6, 3);
    final int score = extraScore.getScore(frame);
    Assert.assertEquals("Extra score", 9, score);
    Assert.assertFalse("Still need more", extraScore.needFrame());
  }

  @Test
  public void strike() {
    final ExtraScore extraScore = new TwoRollsExtraScore();
    Assert.assertTrue("Need more", extraScore.needFrame());

    Frame frame = new Strike();
    int score = extraScore.getScore(frame);
    Assert.assertEquals("Extra score", 10, score);
    Assert.assertTrue("Still need more", extraScore.needFrame());

    frame = new RegularFrame(6, 3);
    score = extraScore.getScore(frame);
    Assert.assertEquals("Extra score 2", 6, score);
    Assert.assertFalse("Still need more 2", extraScore.needFrame());
  }

}
public class TwoRollsExtraScore implements ExtraScore {

  private int numNeeded = 2;

  @Override
  public boolean needFrame() {
    return numNeeded > 0;
  }

  @Override
  public int getScore(final Frame frame) {
    int result = 0;
    final int numGotten = Math.min(numNeeded, frame.numRolls());
    for (int i = 0; i < numGotten; i++) {
      result += frame.pins(i);
    }
    numNeeded -= numGotten;
    return result;
  }

}

Now we need to glue all the previous together to calculate the game’s score:

public class FramesScorerTest {

  @Test
  public void score() {
    final FramesScorer scorer = new FramesScorer();
    Frame frame = new FakeFrame(2);
    scorer.add(frame);
    Assert.assertEquals("Score", 9, scorer.getScore());

    List<ExtraScore> extraScores = scorer.getExtraScores();
    Assert.assertNotNull("Missing extra scores", extraScores);
    Assert.assertEquals("# Extra scores", 1, extraScores.size());
    Assert.assertTrue("Extra score", extraScores.get(0) instanceof FakeExtraScore);

    frame = new FakeFrame(1);
    scorer.add(frame);
    Assert.assertEquals("Score 2", 19, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 2", 2, extraScores.size());

    frame = new FakeFrame(0);
    scorer.add(frame);
    Assert.assertEquals("Score 3", 30, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 3", 3, extraScores.size());

    scorer.add(frame);
    Assert.assertEquals("Score 4", 41, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 4", 3, extraScores.size());
  }


  private static class FakeFrame extends BaseFrame {

    protected FakeFrame(final int pins) {
      super(Arrays.asList(pins, 9 - pins));
    }

    @Override
    public ExtraScore getExtraScore() {
      return new FakeExtraScore(pins(0));
    }

  }


  private static class FakeExtraScore implements ExtraScore {

    private final int numNeeded;

    public FakeExtraScore(final int numNeeds) {
      numNeeded = numNeeds;
    }

    @Override
    public boolean needFrame() {
      return numNeeded  > 0;
    }

    @Override
    public int getScore(final Frame frame) {
      return 1;
    }

  }

}

This test is a lot more involved than normal, and it took some time to get it right. This is because it’s really the sequence of steps that we want to test, as all the individual steps are already tested.

Let’s implement this assert by assert. First, the base score:

public class FramesScorer {

  private int score;

  public void add(final Frame frame) {
    score += frame.getBaseScore();
  }

  public int getScore() {
    return score;
  }

  public List<ExtraScore> getExtraScores() {
    return null;
  }

}

Next, we need to remember the extra score object:

public class FramesScorer {

  private final List<ExtraScore> extraScores = new ArrayList<ExtraScore>();

  public void add(final Frame frame) {
    score += frame.getBaseScore();
    extraScores.add(frame.getExtraScore());
  }

  public List<ExtraScore> getExtraScores() {
    return extraScores;
  }

  // ...
}

Then we need to add the collected extra scores:

public class FramesScorer {

  public void add(final Frame frame) {
    score += frame.getBaseScore();
    final Iterator<ExtraScore> iterator = extraScores.listIterator();
    while (iterator.hasNext()) {
      final ExtraScore extraScore = iterator.next();
      score += extraScore.getScore(frame);
    }
    extraScores.add(frame.getExtraScore());
  }

  // ...
}

And we must not forget to remove extra scores that are done:

public class FramesScorer {

  public void add(final Frame frame) {
    score += frame.getBaseScore();
    final Iterator<ExtraScore> iterator = extraScores.listIterator();
    while (iterator.hasNext()) {
      final ExtraScore extraScore = iterator.next();
      if (extraScore.needFrame()) {
        score += extraScore.getScore(frame);
      } else {
        iterator.remove();
      }
    }
    extraScores.add(frame.getExtraScore());
  }

  // ...
}

Actually, the way I implemented this puts the ExtraScore object on the list even if it is a NoExtraScore object, which seems a bit wasteful. So let’s fix that:

public class FramesScorerTest {

  @Test
  public void score() {
    final FramesScorer scorer = new FramesScorer();
    Frame frame = new FakeFrame(2);
    scorer.add(frame);
    Assert.assertEquals("Score", 9, scorer.getScore());

    List<ExtraScore> extraScores = scorer.getExtraScores();
    Assert.assertNotNull("Missing extra scores", extraScores);
    Assert.assertEquals("# Extra scores", 1, extraScores.size());
    Assert.assertTrue("Extra score", extraScores.get(0) instanceof FakeExtraScore);

    frame = new FakeFrame(1);
    scorer.add(frame);
    Assert.assertEquals("Score 2", 19, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 2", 2, extraScores.size());

    frame = new FakeFrame(0);
    scorer.add(frame);
    Assert.assertEquals("Score 3", 30, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 3", 2, extraScores.size());

    scorer.add(frame);
    Assert.assertEquals("Score 4", 41, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 4", 2, extraScores.size());
  }

  // ...
}
public class FramesScorer {

  public void add(final Frame frame) {
    score += frame.getBaseScore();
    final Iterator<ExtraScore> iterator = extraScores.listIterator();
    while (iterator.hasNext()) {
      final ExtraScore extraScore = iterator.next();
      score += extraScore.getScore(frame);
      if (!extraScore.needFrame()) {
        iterator.remove();
      }
    }

    final ExtraScore extraScore = frame.getExtraScore();
    if (extraScore.needFrame()) {
      extraScores.add(extraScore);
    }
  }

  //  ...
}

Now we need to clean this up a bit:

public class FramesScorer {

  private int score;
  private final List<ExtraScore> extraScores = new ArrayList<ExtraScore>();

  public void add(final Frame frame) {
    score += frame.getBaseScore() + extraScore(frame);
    rememberExtraScore(frame);
  }

  private int extraScore(final Frame frame) {
    int result = 0;
    final Iterator<ExtraScore> iterator = extraScores.listIterator();
    while (iterator.hasNext()) {
      final ExtraScore extraScore = iterator.next();
      result += extraScore.getScore(frame);
      if (!extraScore.needFrame()) {
        iterator.remove();
      }
    }
    return result;
  }

  private void rememberExtraScore(final Frame frame) {
    final ExtraScore extraScore = frame.getExtraScore();
    if (extraScore.needFrame()) {
      extraScores.add(extraScore);
    }
  }

  // ...
}

All that’s left to do, is collect the rolls and provide them to the scorer. First the collecting:

public class GameTest {

  @Test
  public void rolls() {
    final Game game = new Game();
    game.roll(3);
    game.roll(1);
    game.roll(3);
    Assert.assertEquals("Rolls", Arrays.asList(3, 1, 3), game.getRolls());
  }

}
public class Game {

  private final List<Integer> rolls = new ArrayList<Integer>();

  public void roll(final int pins) {
    rolls.add(pins);
  }

  protected List<Integer> getRolls() {
    return rolls;
  }

}

And finally the glue that ties the scoring together:

public class GameTest {

  @Test
  public void score() {
    final Game game = new Game();
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    game.roll(1);
    Assert.assertEquals("Score", 20, game.getScore());
  }

}
public class Game {

  public int getScore() {
    final FramesScorer scorer = new FramesScorer();
    final FrameIterator frames = new FrameIterator(getRolls());
    while (frames.hasNext()) {
      scorer.add(frames.next());
    }
    return scorer.getScore();
  }

  // ...
}

I’m confident that the code works, but let’s make sure by adding some tests with well-known answers:

public class GameTest {

  @Test
  public void perfect() {
    final Game game = new Game();
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    game.roll(10);
    Assert.assertEquals("Score", 300, game.getScore());
  }

  @Test
  public void alternateStrikesAndSpares() {
    final Game game = new Game();
    for (int i = 0; i < 5; i++) {
      game.roll(10);
      game.roll(4);
      game.roll(6);
    }
    game.roll(10);
    Assert.assertEquals("Score", 200, game.getScore());
  }

}

All green, so I guess we’re done!

Reflection

Let’s take another look at all the code to see if there is anything that needs some more attention.

One thing I notice, is that the code for TwoRollsExtraScore is a generalization for the other extra scores, so we could extract a base class. First, I introduce a constructor to set the private field:

public class TwoRollsExtraScore implements ExtraScore {

  private int numNeeded;

  public TwoRollsExtraScore() {
    this(2);
  }

  protected TwoRollsExtraScore(final int numNeeded) {
    this.numNeeded = numNeeded;
  }

  // ...
}

Then use Extract Superclass (again with manual cleanup of the errors that Eclipse introduces):

public class TwoRollsExtraScore extends BaseExtraScore {

  public TwoRollsExtraScore() {
    super(2);
  }

}
public class BaseExtraScore implements ExtraScore {

  private int numNeeded;

  protected BaseExtraScore(final int numNeeded) {
    this.numNeeded = numNeeded;
  }

  @Override
  public boolean needFrame() {
    return numNeeded > 0;
  }

  @Override
  public int getScore(final Frame frame) {
    int result = 0;
    final int numGotten = Math.min(numNeeded, frame.numRolls());
    for (int i = 0; i < numGotten; i++) {
      result += frame.pins(i);
    }
    numNeeded -= numGotten;
    return result;
  }

}

And now we can use the base class to derive the other extra score classes from:

public class OneRollExtraScore extends BaseExtraScore {

  protected OneRollExtraScore() {
    super(1);
  }

}
public class NoExtraScore extends BaseExtraScore {

  protected NoExtraScore() {
    super(0);
  }

}

With that little code in the classes, one may wonder wether they are still pulling their weight. I like to think so, since they express the concepts in the game well. I introduced them because of that, after all. So I’m keeping them.

I also find a “bug” in FakeExtraScore: the numNeeded field is never decreased, so basically it will always keep requesting new frames. Here’s the fix:

public class FramesScorerTest {

  @Test
  public void score() {
    final FramesScorer scorer = new FramesScorer();
    Frame frame = new FakeFrame(2);
    scorer.add(frame);
    Assert.assertEquals("Score", 9, scorer.getScore());

    List<ExtraScore> extraScores = scorer.getExtraScores();
    Assert.assertNotNull("Missing extra scores", extraScores);
    Assert.assertEquals("# Extra scores", 1, extraScores.size());
    Assert.assertTrue("Extra score", extraScores.get(0) instanceof FakeExtraScore);

    frame = new FakeFrame(1);
    scorer.add(frame);
    Assert.assertEquals("Score 2", 19, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 2", 2, extraScores.size());

    frame = new FakeFrame(0);
    scorer.add(frame);
    Assert.assertEquals("Score 3", 30, scorer.getScore());
    extraScores = scorer.getExtraScores();
    Assert.assertEquals("# Extra scores 3", 0, extraScores.size());
  }


  private static class FakeFrame extends BaseFrame {

    protected FakeFrame(final int pins) {
      super(Arrays.asList(pins, 9 - pins));
    }

    @Override
    public ExtraScore getExtraScore() {
      return new FakeExtraScore(pins(0));
    }

  }


  private static class FakeExtraScore implements ExtraScore {

    private int numNeeded;

    public FakeExtraScore(final int numNeeds) {
      numNeeded = numNeeds;
    }

    @Override
    public boolean needFrame() {
      return numNeeded  > 0;
    }

    @Override
    public int getScore(final Frame frame) {
      numNeeded--;
      return 1;
    }

  }

}

The rest of the code seems fine.

I must say that of all the times I’ve done this exercise, this time I ended up with the most classes/interfaces: 14 all together! But these total just 356 lines, or about 25 lines per type. Between all the syntactic cruft that Java makes me write and the generous use of blank lines that I prefer, I’d say that’s a pretty good score. I wonder how that will turn out in Groovy. Stay tuned.

Advertisements

Please Join the Discussion

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s