ArticleS. DavidChelimsky.
BowlingWithoutPrefactoring [add child]

Bowling Without Prefactoring


In Paul Pagel's blog about prefactoring, he talks about when you refactor in advance of adding a new feature with the idea that you're making the code easier to add that feature. The problem, as Paul points out, is that you're really designing up front when you do that.

I was discussing this with some students this week after we had done the bowling game exercise. As I was talking about the pit falls of prefactoring, I realized that there is a step we typically do in that exercise where we prefactor. It's when we are preparing to solve for spares. Before we do that, the test and code look something like this:
public class GameTest extends TestCase {

private Game game;

public void setUp() {
game = new Game();
}

public void testAllGuttersScoreShouldBeZero() {
rollPinsNTimes(0, 20);
assertEquals(0, game.score());
}

public void testAllOnesScoreShouldBe20() {
rollPinsNTimes(1, 20);
assertEquals(20, game.score());
}

private void rollPinsNTimes(int numPins, int numRolls) {
for (int i = 0; i < numRolls; i++) {
game.roll(numPins);
}
}
}

public class Game {
private int score;

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

public int score() {
return score;
}
}

The next step is typically to prefactor the Game class to handle an array of ints:
public class Game {

private int[] rolls = new int[20];
private int currentRoll;

public void roll(int pins) { rolls[currentRoll++] = pins; }

public int score() {
int score = 0;
for (int roll = 0; roll < rolls.length; roll++) {
score += rolls[roll];
}
return score;
}
}

Now all the tests pass and we are allegedly set up to handle spares, but when we add the test for spares we end up changing the nature of the loop to solve the problem. Instead of looping from 0 to the rolls.length, we iterate from 0 to 10, representing frames. I remember when I first encountered this and thought how strange it felt to change the nature of the loop at that point.

I've come to believe that the oddness was related to this notion of prefactoring. We changed the design thinking we were going to support spares but, in fact, we got it wrong. We designed the wrong loop. Thinking about this brings up a number of questions. Aren't we designing up front in that case? Isn't that something to avoid? Isn't the idea to refactor to improve the structure?

So let's try a different approach. Let's go back to our implementation as it was after testing for all gutters and all ones.
public class Game {
private int score;
public void roll(int pins) { score = pins; }
public int score() { return score; }
}

Now, rather than prefactoring for spares, we just add the test...
public void testAllFivesScoreShouldBe150() {
rollPinsNTimes(5, 21);
assertEquals(150, game.score());
}

... and run it and watch it fail. Now we have to make it pass without the other tests failing, right? We know that our tests use all 0's, all 1's and all 5's. The tests using 0's and 1's are passing. The only failing test is the test that uses 5's. We can use that temporarily to help us isolate the code to pass that test from the other code. First we modify Game as follows:
public class Game {
private boolean handlingSpares;
...
public int score() {
if (handlingSpares)
return 150;
return score;
}
}

We run the test and it still fails because the game doesn't know we're testing spares. So we give it a way to know that it is...
public class Game {

private int score;
private boolean handlingSpares;

public void roll(int pins) {
score += pins;
if (pins == 5)
handlingSpares = true;
}

public int score() {
if (handlingSpares)
return 150;
return score;
}
}

At first glance, this seems a bit crazy. We're introducing this variable that bears all sorts of assumptions. Well, how different is that, as a temporary measure, from hard coding a return value to get a test to pass. When we tell score() to return 0 to get the all gutters test to pass, aren't we doing something very similar? This is the simplest thing we can do to get our tests to pass.

And in fact, all the tests do pass.

