Recap

Back in "Warts and All", you recall that we set up an acceptance test that allowed the Customer to say where the cursor was in the input, and to condition the output based on that result. Then we went down a rat hole and backed out the code. We had identified that we needed an object, and sketched the way that object would be used. Then, last time, we worked a bit on code management, because we had to back the code out manually.

The object we wanted was called "InputCommand", and the idea was that it would be able to answer the clean input (the input except with the cursor position indication removed), and the selection start, (the location of the input cursor). The code we wanted to make work looked like this:

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

At this point, there is no InputCommand class, so this is all just speculation. We set out to create this class. Therefore we needed a test. Here's the first test we wrote:

    [Test] public void EmptyCommand() {
      command = new InputCommand();
      Assert(1==1);
    }

Once we made that work, by creating an empty class named InputCommand, we extended the test to say:

    [Test] public void EmptyCommand() {
      command = new InputCommand();
      AssertEquals(0, command.CleanLines().Length);
    }

The idea is that there's no input, so that CleanLines will return an array of String of length zero. We built a CleanLines method that returned new Striong[0], which made that work. So far, so good.

We like writing these trivial tests. It's tempting to really "do" something, but, as we learned in the Warts and All article, we get into trouble when we try to do too much. Starting every class this same way gives us a nice rhythm, and we rarely get into trouble. Better to stay simple, we find.

But now we have to do some work. Looking at the place where we actually intend to use this code, we see that the InputCommand wants to be driven from a StringReader. So we wrote a new test requiring a StringReader, thusly:

    [Test] public void OneLineCommand() {
      String oneLineString = 
@"one line
*end";
      StringReader reader = new StringReader(oneLineString);
      command = new InputCommand(reader);
      AssertEquals(1, command.CleanLines().Length);
    }

No biggie. We're getting better at knowing how to work C#, though there's probably a better way to do this. We create a string that matches some legal input, we create a StringReader on it, se create an InputCommand on that, and assert that we get one line back from CleanLines(). That test doesn't even compile, because InputCommand doesn't have a constructor on StringReader. We debated whether to write a second constructor, to learn about overloads, but decided that was a digression. So we changed our first test to use a StringReader also, like this:

    [Test] public void EmptyCommand() {
      command = new InputCommand(new StringReader(""));
      AssertEquals(0, command.CleanLines().Length);
    }

Fine. We have two tests that won't compile, because there's no such constructor. So we build the constructor:

    private ArrayList lines;

    public InputCommand(StringReader reader) {
      lines = new ArrayList();
      String line = reader.ReadLine();
      while (line != null && line != "*end") {
        lines.Add(line.TrimEnd());
        line = reader.ReadLine();
      }
    }

We decided to use an ArrayList for the member variable lines, which was what we were using internally in the original CustomerTest. So we just read to end, and add the lines to an array. And we implemented CleanLines to return the ArrayList. There was a little discussion on that: the rest of the code isn't clear on whether it wants arrays of strings or ArrayLists, and we decided that we like ArrayList better so that we'd deal with any changes that we needed. The first change we needed was to change the .Length calls in our tests to .Count, because in their wisdom, Microsoft uses those two words to mean pretty much one thing. So now our two tests look like this::

    [Test] public void EmptyCommand() {
      command = new InputCommand(new StringReader(""));
      AssertEquals(0, command.CleanLines().Count);
    }

    [Test] public void OneLineCommand() {
      String oneLineString = 
@"one line
*end";
      StringReader reader = new StringReader(oneLineString);
      command = new InputCommand(reader);
      AssertEquals(1, command.CleanLines().Count);
    }

And now the InputCommand CleanLines method looks like this:

    public ArrayList CleanLines() {
      return lines;
    }

Instead of the earlier version which was answering an empty array of String. At this point, all our tests run. We did a little Ann Anderson goal line victory dance and then got back to work. The thing is, the lines aren't really clean. CleanLines() is supposed to strip out any vertical bars (the cursor location indicator) that are in the input. We haven't written a test that calls for that. So we write it:

    [Test] public void OneDirtyLine() {
      command = new InputCommand(new StringReader("a|b\n*end"));
      AssertEquals("ab", command.CleanLines()[0]);
    }
  }

That should be enough to do it. We put in "a|b" and expect "ab" back.

Let's emphasize again how simple and tiny our tests have been. Certainly we could have put the vertical bar into the second test. If we had, however, we would not have been able to just answer lines back as the result. Now returning lines is simply wrong, as we'll see in a minute. But in fact we had lots of work to do, with the constructor and adjusting .Length to .Count and so on.

We didn't foresee that there would be enough to do without the added complexity of the vertical bar: we just try always to take tiny little steps. It usually makes the job easier, by requiring us to worry about less and keep less in mind. This was just one of those times.

Cleaning the Lines

Anyway, now that we have the OneDirtyLine test, our mission is to produce an ArrayList of lines with no vertical bars in them. (Here again, we're skipping something. We also need to be answering where the cursor-indicating vertical bar was. But we're not thinking about that. So stop thinking about that.) We futzed a bit about how to do CleanLines. One idea was to calculate it right in the CleanLines method. I was going that route, but Chet suggested that we just create the clean lines right as we built the input. So we did that. With our test not running, we changed the constructor to look like this:

    public InputCommand(StringReader reader) {
      lines = new ArrayList();
      String line = reader.ReadLine();
      while (line != null && line != "*end") {
        lines.Add(line.TrimEnd());
        line = reader.ReadLine();
      }
      CleanTheLines();
    }

Note that we added the method CleanTheLines. We're expressing intention here. We also discussed that the constructor needs refactoring now. Since it has two things that it does, setting up lines (which it does manually) and setting up the clean lines, which it does by sending a message, we should make that first bit of code into a method also. But that's not for now: we have our adding functionality hat on, not our code cleaning hat. So we wrote CleanTheLines, like this, adding a new member variable to hold the clean lines:

    private ArrayList cleanLines;
		
    private void CleanTheLines() {
      cleanLines = new ArrayList();
      foreach ( String line in lines) {
        cleanLines.Add(CleanTheLine(line));
      }
    }

(Naturally, the member variables aren't spread around in the code like I'm showing them here: they're up at the top of the class. Just didn't want you to worry about that ...)

