Developer Testing: First Design Your Data Structure

March 15, 2007 - First Design Your Data Structure

It's at about this stage of the bowling example that people usually leap into a discussion about the appropriate data structure to store the rolls and the APIs for exposing the results.

I am also feeling a tension between my desire to model the domain accurately and rule 4 of Kent Beck's criteria for simple design:

Runs all the Tests
Reveals all the intention
No duplication
Fewest number of classes or methods

The UI wants to iterate over frames so the natural thing seems to be a method that returns Iterator<Frame>. This reveals intention (and models the domain) at the cost of an additional class. As an added bonus, the iterator pattern will allow me to hide the data structure inside my Game class and put off the choice of structure until later.

Top-down or Bottom-up?

A common question on the TDD list is whether you should work top-down or bottom-up.

I like to work from the top until I understand enough about how the objects interact with each other and then I switch my focus to TDDing the lower-level objects.

I'll start by adding a test to my existing GameTest.


    @Test
    public void shouldBeNoScoreUntilBowlIsBowled() {
      Frame frame = game.getFrames().next();

      assertThat(frame.getFirstBall(), is(""));
      assertThat(frame.getSecondBall(), is(""));
    }

That's a big step to take for a first test, but I can't see how to break it down without losing the essence of the relationship between the game and its frames. I'll take the big step and follow where it leads me. Eclipse magic gives me a new class, Frame:


  public class Frame {
    public String getFirstBall() {
      return null;
    }
    public String getSecondBall() {
      return null;
    }
  }

and a new method on Game:


    public Iterator <Frame> getFrames() {
      return null;
    }

Running the test gives me an NPE which I can fix by returning an iterator from an empty array list...


    public Iterator <Frame> getFrames() {
      return new ArrayList<Frame>().iterator();
    }

JUnit now says NoSuchElement, which I can fix with...


    public Iterator <Frame> getFrames() {
      ArrayList<Frame> frames = new ArrayList<Frame>()
      frames.add(new Frame());
      return frames.iterator();
    }

...which gives us: Expected: is "" got: null. I can make the test pass by returning empty strings from the methods in Frame.

That wasn't so bad. I didn't give a lot of thought to the data structure; ArrayList is usually my default choice if I don't have a good reason to use something else and, since my data is well encapsulated, I can always change it later.

Showing the Rolls

That first test passes but the rolls are hard-coded. I'll write another test that will replace the empty strings with actual rolls.


    @Test
    public void firstBallShouldBeRecordedInFirstFrame() {
      game.bowl(3);

      Frame frame = game.getFrames().next();
      assertThat(frame.getFirstBall(), is("3"));
    }

I can make that test pass by making frames a field of Game and moving the initialization to the constructor...


  public Game() {
    frames = new ArrayList();
    frames.add(new Frame());
  }

... and recording the pins in the first frame.


  public void bowl(int pins) {
    if(pins > 10 || pins <0) {
      throw new IllegalArgumentException("Invalid pin count - " + pins);
    }
    ballCount++;
    frames.get(0).addBall(pins);
  }

I add this code to Frame...


  public class Frame {
    private Integer firstBall;

    public String getFirstBall() {
      return firstBall != null
           ? firstBall.toString()
           : "";
    }

    public String getSecondBall() {
      return "";
    }

    public void addBall(int pins) {
      firstBall = new Integer(pins);
    }
  }

... and the test passes. I still have a couple of loose ends though. All the balls go into the first frame and I only deal with the first ball in each frame. It's time to make a new test for Frame.

A New Fixture

I'll create a new fixture - FrameTest - to separate the low-level details of the Frame from the high-level details of the Game's interaction with external systems.

I start FrameTest with a simplified copy of that test for the first ball in a frame. This test already passes, so I make another one just like it for the second ball:


  public class FrameTest {
    private Frame frame;

    @Before
      public void createFrame() {
      frame = new Frame();
    }

    @Test
    public void firstBallShouldBeRecorded() {
      frame.addBall(3);
      assertThat(frame.getFirstBall(), is("3"));
    }

    @Test
    public void secondBallShouldBeRecorded() {
      frame.addBall(3);
      frame.addBall(4);
      assertThat(frame.getSecondBall(), is("4"));
    }
  }

