The Nature of Software Development

I’ve been developing software for a while now. Over fifteen years professionally, and quite some time before that too. One would think that when you’ve been doing something for a long time, you would develop a good understanding of what it is that you do. Well, maybe. But every once in a while I come across something that deepens my insight, that takes it to the next level.

I recently came across an article (from 1985!) that does just that, by providing a theory of what software development really is. If you have anything to do with developing software, please read the article. It’s a bit dry and terse at times, but please persevere.

The article promotes the view that, in essence, software development is a theory building activity. This means that it is first and foremost about building a mental model in the developer’s mind about how the world works and how the software being developed handles and supports that world.

This contrasts with the more dominant manufacturing view that sees software development as an activity where some artifacts are to be produced, like code, tests, and documentation. That’s not to say that these artifacts are not produced, since obviously they are, but rather that that’s not the real issue. If we want to be able to develop software that works well, and is maintainable, we’re better off focusing on building the right theories. The right artifacts will then follow.

This view has huge implications for how we organize software development. Below, I will discuss some things that we need to do differently from what you might expect based on the manufacturing view. The funny thing is that we’ve known that for some time now, but for different reasons.

We need strong customer collaboration to build good theories
Mental models can never be externalized completely, some subtleties always get lost when we try that. That’s why requirements as documents don’t work. It’s not just that they will change (Agile), or that hand-offs are a waste (Lean), no, they fundamentally cannot work! So we need the customer around to clarify the subtle details, and we need to go see how the end users work in their own environment to build the best possible theories.

Since this is expensive, and since a customer would not like having to explain the same concept over and over again to different developers, it makes sense to try and capture some of the domain knowledge, even though we know we can never capture every subtle detail. This is where automated acceptance tests, for example as produced in Behavior Driven Development (BDD), can come in handy.

Software developers should share the same theory
It’s of paramount importance that all the developers on the team share the same theory, or else they will develop things that make no sense to their team members and that will not integrate well with their team members’ work. Having the same theory is helped by using the same terminology. Developers should be careful not to let the meaning of the terms drift away from how the end users use them.

The need for a shared theory means that strong code ownership is a bad idea. Weak ownership could work, but we’d better take measures to prevent silos from forming. Code reviews are one candidate for this. The best approach, however, is collective code ownership, especially when combined with pair programming.

Note that eXtreme Programming (XP) makes the shared theory explicit through it’s system metaphor concept. This is the XP practice that I’ve always struggled most with, but now it’s finally starting to make sense to me.

Theory building is an essential skill for software developers
If software development is first and foremost about building theories, then software developers better be good at it. So it makes sense to teach them this. I’m not yet aware of how to best do this, but one obvious place to start is the scientific method, since building theories is precisely what scientist do (even though some dispute the scientific method is actually the way scientists build theories). This reminds me of the relationship between the scientific method and Test-Driven Development (TDD).

Another promising approach is Domain Driven Design, since that places the domain model at the center of software development. Please let me know if you have more ideas on this subject.

Software developers are not interchangeable resources
If the most crucial part of software development is the building of a theory, than you can’t just simply replace one developer with another, since the new girl needs to start building her theory from scratch and that takes time. This is another reason why you can’t add people to a late project without making it later still, as (I wish!) we all know.

Also, developers with a better initial theory are better suited to work on a new project, since they require less theory building. That’s why it makes sense to use developers with pre-existing domain knowledge, if possible. Conversely, that’s also why it makes sense for a developer to specialize in a given domain.

We should expect designs to undergo significant changes
Taking a hint from science, we see that theories sometimes go through radical changes. Now, if the software design is a manifestation of the theory in the developers’ mind, then we can expect the design to undergo radical changes as well. This argues against too much design up front, and for incremental design.

Maintenance should be done by the original developers
Some organizations hand off software to a maintenance team once its initial development is done. From the perspective of software development as theory building, that doesn’t make a whole lot of sense. The maintenance team doesn’t have the theories that the original developers built, and will likely make modifications that don’t fit that theory and are therefore not optimal.

If you do insist on separate maintenance teams, then the maintainers should be phased into the team before the initial stage ends, so they have access to people that already have well formed theories and can learn from them.

