That Ugly Get

I posted for help on converting the newlines ArrayList to an array of String, and got a reply that showed how to do it. The trick was the "typeof" function, which I had forgotten about since C days so long ago. So, with nothing else to do after dinner, I decided to clean up the code a bit more. Remember that we had:

    public String[] Lines {
      get {
        // there must be a way to cast this??
        String[] textlines = new String[newlines.Count];
        for (int i = 0; i < newlines.Count; i++) {
          textlines[i] = (String) newlines[i];
        }
        return textlines;
      }
      set {
        lines = new ArrayList(value);
        newlines = lines;
      }
    }

It's that ugly "get" of the Lines property that I'm trying to fix. Sure enough, this code works just fine:

    public String[] Lines {
      get {
        return (String[]) lines.ToArray(typeof(String));
      }
      set {
        lines = new ArrayList(value);
      }

I don't quite see why both the String[] and the typeof() are necessary, but it turns out that they are. So that's much better.

Get Rid of the Text Copy

Recall that the InsertParagraphTag method makes a copy of "lines" into "newlines", with this code:

      newlines = new ArrayList();
      newlines.AddRange(LinesThroughCursor());
      newlines.AddRange(NewParagraph());
      newlines.AddRange(LinesAfterCursor());

We should be able to do this edit directly into the "lines" variable, using InsertRange instead of AddRange. To make it work completely, however, is a pretty big edit: there are places all over that rely on newlines. So I decided that first I would make the InsertRange work, and only then remove "newlines". So I wrote this:

      newlines = new ArrayList(lines);
      newlines.InsertRange(LineContainingCursor()+1, NewParagraph());
	  	

That leaves the result in the newlines variable, so this is the only code that has to change to test things. And sure enough, it worked just fine! The next step was just to replace all references to "newlines" with references to "lines". I did a few of those with a quick text find -- where, oh where, is Smalltalk's "Browse References"? Then I removed the instance variable "newlines", compiled (finding one more reference), and the program still runs all the tests, even the acceptance test that we haven't told you about yet. I wanted to put this article out first while it was fresh in my mind, and the acceptance tests aren't quite done yet. I'll want to work with Chet on those, when I get back from a gig next Thursday. Unless I just can't resist.

Observations and Conclusions

Take a look at the InsertParagraphTag method as it now stands:

    public void InsertParagraphTag() {
      //
      // On Enter, we change the TextModel lines to insert, after the line containing 
      // the cursor, a blank line, and a line with <P></P>. We set the new cursor
      // location to be between the P tags: <P>|</P>.
      //
      // handle empty array special case (yucch)
      if ( lines.Count == 0 ) {
        lines.Add( "<P></P>" );
        selectionStart = 3;
        return;
      }

      lines.InsertRange(LineContainingCursor()+1, NewParagraph());

      // set cursor location
      selectionStart = NewSelectionStart(LineContainingCursor() + 2);
    }

Frankly, I think that's getting downright clean. And each step from the original ugly version to this one was a simple refactoring, supported by tests that told us that we weren't breaking anything. (Well, OK, the first couple of refactorings weren't test-supported. That was risky, but I got away with it this time. Don't try that at home. Since then, things have gone faster and we've been much more confident. I even felt pretty safe working alone on this chapter, because the tests Chet and I had devised were working for us even when he was home with his wife, where he belongs on a Saturday.

Now, the most irritating part of the InsertParagraphTag method is that special case for an empty array. I'd like to clean that up sometime, but not right now. And the selectionStart doesn't satisfy me entirely. I'm tempted to move that searching code to the get method that returns the variable. That would let me remove this line. But I'm not sure that will work, because the cursor setting depends on the size of the tag inserted. I'll talk about that with Chet next time we get together. It's a pretty small wart, I think.

Reflection

Think back, if you will, to how we built this method. Originally it was written inline in the OnKeyDown method of the Form. It was pretty ugly then, and we put all those comments in it hoping to make it better. Then, in a few short sessions -- none of them over two hours -- we cleaned it up, using these techniques:

1. Extract Class -- to create the TextModel class.
2. Extract Method -- to give good names to the loops that copied and added.
3. Extract Method -- was also used to clean up a few other things.
4. Substitute Algorithm -- to move to ArrayList, and again to get rid of newline
5. Replace Magic Number with Symbolic Constant -- (sort of) to move the return and paragraph tag out of the mainline.

... and probably more. Each change was pretty simple and almost all of them worked the first time. The result is very clean code.

It is possible to imagine having done better with forethought. Hindsight is like that. What we'd like you to take away from this experience is that -- believe us -- it almost always works out this way. We begin with simple, straightforward, almost procedural code. At this point we're working to understand how to do the thing at all. We usually write more tests up front, as you'll see in later chapters, and we usually express intention a bit more than we did here. But we always wind up with code that wants a bit of refactoring to be really nice. And the refactoring always goes like it did this time: one little step after another, each one going along quite well. Once in a while, a refactoring won't work or will be a bad idea. Since it's a small step, we just back it out.

Chet and I have only been working on this a couple of hours at a time. There isn't much time in it at all, and it's looking pretty good!!

The Key Lesson

This style of programming works better for us -- and for most people who try it -- than doing a whole lot of design up front and trying to get it all right the first time. The lesson to take home is that this approach is worth trying. Push it as far as you can, especially when you're doing things where you're comfortable. We think you'll find that the optimal point has far less up front design than you would have expected, and that the whole process is very easy and stress-free.

Current Code

using System;
using System.Collections;
using System.Text;

namespace Notepad {
  class TextModel {
    private ArrayList lines;
    private int selectionStart;

    public TextModel() {
      lines = new ArrayList();
    }

    public void InsertControlPText() {
    }

    private int LineContainingCursor() {
      int length = 0;
      int lineNr = 0;
      int cr = Environment.NewLine.Length;
      foreach ( String s in lines) {
        if (length <= selectionStart
          && selectionStart < length+s.Length + cr )
          break;
        length += s.Length + cr;
        lineNr++;
      }
      return lineNr;
    }

    public void Enter() {
      InsertParagraphTag();
    }

    public void InsertParagraphTag() {
      //
      // On Enter, we change the TextModel lines to insert, after the line containing 
      // the cursor, a blank line, and a line with <P></P>. We set the new cursor
      // location to be between the P tags: <P>|</P>.
      //
      // handle empty array special case (yucch)
      if ( lines.Count == 0 ) {
        lines.Add( "<P></P>" );
        selectionStart = 3;
        return;
      }

      lines.InsertRange(LineContainingCursor()+1, NewParagraph());

      // set cursor location
      selectionStart = NewSelectionStart(LineContainingCursor() + 2);
    }

    private int NewSelectionStart(int cursorLine) {
      int length = 0;
      for (int i = 0; i < cursorLine; i++)
        length += ((String)lines[i]).Length + Environment.NewLine.Length;
      return length + "<p>".Length;
    }

    public ArrayList LinesThroughCursor() {
      return lines.GetRange(0,LineContainingCursor()+1);
    }

    public ArrayList NewParagraph() {
      ArrayList temp = new ArrayList();
      temp.Add("");
      temp.Add("<P></P>");
      return temp;
    }

    public ArrayList LinesAfterCursor() {
      int cursorLine = LineContainingCursor();
      return lines.GetRange(cursorLine+1,lines.Count - cursorLine - 1);
    }

    public String[] Lines {
      get {
        return (String[]) lines.ToArray(typeof(String));
      }
      set {
        lines = new ArrayList(value);
      }
    }

    public String TestText {
      get {
        StringBuilder b = new StringBuilder();
        foreach(String s in lines) {
          b.Append(s);
          b.Append(System.Environment.NewLine);
        }
        b.Insert(SelectionStart,"|");
        return b.ToString();
      }
    }

    public int SelectionStart {
      get {
        return selectionStart;
      }
      set {
        selectionStart = value;
      }
    }
  }
}

using System;
using NUnit.Framework;

namespace Notepad {

  [TestFixture] public class TestTextModel : Assertion {

    private TextModel model;

    [SetUp] public void CreateModel() {
      model = new TextModel();
    }

    [Test] public void TestNoLines() {
      model.Lines = new String[0];
      AssertEquals(0, model.Lines.Length);
    }
    [Test] public void TestNoProcessing() {
      model.Lines = new String[3] { "hi", "there", "chet"};
      AssertEquals(3, model.Lines.Length);
    }
    [Test] public void TestOneEnter() {
      model.Lines = new String[1] {"hello world" };
      model.SelectionStart = 5;
      model.InsertParagraphTag();
      AssertEquals(3, model.Lines.Length);
      AssertEquals(18, model.SelectionStart);
    }
    [Test] public void TestEmptyText() {
      model.Lines = new String[0];
      model.InsertParagraphTag();
      AssertEquals(1, model.Lines.Length);
      AssertEquals(3, model.SelectionStart);
    }

    // Chet comments that he hates the comments

    [Test] public void InsertWithCursorAtLineStart () {
      model.Lines = new String[3] { "<P>one</P>", "", "<P>two</P>"};
      model.SelectionStart = 14;
      model.InsertParagraphTag();
      AssertEquals("<P>two</P>", model.Lines[2]);
    }

    [Test] public void TestLineContainingCursorDirectly() {
      // todo?
    }
  }
}