Judgment Call

Earlier I was complaining about the AltS method having just that simple InsertSectionTags method call. Take a look at the InsertSectionTags method now. We could eliminate that method entirely, by putting its InsertTags call up in AltS. That would change this

 public void AltS() { 
 InsertSectionTags(); 
 } 
 public void InsertSectionTags() { 
 InsertTags(newSection, sectionSkip); 
 } 

and would give us this:

 public void AltS() { 
  InsertTags(newSection, sectionSkip);  
 } 

We need to decide if that code communicates sufficiently what s going on. My judgment is that it does, so I ll leave it that way. On the other hand, I m working without a pair, who might not agree. And you might not agree: that s why I call it a judgment call. For now, I like it, and I ll do the other method the same way:

 public void Enter() { 
  InsertTags(newParagraph, paragraphSkip);  
 } 

I run the tests...and they don t run!!! It turns out there are three tests that were calling InsertParagraphTag directly. Now the decision is harder. If I change this one back, I really should change the other one, or else the code here will look odd. Or I can change the tests to send Enter instead of InsertParagraphTag. That s not clearly the right thing to do. Let s look at the tests to see whether doing that will make them more confusing.

 [Test] public void TestOneEnter() { 
 model.SetLines(new String[1] {"hello world" }); 
 model.SelectionStart = 5; 
 model.InsertParagraphTag(); 
 AssertEquals(2, model.Lines.Count); 
 AssertEquals(16, model.SelectionStart); 
 } 
 
 [Test] public void TestEmptyText() { 
 model.Lines = new ArrayList(new String[0]); 
 model.InsertParagraphTag(); 
 AssertEquals(1, model.Lines.Count); 
 AssertEquals(3, model.SelectionStart); 
 } [Test] public void InsertWithCursorAtLineStart () { 
 model.Lines = new ArrayList(new String[3] { 
 "<P>one</P>", "", "<P>two</P>"}); 
 model.SelectionStart = 14; 
 model.InsertParagraphTag(); 
 AssertEquals("<P>two</P>", model.Lines[2]); 
 } 

Again, judgment. I feel that I like the compression of the TextModel class enough to live with the slightly less clear tests. And the first one is called Enter and then calls InsertParagraphTag, so it will actually be improved.

But wait! There s another reason why this should NOT be done, however. The InsertParagraphTag and InsertSectionTags methods are logical behavior, and the AltS and Enter methods are physical : they represent the particular choice of which keystroke should do which function. And I m reminded that we have already changed AltS once, from ControlS. That s enough to convince me to go back to the previous arrangement with AltS calling InsertSectionTags and so on. A little undo, and we should be all right. Hold on...bunch of Ctrl+Zs, compile and run tests...all back the way it was.

Lesson  

Is there a lesson here? Well, probably a few (in no particular order):

• Too tight a linkage between user actions and system actions is probably a bad idea.

• A partner might have helped me not make the mistake.

• My tests already embodied the logical/physical distinction and gave me an important clue.

• It was easy to do the experiment, and easy to undo it, because I was going in tiny steps.

• Don t be afraid to change your mind.

Depending on your original view, you might also offer another lesson: don t fall too much in love with tight code at the expense of clarity.