Software developers should not be shared between teams
For productivity reasons, software developers shouldn’t divide their attention between multiple projects. But the theory building view of software development gives another perspective. It’s hard enough to build one theory at a time, especially if one is also learning other stuff, like a new technology. Developers really shouldn’t need to learn too much in parallel, or they may feel like their head is going to explode.

Top-Down Test-Driven Development

In Test-Driven Development (TDD), I have a tendency to dive right in at the level of some class that I am sure I’m gonna need for this cool new feature that I’m working on. This has bitten me a few times in the past, where I would start bottom-up and work my way up, only to discover that the design should be a little different and the class I started out with is either not needed, or not needed in the way I envisioned. So today I wanted to try a top-down approach.

I’m running this experiment on a fresh new project that targets developers. I’m going to start with a feature that removes some of the mundane tasks of development. Specifically, when I practice TDD in Java, I start out with writing a test class. In that class, I create an instance of the Class Under Test (CUT). Since the CUT doesn’t exist at this point in time, my code doesn’t compile. So I need to create the CUT to make it compile. In Java, that consists of a couple of actions that are pretty uninteresting, but that need to be done anyway. This takes away my focus from the test, so it would be kinda cool if it somehow could be automated.

In work mostly in Eclipse, and Eclipse has the notion of Quick Fixes. So that seems like a perfect fit. However, I don’t want my project code to be completely dependent on Eclipse, if only because independent code is easier to test.

So I start out with a top-down test that shows how all of this is accomplished:

public class FixesFactoryTest {

  @Test
  public void missingClassUnderTest() {
    FixesFactory fixesFactory = new FixesFactory();
    Issues issues = new Issues().add(Issue
        .newProblem(new MissingType(new FullyQualifiedName("Bar")))
        .at(new FileLocation(
            new Path("src/test/java/com/acme/foo/BarTest.java"),
            new FilePosition(new LineNumber(11), new ColumnNumber(5)))));
    Fixes fixes = fixesFactory.newInstance(issues);

    Assert.assertNotNull("Missing fixes", fixes);
    Assert.assertEquals("# Fixes", 1, fixes.size());

    Fix fix = fixes.iterator().next();
    Assert.assertNotNull("Missing fix", fix);
    Assert.assertEquals("Fix", CreateClassFix.class, fix.getClass());

    CreateClassFix createClassFix = (CreateClassFix) fix;
    Assert.assertEquals("Name of new class", new FullyQualifiedName("com.acme.foo.Bar"),
        createClassFix.nameOfClass());
    Assert.assertEquals("Path of new class", new Path("src/main/java/com/acme/foo/Bar.java"),
        createClassFix.pathOfClass());
  }

}

This test captures my intented design: a FixesFactory gives Fixes for Issues, where an Issue is a Problem at a given Location. This will usually be a FileLocation, but I envision there could be problems between files as well, like a test class whose name doesn’t match the name of its CUT. For this particular issue, I expect one fix: to create the missing CUT at the right place.

I’m trying to follow the rules of Object Calistenics here, hence the classes like LineNumber where one may have expected a simple int. Partly because of that, I need a whole bunch of classes and methods before I can get this test to even compile. This feels awkward, because it’s too big a step for my taste. I want my green bar!

Obviously, I can’t make this pass with a few lines of code. So I add a @Ignore to this test, and shift focus to one of the smaller classes. Let’s see, LineNumber is a good candidate. I have no clue as to how I’ll be using this class, though. All I know at this point, is that it should be a value object:

public class LineNumberTest {

  @Test
  public void valueObject() {
    LineNumber lineNumber1a = new LineNumber(313);
    LineNumber lineNumber1b = new LineNumber(313);
    LineNumber lineNumber2 = new LineNumber(42);

    Assert.assertTrue("1a == 1b", lineNumber1a.equals(lineNumber1b));
    Assert.assertFalse("1a == 2", lineNumber1a.equals(lineNumber2));

    Assert.assertTrue("# 1a == 1b", lineNumber1a.hashCode() == lineNumber1b.hashCode());
    Assert.assertFalse("# 1a == 2", lineNumber1a.hashCode() == lineNumber2.hashCode());

    Assert.assertEquals("1a", "313", lineNumber1a.toString());
    Assert.assertEquals("1b", "313", lineNumber1b.toString());
    Assert.assertEquals("2", "42", lineNumber2.toString());
  }

}