So what's the next step? Refactor? Well, it might be, but there's not really much clear duplication here (at least I don't see it yet), and we really don't have guidance as to what to refactor to. So, next test:
public void testOneSpareGameScore() {
rollPinsNTimes(5, 3);
assertEquals(20, game.score());
}

We now have two tests on spares that have to produce different scores. This is the first time that we are forced to collect the rolls and keep track of them in any way. So we introduce our integer array and our loop
public class Game {
private int[] rolls = new int[21];
...
public int score() {
if (handlingSpares) {
score = 0;
int roll = 0;
for (int frame = 0; frame < 10; frame++) {
score += rolls[roll] + rolls[roll + 1] + rolls[roll + 2];
roll += 2;
}
}
return score;
}
}

Running the tests at this point fails both spares tests. The results are 0 for both games instead of 150 and 15. Why? Well we haven't started populating rolls yet. So we add it, leaving the whole class looking like this:
public class Game {

private int score;
private boolean handlingSpares;
private int[] rolls = new int[21];
private int currentRoll;

public void roll(int pins) {
score += pins;
rolls[currentRoll++] = pins;
if (pins == 5)
handlingSpares = true;
}

public int score() {
if (handlingSpares) {
score = 0;
int roll = 0;
for (int frame = 0; frame < 10; frame++) {
score += rolls[roll] + rolls[roll + 1] + rolls[roll + 2];
roll += 2;
}
}
return score;
}
}

At this point all four tests pass. And NOW, for the first time, we have clear duplication that we can factor out. We have two different ways of tallying the totals. The cool thing about this approach, is that the first time we introduce a loop through an array of ints, it's the right loop. We never needed to just loop through all the ints and tally them, so iterating from 0 to rolls.length was never required.

So now, we want to factor out the duplication in very small steps, with green bar between each. Here's a first step:
public int score() {
score = 0;
int roll = 0;
for (int frame = 0; frame < 10; frame++) {
if (handlingSpares) {
score += rolls[roll] + rolls[roll + 1] + rolls[roll + 2];
} else {
score += rolls[roll] + rolls[roll + 1];
}
roll += 2;
}
return score;
}

All the tests pass. Next we replace handlingSpares with the spare calculation:
public int score() {
score = 0;
int roll = 0;
for (int frame = 0; frame < 10; frame++) {
if (rolls[roll] + rolls[roll + 1] == 10) {
score += rolls[roll] + rolls[roll + 1] + rolls[roll + 2];
} else {
score += rolls[roll] + rolls[roll + 1];
}
roll += 2;
}
return score;
}

All the tests pass. At this point we can get rid of handlingSpares entirely and make score local to the score() method. Always running the tests between each step. This leaves the code looking like this:
public class Game {

private int[] rolls = new int[21];
private int currentRoll;

public void roll(int pins) {
rolls[currentRoll++] = pins;
}

public int score() {
int score = 0;
int roll = 0;
for (int frame = 0; frame < 10; frame++) {
if (rolls[roll] + rolls[roll + 1] == 10) {
score += rolls[roll] + rolls[roll + 1] + rolls[roll + 2];
} else {
score += rolls[roll] + rolls[roll + 1];
}
roll += 2;
}
return score;
}
}

This is pretty much how the code usually ends up after solving for spares, and before refactoring out methods like isSpare(int roll) and nextRollForSpare(roll).

So there it is. I had a lot of fun with this experiment. Perhaps for some of you this is news, perhaps not so much for others. For me it was. For me this changes the way I think about the TDD cycle. For those of you for whom this IS news, I encourage you to experiment with it and post your experiences. The notion of prefactoring to add a new feature runs counter to the TDD flow. It's Test-Code-Refactor, not Refactor-Test-Code. Prefactoring is designing up front, and as with all designs up front, sometimes you get it right. Sometimes not so much.

!commentForm
 Sat, 25 Mar 2006 02:15:00, Uncle Bob, Firefly
There is a step above that puzzles me. When you first introduce the for loop, you are already looping by frames.
for (int frame = 0; frame < 10; frame++) 
How did you know to do that? Indeed! Why did you think that an array was needed at all? There must have been some design thought being applied at that point.

The situation you pose corresponds to slide 29 of the Bowling Game Kata. At that point we write the first test for a spare, and note that it fails. Then, in slide 31 we are tempted to use a flag, similar to your "handlingSpares" flag. But we reject that decision because, on slide 32, we note something that you seem to have noticed as well, that the score() method should be calculating the score instead of having the roll() method calculate the score. You made the same decision because you put the if(handlingSpares) statement in the score method. How did you know to do that? Why didn't you just put if (handlingSpares) score = 150; in the roll() method? You must have had a small flash of design insight telling you that score() should be your focus. Were you prefactoring?

In slide 38 of the kata, we finally realize that the monotonic loop through the array is insufficient because we can't reliably detect a spare. So in slide 40 we solve this by iterating through the array one frame at a time. This seems less a prefactoring and more a realization that our loop is in the wrong form.

Anyway, I think this kind of deep introspective thought on how and why we write code is very important. The older and more experienced I get, the more I seem to be focussing away from big design issues and more on individual coding decisions like these. Perhaps that's because I'm very comfortable with big design issues and so they don't occupy a lot of my thoughts. If that's true, it's scary, because it means that I left code as the last challenge to master. Shouldn't it have been the first?
 Sat, 25 Mar 2006 07:25:20, David Chelimsky, re: Firefly
Caught red-handed! Yes, there was some design thought at the point that I chose to add an array. And, naturally, all of my experience dealing with this exercise informed me of that. It also informed me on the issue of where to calculate the score. I imagine that a more naive self might have written this first:
public void roll(int pins) {
if (handlingSpares)
score = 150;
else
score += pins;
}

and moved it to score later.

One way I like to think about TDD is as a game that you play against your inner designer. You can't hide from him completely. In fact you can't succeed without him. But you want to keep him in check (avoiding over design) and you do that by pushing design decisions further back, forcing yourself to write more tests before you write more code.

In this example, I wait a little longer to introduce the array than in the kata. Sure, when the array IS introduced, I could have added more conditionals:
public void score() {
if (rolls[2] + rolls[3] == 10)
score = 150;
else if (rolls[2] == 5)
score = 20;
return score;
}

That might be a natural extension of the "design checker" in me who introduced "handlingSpares" to begin with. So I guess I let the designer take control of that step.

Ultimately, what I'm exploring here is the tug of war between these two characters: the inner designer and the design checker. Both of them are important players. When one is getting too powerful, the other takes over for a bit. And at the end of the game, you've only won if both are still standing.
 Wed, 29 Mar 2006 12:09:59, Clint Shank, Duplication
"So what's the next step? Refactor? Well, it might be, but there's not really much clear duplication here (at least I don't see it yet)"

I think there is clear duplication at this point. The duplication is between the test and the code: the numbers 5 and 150 to be exact. The first time you saw this type of duplication was when you wrote the first all gutters test:
assertEquals(0, game.score());

To get that to pass, you returned 0. Duplication. Refactor. Write all ones test.

I haven’t tried it, but I wonder how the code would have evolved if you would have recognized that duplication.
 Wed, 29 Mar 2006 16:08:52, David Chelimsky, duplication between the tests and thecode
Cool! I'm going to give that some more thought. Thanks Clint!
 Fri, 14 Apr 2006 18:28:33, Robert DiFalco[?], The Theocracy of Programming
"...and as with all designs up front, sometimes you get it right. Sometimes not so much."

I'm confused. Why can't design been in this iterative cycle of revision as well? I don't see much of a difference between doing the test first or the design first; the former is design in the small (for nice small things like a bowling game) and the second is design in the large; say for something that needs a little more thought like a Distributed Replication Server with tons of requirements. Different jobs needs different approaches. I find this trend against design or thinking upfront really naive and distasteful. Sadly, I've had to work on some systems that have been created with the recent trendy thinking of Do the Simplest Thing That Works and Up Front Design Is Bad. The number of disparate abstractions and over all code complexity can be just awful; flying in the face of those principles of object oriented design that Uncle Bob has done such a great job of educating people on.

Uncle Bob, For your comment:

"The older and more experienced I get, the more I seem to be focussing away from big design issues and more on individual coding decisions like these."

I'd ask you this. Do you work on systems that are as large as those you have worked on in earlier years? Is your time on a project as long as it used to be? Seeing how the code morphs and/or rots over time? How easy it is to extend when marketing adds new requirement or wants to move in a new direction? Or is it possible that you are doing more teaching now which necessarily uses smaller examples that lend themselves more to "design in the small techniques" and are shorter lived? Either way I've learned a lot from you; but this concept of shunning design up-front just doesn't work for the type of systems I need to build. TDD should be a tool in the toolbox, not a religion. Same with design up front.
 Sat, 15 Apr 2006 08:52:43, David Chelimsky, sometimes you get it right
Robert,

Please note that I said "sometimes you get it right".

When we talk about getting the design wrong we're not talking about being bad designers (i.e. given the existing requirements, we wrote a bad design), but rather the design doesn't support requirements that appear later. This exercise is an experiment in taking the larger concept of just-in-time design and applying it at a much more granular level. The comment at the end was intended to simply tie that all together.

I agree that doing The Simplest Thing That Could Possibly Work can lead to complexity and disparate abstractions, but TDD isn't just TSTTCPW. When the resulting design is complex or lacking in cohesion, it's our responsibility to address those issues. When we fail to do so we fail to do TDD well, and the result is the sort of system that you're talking about.
 Sat, 15 Apr 2006 12:22:33, Robert DiFalco[?], Just like code, designs evolve, so why avoid design
David,

I appreciate your point. I guess what I mean is that design shouldn't have to be right the first time. For a smaller system where I do employ a test-first approach I find the same thing with my tests. Sometimes their approach is wrong the first, second, or third time. I find that as I am writing the test that there is a much easier or simple or even more correct way to use the objects and thus interfaces change. This is obviously a good thing. However, for some reason the new generation think about design differently. Why? Why frame it as DUF or even BDUF instead of just "design" that is iterated and refactored just like a test or code might be? Different kinds of problems require different approaches. Some problems work great with the TDD flow of Test-Code-Refactor, many other problems work better with Design-Test-Code-Refactor/Repeat. Still other problems work best with the Fred Brooks approach of throwing the first one away. That kind of flow might look like Test-Code-Refactor-Oh shit-Design-Repeat. In this last flow, the refactoring of the code leads to an oh-shit moment where you realize that you've coded yourself into a corner or have created an amorphous morass of human insensitivity so you need to completely step away from the code, clear your head, and draw a bit on the white board before starting over again.

As a human race, we often want to be "saved" by a new idea, discipline, philosophy, et al. We see a new approach as discounting instead of augmenting previous approaches. As such, you can read through any XP centric mailing list or forum and see over and over again an effort to "disprove" the need for design in any situation. It's even on this sight, consider some of the posts here: http://www.butunclebob.com/ArticleS.UncleBob.JoelOnXp. Uncle Bob seems to be saying that DUF is NOT a bad thing and then gets some replies that "I don't mind DUF" or that DUF implies you are not making progress with working software. While it is clear to Uncle Bob (I think) that DUF and BDUF do not speak against "design" or "specification" (when needed), it is also clear that most XPers use the XP discipline to excuse the lack of any design or analysis beyond tests and refactoring. After all, even if you are building a "Dog House" you will put some thought into what it will look like before building it.

So I guess my point is that I admire the new religion; but it is not without flaws. I like to pepper with a bit of that "old time religion" and draw from my OO roots. So I'll say it again, different problems require different approaches. TDD is *not* always the right solution; but often times it can be. We are software engineers, we shouldn't be as one-sided as Sean Hannity. :)

We don't avoid writing tests or code because they evolve, why avoid design?