Well, we expressed intention again in the above method. We went over each line, and added a clean version of that line to the ArrayList. Let's underline that for a moment. By expressing intention here, we produce code that is more expressive, but we also limit what we have to think about all at the same time. When I'm in "C programmer" mode, I'd be more inclined to write the foreach loop there, and then inside it try to expand out what CleanTheLine would be. I'm sure I'm up to that challenge, most of the time. But sometimes I'm not. And, again, when we work in these tiny steps, we find that we're more relaxed, and more able to deal with the distractions that inevitably occur, especially when you're sitting in the Michigan Union. So now to write CleanTheLine(). We got smart, and typed line. (line dot) in the editor. The dot brings up the list of methods on the object (thanks, Visual Studio), and we scrolled through the list until we found Replace, whose writeup says: "Replaces all occurrences of a specified string in this instance, with another specified string". That sounds just right. (I just thought of something to worry about: that method appears to be changing the string in place. That might actually be messing up our input lines. I'll test that later. Since our code is working, as you'll find out by the end of the article, there's surely something to learn here.) Anyway, we coded CleanTheLine this way:

    private String CleanTheLine(String dirty) {
      return dirty.Replace("|", "");
    }

That makes our test run! We've now got CleanLines() working. So we decide to put the InputCommand into the CustomerTest class as well:

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

Note that we didn't put the SelectionStart stuff in, since it isn't built yet, but we wanted to see if the string part of the customer tests still worked. Surprisingly, all our customer tests worked, including this one, which we thought shouldn't have:

*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>

We were surprised by this, because we expected that we needed the selection start logic to figure out where the cursor was. But the default cursor location is zero, so that since we didn't set it explicitly, our TextModel assumed that the cursor was on the first line, and worked correctly. Oops. We wrote another customer test to verify this theory, and sure enough this doesn't work:

*input
<P>This is the first paragraph.</P>

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

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

<P>|</P>

Good. We thought we were wrong, and it turns out we were.

Building SelectionStart

I wanted to go ahead and build SelectionStart, but Chet asked if I was going to write a programmer unit test first. I of course agreed. This is a good habit to have, and especially in this new language it's good to practice good habits. In the larger scheme of things, customer tests may take longer to run and be run less frequently than programmer tests. Furthermore, programmer tests are generally point more directly to what the problem is when they fail. Therefore, when a customer test fails, the wise programmer writes a failing programmer test, then makes it work. So we added a line to our OneDirtyLine test, like this:

    [Test] public void OneDirtyLine() {
      command = new InputCommand(new StringReader("a|b\n*end"));
      AssertEquals("ab", command.CleanLines()[0]);
      AssertEquals(1,command.SelectionStart());
    }

This is enough to drive a trivial implementation of SelectionStart that returns 1, and then we added a line to our OneLineCommand test. We figured that since it didn't have a cursor indicator, the answer should be at the end of the lines.

    [Test] public void OneLineCommand() {
      String oneLineString = 
@"one line
*end";
      StringReader reader = new StringReader(oneLineString);
      command = new InputCommand(reader);
      AssertEquals(1, command.CleanLines().Count);
      AssertEquals(10,command.SelectionStart());
    }

That was enough to get us to write the following (with a little help from similar code that's elsewhere in our little system):

    public int SelectionStart() {
      int charactersSoFar = 0;
      foreach (String line in lines) {
        int index = line.IndexOf("|");
        if (index != -1)
          return charactersSoFar + index;
        else
          charactersSoFar += line.Length + Environment.NewLine.Length;
      }
      return charactersSoFar;
    }

Now we really did type this in in one go. We had written a bunch of line-tracking loops recently, and this one just fell trippingly from my fingers. We keep track of how many characters we have read so far (which will be the number of characters in all the lines ahead of the line containing the vertical bar cursor indicator). When we find the first line with a vertical bar, we return the characters so far, plus the index of the cursor. That'll be the position in the whole "file" of the vert. If the line doesn't contain the vert, we update CharactersSoFar.

Both our unit tests ran. However, most of the customer tests threw exceptions: index outside array. We fumbled with this a bit, then figured out how to tell from NUnit where the exception was thrown. We finally realized that what had happened was that when we run off the end of the list without finding the vert, we set the selection start after the newline on the last line. Bad things happen, because the TextModel thinks the cursor is in the next line after the last line. So we realized that if SelectionStart finds no cursor, it should assume the cursor is in the last line, not after it. So we have to back out the last NewLine. The new SelectionStart is therefore:

    public int SelectionStart() {
      int charactersSoFar = 0;
      foreach (String line in lines) {
        int index = line.IndexOf("|");
        if (index != -1)
          return charactersSoFar + index;
        else
          charactersSoFar += line.Length + Environment.NewLine.Length;
      }
      return charactersSoFar - Environment.NewLine.Length;
    }

And the tests all run! We think our story is actually running now, and that all the customer tests imaginable for this feature will work. We'll probably write a few more, but for today, we're finished.

What We Learned

Well, we certainly confirmed that backing out the code the other day was a good idea, and we confirmed that our feeling that the objects weren't helping us was correct. Notice that the InputCommand object is pretty straightforward, and that if we did the same operations with utility methods on TextModel, as we were trying to do a couple of days ago, the code would be more bulky, more messy, and certainly -- based on actual experience -- more prone to error.

We learned that our reflexes to create new objects need to be more sensitive. There's a lot of hassle in C# (and Java) surrounding building a new class, compared to what we're used to in Ruby. But the need for new objects is just as strong, so we have to be more sensitive to that need.

The payoff for the new object is great. Simple code, nicely partitioned off, easy to test and write. We certainly hope you can sense, both from our expression of the experience, and from the code steps themselves, how much easier it was this time. Took only about 1/2 the original time, by the way, with far less stress.

Cruft Buildup

However, there's some cruft buildup in the system now. There is a general confusion between arrays of strings and ArrayLists, though we cleaned some of this up as we went, by virtue of having chosen ArrayList as the preferred object in InputCommand. Somewhere up there, there were a couple of edits that I didn't mention, where we matched up the interfaces. The TextModel used to take an array of strings in its Lines method, and converted it to an ArrayList -- now it takes an ArrayList. And because the TextForm wants an array of Strings, we had to convert the ArrayList to strings in a new place. We didn't want to go into that, because it interrupted the flow of the narrative more than it interrupted the coding progress. Which brings me to my next point:

Pre-compile Checking for Watts Humphrey

I told Watts at XP/Agile/Universe that we'd try going over our code by eyeball before compiling, shooting for clean compiles. We haven't started doing that yet. but we did talk about it a few times as we went along. The standard XP thing to do is to type in a test, or some code, then compile and run the tests. You let the compiler, or the failed test, direct your attention to the next thing. We were still doing that. We noticed that we would quite typically do something like this:

We changed the return type of CleanLines from String[] to ArrayList. We could have looked through the code to see where the changes needed to be made. Instead, we compiled and then clicked on the compiler errors, which took us in the editor to the lines that needed changing. We fixed them, then went on. Few if any of the compiler errors surprised us. And the very few logic problems we had -- we believe -- would we have detected by eyeball.

So we haven't done the experiment yet, though we remain committed to doing so. But our subjective impression is that while in the olden days we might have printed a listing and looked at it, marking what we saw, the modern tools change the cost/benefit equation. To scrunch through the code looking for places that assume String[] would be time-consuming and subject to human error. To let the compiler do it is faster, and completely accurate. The downside, of course, is that while scanning for those errors, we might well find other errors that the compiler cannot help with. This time, we had very few such errors, and as far as we know, there are none that weren't detected by the tests. (I still have to look at that Replace method, though.)

Right now, our tentative conclusion is that the tools are different enough to move the best strategy in the direction of automation, especially in the presence of so many tests. But we still plan to try it, to find out for sure.