This is very easy to implement in Eclipse: just select the Quick Fix to Assign Parameter To Field on the constructor’s single parameter and then select Generate hashCode() and equals()…:

public class LineNumber {

  private final int lineNumber;

  public LineNumber(int lineNumber) {
    this.lineNumber = lineNumber;
  }

  @Override
  public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + lineNumber;
    return result;
  }

  @Override
  public boolean equals(Object obj) {
    if (this == obj) {
      return true;
    }
    if (obj == null) {
      return false;
    }
    if (getClass() != obj.getClass()) {
      return false;
    }
    LineNumber other = (LineNumber) obj;
    if (lineNumber != other.lineNumber) {
      return false;
    }
    return true;
  }

}

This is not the world’s most elegant code, so we’ll refactor this once we’re green. But first we need to add the trivial toString():

  @Override
  public String toString() {
    return Integer.toString(lineNumber);
  }

And we’re green.

EclEmma tells me that some code in LineNumber.equals() is not covered. I can easily fix that by removing the if statements. But the remainder should clearly be refactored, and so should hashCode():

  @Override
  public int hashCode() {
    return 31 + lineNumber;
  }

  @Override
  public boolean equals(Object object) {
    LineNumber other = (LineNumber) object;
    return lineNumber == other.lineNumber;
  }

The other classes are pretty straightforward as well. The only issue I ran into was a bug in EclEmma when I changed an empty class to an interface. But I can work around that by restarting Eclipse.

If you are interested to see where this project is going, feel free to take a look at SourceForge. Maybe you’d even like to join in!

Retrospective

So what does this exercise teach me? I noted earlier that it felt awkward to be writing a big test that I can’t get to green. But I now realize that I felt that way because I’ve trained myself to be thinking about getting to green quickly. After all, that was always the purpose of writing a test.

But it wasn’t this time. This time it was really about writing down the design. That part I usually did in my head, or on a piece of paper or whiteboard before I would write my first test. By writing the design down as a test, I’m making it more concrete than UML could ever hope to be. So that’s definitely a win from my perspective.

The other thing I noted was not so good: I set out to write a top-down test, yet I didn’t. I didn’t start at the bottom either, but somewhere in the middle. I was quick to dismiss the Eclipse part, because I wanted at least part of the code to be independent from Eclipse. Instead, I should have coded all of that up in a test. That would have forced me to consider whether I can actually make the design work in an Eclipse plug-in. So I guess I have to practice a bit more at this top-down TDD stuff…

Software Craftmanship

There has been quite some activity in the blogosphere around the term Software Craftmanship lately, and I’d like to chime in with my two cents.

Background
Pete McBreen’s book Software Craftmanship started what now can be called a movement, as is evidenced by the fact that there is a manifesto, a mailing list, and conferences in Europe and the US. One of the best descriptions of the movement, in my opinion anyway, is by Corey Haines.

The Discussion
It all started with a post by Dan North that was critical of the movement. Members risked being elitist, indulging in navel-gazing, and coding for code’s sake. Others, like Liz Keogh, chimed in.

Replies were inevitable, of course, and came from Jason Gorman, Dave Hoover, Ade Oshineye, and George Dinwiddie, among others.

Dan North just replied to the replies, so I guess the discussion will continue for a while.

My position
So where do I stand?

I signed the manifesto because of two main reasons:

  1. I take pride in my work
  2. I want to be the best I can be in what I do

And that’s what I think the manifesto and the movement is all about. Let me explain.

Stages of Learning
The craft model assumes that there are different levels (apprentice, journeyman, master) at which software developers operate. That does not mean that software craftsmen want to return to the Dark Ages of the guilds. It simply means that they acknowledge that there are stages to learning. Whether we name those stages after the five stages in the Dreyfus model, the four stages of competence, Shu-Ha-Ri, or something else, doesn’t really matter.

Nor does it matter where exactly we draw the line between those stages. The basic thing to understand here, is that we have to go through different phases on our journey to become ever better. I think that being aware of how we learn, and how learning progresses, helps us learning.

Experience
Another thing the craft model emphasizes, is that there are different schools of thought on what is best. There is no one true way of doing things that we can learn. Instead, we must seek out different experiences in different places if we want to become the best we can be.

Don’t get me wrong: the journeyman model, however fascinating, is hardly practical for most people. But you don’t have to literally travel the world to benefit from it. Just keep the model in mind when selecting your next employer, or the next step in your career at the same employer. Take into account what you want to learn, what you haven’t yet been exposed to.

For instance, I learned the basics about databases in college. My knowledge expanded greatly (particularly about the Oracle RDBMS) while working with Kees Verruijt at Redwood. And then that understanding deepened again when I learned about XML databases at X-Hive (acquired by EMC, where I still work).

Practice
So, does the manifesto help at all with learning or with learning how to learn? I’m afraid not. That’s why some have suggested adding a Further Reading section to it. But that won’t cut it, since learning is about much more than reading.

You also need to practice and reflect to grow. That’s why there is so much talk about coding dojos, katas, code retreats, or even etudes and object calisthenics.

I can understand that some people get uneasy by all that weird terminology. But I encourage them to look beyond the words. The point is simply that in order to become an expert, you need to practice, as the craft model teaches us.

Beyond the Manifesto
In summary, my position is that there is value in thinking about software development as a craft. That doesn’t mean that I think that the Manifesto is perfect. In fact, it doesn’t help me much in learning or in learning about learning. So where do we go from here? We need more concrete help on our journey to mastery.

But I don’t particularly like the idea of re-introducing guilds through some formal institution like a Software Craftsmanship Council. I also don’t think we need it.

We already have ways of knowing how good people are. We have to, or else how could we hire good software developers? So I think we should start by looking at the types of things we look for in candidates during our hiring process. Something like a Software Competency Matrix would be a good start for an aspiring craftsman to get a feel for what areas to improve.

Score yourself on each of the items in the matrix. Find your weak spots, and improve them through practice, practice, and some more practice. And have fun doing it!

The verdict on Perforce

At work, I’m now forced to use the Perforce version control system, since that’s what our company has standardized upon. I’ve had some bad feelings about that from the start (based on reading about it), but I’ve hold off on publicizing them so I could give Perforce a fair chance. After all, I have been wrong before ;). So now that I’ve worked with it for several months, here’s my verdict.

Speed
The Perforce slogan is Perforce, The Fast Software Configuration Management System. So they’re basically claiming that they are faster than their competitors. How does this claim hold up?

That question is not so easy to answer, since their competitors are not a homogeneous bunch. But let’s look at one category of competitors: the distributed version control systems. The most well-known of these are Git, Bazaar, and Mercurial. Interestingly, Git calls itself The fast version control system and Mercurial’s slogan is Work easier, Work faster.

Distributed version control systems work locally, meaning they don’t need a network connection. Contrast this with Perforce, that needs a network connection for everything. And I mean everything, to a ridiculous level. For instance, even the help command needs a network connection:

$ p4 help
Perforce client error:
        Connect to server failed; check $P4PORT.
        TCP connect to perforce failed.
        perforce: host unknown.

Now, obviously network access is going to slow things down a lot, so it’s difficult to see how Perforce can still beat their competitors on speed. And my experience has been very clear: it doesn’t! In fact, Perforce is very slow. Now obviously, that depends on your network bandwidth, so your mileage may vary.

But it gets worse. My primary interface to Perforce is not the command line client, but the Eclipse integration, P4WSAD. And although Perforce claims that this is the best of both worlds, my opinion is that this is a piece of crap. There, I said it. P4WSAD makes my life as a developer a hell.

Perforce makes all files read-only by default. Only once you’ve checked out a file, will it become writable. And, you’ve guessed it, that requires network access. In practice, this means that everytime I want to change some source file, I have to wait until P4SWAD checks out the file, which can take up to five seconds! This is extremely annoying, because it completely breaks my flow. And if you think one file is bad, try doing a refactoring that affects multiple files… It is reason enough for me to not ever want to work with Perforce again.

BTW, it is interesting to note that none of the aforementioned distributed version control systems appear in Perforce’s comparison with its competitors.