A little cutting and pasting makes the test pass...


    public class Frame {
      public String getSecondBall() {
        return secondBall != null
             ? secondBall.toString()
             : "";
      }

      public void addBall(int pins) {
        if(firstBall == null) {
          firstBall = new Integer(pins);
        }
        else {
          secondBall = new Integer(pins);
        }
      }
    }

... and a little refactoring gets rid of the duplication.


  public String getFirstBall() {
    return formatBall(firstBall);
  }

  public String getSecondBall() {
    return formatBall(secondBall);
  }

  private String formatBall(Integer ball) {
    return ball != null
         ? ball.toString()
         : "";
  }

Having separate variables for each of the balls, rather than an array, makes me a little uncomfortable but not enough to make me want to change it yet - so I'll get back to the problem of allocating balls to the right frame.

Getting My Balls in Order

You will recall that all the balls are going into the first frame. I'll add a new test to GameTest to highlight the problem. I'll test a little more than is strictly necessary to illustrate the bigger picture.


  @Test
  public void thirdBallShouldBeAddedToTheSecondFrame() {
    game.bowl(1);
    game.bowl(2);
    game.bowl(3);
    game.bowl(4);

    Iterator<Frame> frames = game.getFrames();

    Frame frameOne = frames.next();
    assertThat(frameOne.getFirstBall(), is("1"));
    assertThat(frameOne.getSecondBall(), is("2"));

    Frame frameTwo = frames.next();
    assertThat(frameTwo.getFirstBall(), is("3"));
    assertThat(frameTwo.getSecondBall(), is("4"));
  }

Oops! That test fails but not for the reason I expected. Not only do all the balls go into the first frame, there is only one frame! I'll fix that problem first. I'll do a quick fix now and tidy up after we get back to green.


  public Game() {
    frames = new ArrayList<Frame>();
    frames.add(new Frame());
    frames.add(new Frame());
  }

Now the test says Expected: is "2" got : "4" so all the balls are going into the first frame. A simple solution is to ask the frames whether they need more balls and here I am going to break from TDD orthodoxy and add a new test for Frame while a test for Game is still failing (I hope I don't get excommunicated).


  @Test
  public void frameShouldNeedMoreThanOneBall() {
    frame.addBall(3);
    assertThat(frame.needsMoreBalls(), is(true));
  }

After the usual cycle of compile fails, return false, test fails, return true, test passes, I have a frame that always needs more balls. Another test will fix that...


    @Test
    public void frameShouldNotNeedMoreThanTwoBalls() {
      frame.addBall(3);
      frame.addBall(4);
      assertThat(frame.needsMoreBalls(), is(false));
    }

...and this code...

  public boolean needsMoreBalls() {
    return secondBall == null;
  }

...makes it pass. Now I can go back the game and add this loop to bowl()...


  public void bowl(int pins) {
    if(pins > 10 || pins <0) {
      throw new IllegalArgumentException("Invalid pin count - " + pins);
    }

    ballCount++;

    for(Frame frame : frames){
      if(frame.needsMoreBalls()){
        frame.addBall(pins);
        break;
      }
    }
  }

...and all the tests pass.

More Frames Please

We only work for two frames so let's initialize all the frames in one go. The test...


    @Test
    public void thereShouldBeTenFrames() {
      int counter = 0;
      for (Iterator<Frame> frames = game.getFrames(); frames.hasNext();) {
        frames.next();
        counter++;
      }
      assertThat(counter, is(10));
    }

...and the code...


    public Game() {
      frames = new ArrayList<Frame>();
      for(int i = 0; i < FRAMES_PER_GAME; i++) {
        frames.add(new Frame());
      }
    }

...and the test passes. The code for the iterator bothers me though. Why can't I use the new for loop syntax for Iterator? Does it make sense to make the Game class Iterable so I can do for(Frame frame : game)? I am going to declare that it does. In fact, if you squint a little, that reads as for each frame in the game.

I am changing it. Game implements Iterable<Frame> and the old-style iteration loop is gone for ever. I need to update the draft UI too.


    <c:forEach var='frame' items='${game}'>

If I am not mistaken, I have written enough code to pass the acceptance tests for this story. I'll report back with the test results in the next installment.

Posted by Kevin Lawrence at March 15, 2007 04:08 PM

Trackback Pings

TrackBack URL for this entry: