Enhancing the Customer Tests

Yesterday we got our first Customer Test working, and it was clear that our customer (that's us) could do more. So we did a couple more and sure enough they all ran. But we knew there was something we couldn't test, so we came up with this test as an example:

*input
<P>This is the first p|aragraph.</P>

<P>This is the second paragraph.</P>
*end
*enter
*output
<P>This is the first paragraph.</P>

<P>|</P>

<P>This is the second paragraph.</P>	
		

The tricky bit is in the first line. See that vertical bar? The customer wants to write a test that shows that if the cursor is in the first line, and he types Enter, the editor inserts a blank paragraph tag in between the first and second line, with the cursor in the middle of it. Now, it turns out that the TextModel actually works that way: we can run the GUI and show it. But we can't test that feature of the TextModel, because we don't set the SelectionStart property when we read the input.

Our mission is simple. We need to parse the input lines a little bit, remove the vertical bar from the input (because it isn't really there, it's just a cursor), and remember where it was, so we can set the SelectionStart property.

The following are the results of that trial, and trial it was.

Do You Have a Test for That?

We wrote that Customer Test, and sure enough it didn't work. No surprise there. I was all for fixing it, but Chet asked "Should we write a failing Programmer Unit Test first? Well, of course we should. So we looked at our SetInput method, the one that reads the input from the input stream and sets it into the TextModel. It goes like this:

    private void SetInput(StringReader reader) {
      String[] input = ArrayToEnd(reader);
      model.Lines = input;
    }

We figured it needed to go something like this:

    private void SetInput(StringReader reader) {
      String[] input = ArrayToEnd(reader);
      model.Lines = CleanLines(input);
      model.SelectionStart = CursorLocation(input);      
    }

CleanLines would return a new array of lines with the line containing the vertical bar fixed. And CursorLocation would return the cursor location by counting characters up to the vertical bar. No problem.

The First Test Worked

So we wrote a little unit test, right in the CustomerTest file, that tested a new method, CursorLocation, that answered the cursor location given an array of lines. It just went over the array and counted characters until it found the vertical bar character.It took a little while to get that running, but just a little. Then we went to lunch, at the Lazy Lizard. (No, we aren't getting paid for these plugs.)

At lunch, I was complaining because that new SetInput method was going to loop over the lines twice, once to get the clean lines, and once to get the cursor location. Obviously, that was inefficient. Where was my partner??? What were we thinking??? Everyone knows you don't work on efficiency until you have the code working. Besides that, the candidate code for doing it in a single function looks worse, not better:

    private void SetInput(StringReader reader) {
      String[] input = ArrayToEnd(reader);
      int cursorLocation;
      String[] newLines;
      CleanInput(input, out cursorLocation, out newLines);
      model.Lines = newLines;
      model.SelectionStart = cursorLocation;
    }

We just sketched this code, however, rather than typing it in. So we didn't really see how ugly it was. And we couldn't see how to write a decent unit test for it, and we had this perfectly good acceptance test, so we pushed ahead. Mostly I pushed ahead. I think after a while Chet was swimming as I kept thinking I was just a few more lines away from making it work. The breakdown came when I converted one method to return the new records, thus destroying its ability to be used for whatever it had done before. It was too much. And since we're working without much of a code manager, and we hadn't backed up the files we were doomed. I knew that if I would just write one more method, I could make it work. But I realized that even if I did, I could never explain it to you tonight.

There's Only One Thing to Do

Well, when the code is well and truly screwed, there's just one thing to do: back it out. The easiest way was to hold down control-Z (undo) until the code was backed up to a known good point. So we did that, then took a little time to reflect. It's important to do this when things go wrong. You won't always have the right idea for what's next, but often you will. Then sleep on it and see what you have tomorrow.

I like to think that Chet and I are pretty calm about throwing code away -- we've thrown away so much. It always hurts, but only until you do it. Then it's like putting down a big bucket of water: a great relief. So we thought, and Chet reflected back on something he had said earlier in the process. We kept having to write procedural code inside the CustomerTest to manipulate the strings that make up a test. He had commented that the object wasn't helping us much, but we never followed up on it. This time we did.

We Need an Object

Look at that code up there for SetInput. Either we have a two-pass algorithm, or we have a complex setup to call a function that will be hard to write ... and may still have a two-pass algorithm in it. Maybe I wasn't so wrong after all. The two-pass thing isn't as much a concern over speed, perhaps, as over our ability to express ourselves: we can't say what we want when we do it this way.

So we posited a new object that holds input. We called it an InputCommand, and our proposed code looks like this:

    private void SetInput(StringReader reader) {
      InputCommand input = InputCommand(reader);
      model.Lines = input.CleanLines();
      model.SelectionStart = input.SelectionStart();

Now that's clean! So there's this object, and it will take the input from the reader that holds the whole script, It will know how to produce the clean lines (the ones without the vertical bar), and it will know how to say where the SelectionStart (cursor location) is. We think we like this. And we're pretty sure that it will be easy to write. Tomorrow we're going to start on it.

Lessons Relearned

If You Can't Test It, It's Wrong

When we realized we couldn't readily test this code, we should have stopped coding. The lack of testability was telling us, long before anything else did, that there was at least one missing object.

Procedural Code Is a Very Bad Smell

When you find yourself writing procedural methods and utility methods, and railing at how the objects aren't helping you, this, too, is telling you that there's a missing object.

Too Long Between Green Bars

After we made our one new unit test work, we went for a long time without a new test or a new green bar; basically all the time after lunch. This is at least a sign that you're stuck and should be used to trigger a rewind and a start in some new direction. Or even to start over in the same direction.

Conclusions ... for Now

We screwed up. We finally figured out that we had. We backed up to last known good ... maybe even going a little further back than we felt we had to. We did a quick retrospective, then called it a day. Maybe we wasted two or three hours ... but I bet when we get together tomorrow, it'll go quite smoothly. Wait and see!