More connection troubles
Now if all this slowness actually bought me some nice features, that could change the story, right? Well, yes, it could. But it doesn’t.

The same cause for slowness, accessing the network for everything, is also limiting what you can do.

For instance, I have a long commute in the train, so I like to work there. And guess what, I don’t have an internet connection there. Not to worry, Perforce has a workaround called offline mode. This basically means that P4WSAD will nag you for confirmation every time you try to change a file.

It also means that it looses track of which files were changed, so that when you get back online, you forget to submit some files and break the build. That has happened to me quite a few times now, because the reconcile feature is not available in P4SWAD. You need to use the Perforce Visual Client (P4V) for that. So now I need to use two tools to get my work done.

Another limitation of P4WSAD is that it will block a refactoring affecting a file that you haven’t already modified since you went offline. This means you have to hunt down all the places where, say, a method to be renamed is used, and force a “checkout” of all those places by changing something in the file. Only then can you do your refactoring. Very annoying.

Transactions
Perforce claims to support transactions, which is a must for a source code control system. We don’t want our automated build to pick up part of a set of changed files and break because of that!

Unfortunately, transactions in Perforce only work when they work. In other words, when an error occurs, it’s very well possible that Perforce will have comitted only a subset of the files in the “transaction”. This is not a really big deal, as it doesn’t happen all that often, but still.

Directories
Perforce is completely file based; it doesn’t track directories. So it’s impossible to add an empty directory to a repository, for instance. Also, when someone removes a directory, Perforce by default will leave empty directories on people’s file systems when they synchronize. There is a setting to fix that, but it’s set to the wrong value by default. I consider this only a minor flaw, but it’s annoying nonetheless.

Conclusion
Would I recommend Perforce to anybody? Not really. I think there are better alternatives out there. Free ones, mind you. So save yourself some money and check out (pun intended) Git, Mercurial, or Bazaar.

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.

Root Cause Analysis

I’ve moved on to a new project recently. It’s quite different from the previous one. Before I worked on a monolythic web application, now we’re using OSGi. As a result, our project consists of a lot of sub-projects (OSGi bundles) which makes it very inconvenient to use Ant. So we’ve switched to Gradle. Our company has also standardized on Perforce, where we used Subversion before.

To summarize, a lot has changed and I’m not really up to speed yet. This is evidenced by the fact that I broke our build 4 times in 5 days. As an aspiring software craftsman, I feel really bad about that. So why did this happen so often? And can I do anything to prevent it from happening again? Enter Root Cause Analysis.

Root cause analysis is performed to not just treat the symptoms, but cure the disease:

The key to effective problem solving is to first make sure you understand the problem that you are
trying to solve – why it needs to be solved, how you will know when you’ve solved it, and what the
root cause is.
Often symptoms show up in one place while the actual cause of the problem is somewhere
completely different. If you just “solve” the symptom without digging deeper it is highly likely that
problem will just reappear later in a different shape.

Henrik Kniberg has written about one way of doing root cause analysis: using Cause-Effect diagrams. Using this method, I ended up with the following:

Build Failure Cause Effect Diagram

I started out with the problem: Build Failure, in the orange rectangle. I then repeatedly asked myself why this is a bad thing and added the effects in the red rectangles. Just repeat this until you find something that conflicts with your goal. Finally, I repeatedly added causes in blue rectangles. You can stop when you’ve found something you can fix, but in general it’s good to keep asking a bit deeper than feels comfortable. This is where the Five Whys technique comes in handy.

As you can see, my main problem is impatience. For those who know me, that won’t come as a surprise 😉 However, in this case this personal flaw of mine gets in the way of my goal of making customers happy.

With the causes identified, it’s time to think up some countermeasures. Pick some causes that you can fix, and think of a way to treat them. The ones I’m going to work on are marked with a star in the figure above:

  • Use a checklist when submitting code to the source code repository. This will prevent me from making silly mistakes, such as forgetting to add a new file
  • Take the time to learn the tools better, in particular Gradle and Groovy
  • In general, try to be more patient

The last one is the hard one, of course. Wish me luck!

Update 2010-08-01: I have created a checklist with things to consider before submitting code to our Perforce repository and used this checklist all week. I broke the build twice this week, both times because of special circumstances that were not accounted for on the checklist. So I think I’m improving, but I’m not there yet.

The Case for Test-Driven Development

Not everybody is convinced that Test-Driven Development (TDD) is a good idea. So here’s my attempt to change that ;). Changing the world is hard, though, so this will be a long post.

Feedback
There is value in faster feedback: it allows us to steer earlier, giving us a better chance to accomplish our goals. In software development, the only reliable way to get feedback is by trying the software out in the wild, i.e. by having real end users perform their work with it.

Don’t be fooled into thinking that having someone review a design document is appropriate feedback. The reviewer does give feedback, of course, but not of the kind that will guarantee that we’ll ship a workable product. Just as the original author of the design document may miss things, so may the reviewer. The only way to be sure that our software works, is to see it working for our users. That doesn’t mean a design review isn’t valuable, just that it’s not enough. We need defense in depth all the way through.

So, the only real way of ensuring early feedback is to deliver working software frequently and get it into the hands of end users. This is done in iterations, short time-boxed periods of development. Since we value feedback, we want our iterations to be as short as possible. And short iterations require automated testing, because a manual tester simply doesn’t have enough time to test the entire application.

TDD is one form of automated testing. It is a way of coming up with lots of small, focused, and fast tests. That’s why it’s more suited for rapid feedback than automated acceptance testing. But again, we need to use all techniques in our defense in depth.

So that’s reason #1 for using TDD: it’s required to get real feedback early and often, which gives us a better chance at delivering on time, on budget by allowing us to steer often.

Quality: reliability
There are at least two aspects of quality in software development. The most obvious one is defect density. This is the external view of quality, the one experienced by our end users, and therefore the ultimate one.

Traditionally, quality was achieved using inspection, i.e. looking at some (intermediate) product and determining whether it was good enough. Manual acceptance testing falls into this category, but so do design and code reviews.

One problem with this approach is that testing comes at the end and is thus the most likely to suffer from schedule overruns. By writing the test before the code is written, you guarantee that testing will happen, since you can’t ship without the code.

Another problem is that inspection of products is expensive. You first put effort into creating a product, then into inspecting it, then into reworking it, inspecting it again, etc. That’s why many organizations changed their ways to build quality in from the start, i.e. to inspect the process instead of the product produced by the process.

TDD is a way to do just that: build quality in, i.e. prevent bugs from happening because you write the tests first. We know the code will work, since it passes the test. And since no code is written unless a test fails, we know that all code works. (Unless, of course, a test is wrong, incomplete, or missing, which is why we also need other tools in our defense in depth.)

So that’s reason #2 for using TDD: it builds in quality, leading to fewer defects, leading to happier customers and better productivity (through less time spent fixing bugs).

Quality: modifyability
But there is also the internal view of quality. Here we’re talking about code quality from a developer’s perspective. This is obviously less important than making our customers happy, but it’s significant nonetheless. Poor code slows us down when making changes, because it’s hard to understand. And it might be correct now, but if we don’t understand it well, we have a higher chance of introducing a defect when we change it.

So, what makes code difficult to understand? It’s the complexity of the code: the more things that are going on, the higher the chance that you’re missing some of the subtle interactions between them. The formal name is cyclomatic complexity, which is a measure of the number of possible paths of execution through a piece of code. Higher cyclomatic complexity is linked to higher defects.

Static code analyzers, like CheckStyle, can measure it and report warnings when limits are exceeded, so that you can then fix the code. But low cyclomatic complexity is also a byproduct of TDD, since simpler code is easier to test. Thus using TDD prevents the rework of fixing the CheckStyle warnings.

It’s a pain to have to set up a whole bunch of objects just so you can test one of them. That’s why writing the tests first naturally encourages having objects depend on not to many other objects. This is called low coupling, and together with high cohesion, it’s also a very important aspect of code quality.

So that’s reason #3 for using TDD: it improves code quality, which leads to better productivity and fewer defects (which leads to better productivity as well).

Design
This is where the fun begins.”

Some people claim that TDD is not primarily a testing technique, but a design technique. They say TDD stands for Test-Driven Design and the tests are just a fortunate by-product of the design process.

To evaluate that claim we need to look at what design really is: moving from a problem (requirements) to a solution (working software that fulfills those requirements). Here’s a rough outline of how we do that:

  1. We take one requirement and think of how it constraints the solution.
  2. We then think of possible ways of satisfying those constraits.
  3. We take the next requirement and do the same.
  4. We modify any solutions for the first requirement that conflict with those for the second one.
  5. We repeat until we have satisfied all requirements.

Now compare that with how TDD works:

  1. We take one requirement and think of how it constraints the solution. Then we express these constraints with one or more tests.
  2. We write the simplest code that can make the test pass and then refactor to improve the code organization.
  3. We take the next requirement and do the same.
  4. We fix any test or code for the first requirement that broke while implementing the second one.
  5. We repeat until we have satisfied all requirements.

As you can see, doing TDD is doing design. In fact, TDD is a leaner, more effecient way of doing design, since it limits work in progress. And limiting work in progess is an important driver for productivity gains in Lean, as work in progress is seen as one of the 7 forms of waste.

So how does TDD limit work in progress? First, TDD prevents rework from testing to coding by limiting the amount of untested code.

Second, it limits the amount of design decisions awaiting implementation. Any programmer who has ever been handed a design written by someone else will know that the devil is in the details and those details will require the design to be modified. (In reality it’s likely that the design will not be updated, which is even worse.)

So that’s reason #4 for using TDD: it’s a leaner, more efficient way of designing.

Documentation
Don’t be fooled by the lack of a design document in the above description. Not having a design document does not mean there is no design or that there is no thinking before coding. The design is in the code, and the tests document it. That doesn’t mean we can’t write documentation, just that we don’t have to.

Not only are the tests documentation, they are a superior form of documentation: you can check whether the code matches the design by compiling and running the tests. And since tests are code as well, you get the full power of your IDE to help you keep the tests up-to-date when refactoring your code under test.

So that’s reason #5 for using TDD: it’s a leaner, more efficient way of keeping the design documentation up-to-date.

Conclusion
There you have it, 5 very good reasons to practice TDD:

  1. It’s required to get real feedback early and often.
  2. It builds in quality, preventing rework.
  3. It improves code quality, making maintenance easier.
  4. It’s a leaner, more efficient way of designing.
  5. It’s a leaner, more efficient way of keeping the design documentation up-to-date.

What others are saying
Don’t just take my word for it, read what others have to say as well:

  1. TDD Illustrated
  2. The Art of Agile on TDD
  3. Test infected
  4. TDD doesn’t slow you down
  5. Faster, better, cheaper! TDD wins in a simple experiment

But there is more than just opinions. Although it’s always hard to scientifically test any activity that involves humans, some experiments have been performed to assess the effectiveness and efficiency of TDD.

Moving forward
So now that you’re convinced 😉 of the power of TDD, how do you start?

Well, read the book on it. And maybe read some more on the web, e.g.

  1. The Three Laws of TDD
  2. Unit test patterns, like Four-Phase Test
  3. The TDD Checklist (Red-Green-Refactor in Detail)
  4. Keep insignificant details out of tests
  5. Test data builders

Or, you could just give it a spin. Download one of the xUnit families of unit testing frameworks, like JUnit or NUnit, and get you hands dirty. And once you’ve been there and done it, get the T-shirt.

Brian Marick: I think I finally understand mocks

Yesterday I attended a meeting co-organized by Devnology and Agile Holland and hosted by the Dutch eBay markplaats.nl where Brian Marick talked about mocks.

Brian started out with a visual demonstration of what object oriented programming is all about: interaction between objects. Each object doesn’t do very much by itself, but it’s the complex web of interacting objects that gets the job done. He visualized this interaction by having “volunteers” (representing objects) passing around a little ball (representing the flow of control):
Visual Demonstration of Object Interactions

He then went on to introduce interaction based testing using mock objects. This approach is a really white form of white box testing, since it not only knows about objects and their methods, but also about how these methods are implemented, i.e. what other objects they call and how.

Interaction based testing lends itself naturally to a top-down approach of TDD. This maximizes the chance that the objects that are designed into existence by the tests fit seamlessly with the objects calling them.

Brian described some other differences between interaction based (“London school”) and the more traditional state based (“Detroit school”) of TDD. Both have pros and cons, but ultimately Brian feels that interaction based testing using mocks makes it more likely to work with ease.
Work with Ease

Performance tuning an Ant build

Common advice in the Agile world is to maintain an automated build that runs in under 10 minutes. I doubt anybody would disagree that a faster build is better than a slower one. But how do we keep the build slick?

Usually the bulk of the build time is spend in executing tests, so that is the first place to start. But as with any optimization effort, you shouldn’t guess where the pain is, but measure. So how do we do that for an Ant build?

This turns out to be not hard at all. Ant will happily inform your listener of any interesting event, such as task started or stopped. Using that information, it is pretty straightforward to write a listener that records the time the tasks and targets take.

But you don’t even have to do that, you can simply use the open source Ant Utilities project. Simply place the jar in Ant’s lib directory and run Ant as follows:

ant -listener net.java.antutility.BuildMetricsListener
[target]

At the end of the Ant build, a profile report will be displayed:

...
BUILD SUCCESSFUL
Total time: 4 minutes 6 seconds
BUILD METRICS:
Local Time, Child Time, Invocation Count, Type, Name, Location
88453, 0, 1, TASK, macker, build-core.xml:448:
70955, 0, 8, TASK, javac, dist\build.xml:608:
36563, 0, 6, TASK, jar, dist\build.xml:628:
31047, 0, 1, TASK, checkstyle, build-core.xml:251:
4031, 0, 1, TASK, exec, dist\build.xml:947:
1922, 0, 1, TASK, taskdef, build-core.xml:345:
1797, 0, 1, TASK, signjar, build-core.xml:233:
1688, 0, 1, TASK, uptodate, build-core.xml:425:
1018, 1557, 21, TASK, for, build.xml:19:
688, 0, 1, TASK, taskdef, build-core.xml:404:
609, 0, 1, TASK, property, build-core.xml:171:
533, 0, 21, TASK, taskdef, dist\build.xml:15:

The first column indicates the time spent in the element (task or target), the last two the name of the element and the build file name and line number.

But it gets better still. You can also run your Ant build from Eclipse and get a visual indication of the pain points in your editor:
Visual indication of slow Ant elements in Eclipse

Questioning Automated Acceptance Testing

The controversy
There has been a flurry of activity in the blogosphere and twitterverse about the practice of automated acceptance testing. It all started when James Shore said that “acceptance testing isn’t worth the cost. I no longer use it or recommend it.

Now, when one of the thought leaders in the Agile community says something that goes against the established wisdom, it is sure to provoke reactions:

  • Gojko Adzic wonders whether the cost is due to the tools and whether we could do without those tools.
  • George Dinwiddie thinks Shore is throwing out the baby with the bathwater and that the problems are fixable.
  • Ron Jeffries doesn’t want to give up on the notion of having automated acceptance tests be part of the definition of done, but acknowledges that Fit is a pain to work with.
  • Lisa Crispin thinks that there is no one right way and that everybody should experiment to find out what works for them.

James Shore responded to all of them in an excellent post where he explains that he found other ways to get the benefits that automated acceptance tests bring, without incurring the costs. Some of the comments on this post are excellent as well.

All of these bright minds bring good points to the table. They show that the Agile community is not the dogmatic clique that it is sometimes made out to be.

My two cents
I personally sympathize with Ron Jeffries. Seems like a real shame to not have automated acceptance tests be part of the definition of done. So what can we do?

Well, bottom line is that everybody seems to agree that customers providing examples is a good idea. The disagreement is only over whether those examples should be automated or not, and if so, how.

It’s true that customers don’t want to write tests. Why would they have to? Because they don’t trust tests written by others, James Shore says. Well, what about some collaboration? Why can’t the customer give examples that are turned into tests on the spot by developers or testers? Would the customer still not trust these? Where does the distrust come from, exactly?

As for the high costs, isn’t there something that we can do to lower the costs to the point where they are justified? What exactly is it that makes the costs so high? Is it the tooling, as Gojko Adzic implies? If so, why don’t we improve our tools?