Clearing a Space to Work

Remember that we re starting in the middle here. We have had some false starts, to be described below, and we have accepted that what we did won t work; we have given it enough chances . It s time to start with a better idea. To do that, we need a clean space to work. One way to do this might be to revert the code. I have every version saved in the source manager, and it would be easy to back it up to the point before we started with the Optimized thing. But although it might take a bit longer, I ve decided to go forward by removing the unnecessary code from the current version. I think that will give me a better view of the terrain, in preparation for starting again. It s a judgment call. We ll begin by removing all the old optimization material. You can follow along or skip to the next heading. What you ll see, if you stay with me, is an example of scavenging through bad code trying to preserve some good bits.

Lesson  

From my later perspective, it looks to me as if backing out would have been better. The ideas were all in my head, and the code was at best only partly good. Still, no real harm done. Read on...

I begin by removing the Interface, OptimizedTextModel, and UnoptimizedTextModel from the system. The compiler will tell me what to do next. It says that OptimizedTextModel can t be found, in TextModel s MostRecentlySavedModel method. The method is used in a lot of tests, so I d rather not remove it. I believe, though, that I will add an interface for the Undo operation. I ll call it IUndoRestore, just like before.

 public interface IUndoRestore { 
 } 
 
 and in TextModel: 
 public IUndoRestore MostRecentlySavedModel(){ 
 return (IUndoRestore) SavedModels.Peek(); 
 } 

This triggers a lot of error messages about methods not in IUndoRestore; I m not surprised. I ll remove most of them without remarking here. Some of what needs removal are tests for the old optimize. I ll comment them out for now, but they really need to die. In other cases, I ll just make sure that the test will fail, and then I ll revise it when I know more about what s going on. Here s one example:

 [Test] public void SectionUndo() { 
 ShowLines("empty"); 
 model.Snapshot(); 
 model.InsertTags(model.SectionCommand()); 
 ShowLines("section"); 
 model.Snapshot(); 
 model.InsertCharacter('x'); 
 ShowLines("x"); 
 model.Restore(); 
 ShowLines("no x");  //  
  AssertEquals(2,  
  model.MostRecentlySavedModel().NumberOfLinesToRemove());  
   model.Restore(); 
 ShowLines("no section"); 
 AssertEquals(  99  , model.Lines.Count); 
 } 

The next test I look at tells me that I need to add Restore() to IUndoRestore:

 public void Restore() { 
 Console.WriteLine("Restore {0}", SavedModels.Count); 
 if (SavedModels.Count == 0) return; 
 Console.WriteLine("   non-empty"); 
 IUndoRestore opt = (IUndoRestore) SavedModels.Pop(); 
 opt.Restore(this); 
 } 
 
 which begs: 
 public interface IUndoRestore { 
 void Restore(TextModel model); 
 } 

This, of course, is exactly what we had before, but now we know that we need it.

There are many calls to OptimizeSavedModel in the tests. I hate to remove them, but they are in fact mostly useless, as they are testing that feature and we re not going to have that feature any more. Out they go! The Snapshot() method itself refers to OptimizedTextModel. I change it back to using TextModel, the way it originally did:

 public void Snapshot() { 
 SavedModels.Push(new TextModel(this)); 
 } 

The system compiles. Some unused methods are probably around, but let s see about the tests. Various Undo- related tests fail. I ll look at them and see what to do. I expect that after we fix a few, the rest will work. They are all failing on a cast, in line 313:

 public void Restore() { 
 Console.WriteLine("Restore {0}", SavedModels.Count); 
 if (SavedModels.Count == 0) return; 
 Console.WriteLine("   non-empty"); 
  IUndoRestore opt = (IUndoRestore) SavedModels.Pop();  
   opt.Restore(this); 
 } 

No surprise: TextModel, which is now on the stack, doesn t implement IUndoRestore. I ll fix that:

 public class TextModel : IUndoRestore { 
 ... 
 public void Restore() { 
 IUndoRestore opt = (IUndoRestore) SavedModels.Pop(); 
 opt.Restore(this); 
 } 
 public void Restore(TextModel model) { 
 model.ReplaceAllLines( 
 this.Lines, 
 this.SelectionStart, 
 this.SelectionLength); 
 } 
 ... 

This last method is the callback that we used to have in UnoptimizedTextModel. We ll be calling back to ourselves . This might seem odd, but I think it s right. This is taking a little more time than I would like, a sign that starting over might have been better. Let s see what fails now ”SnapshotLinesIdentity():

 [Test] public void SnapshotLinesIdentity() { 
 model.SetLines(new String[] { "before", "snapshot" }); 
 model.SelectionStart = 6; // after before ;-> 
 AssertEquals("snapshot", model.Lines[1]); 
 model.Snapshot(); 
 model.InsertTags(TextModel.Tags.Pre); 
 AssertEquals("<pre></pre>", model.Lines[1]); 
 model.Restore(); 
 AssertEquals("snapshot", model.Lines[1]); 
 } 

This is failing because it expects snapshotxxx (which I changed just to be sure the test would fail) and gets snapshot. This test is working now!

SectionUndo() is also failing. That s the method I put the 99 into:

 [Test] public void SectionUndo() { 
 ShowLines("empty"); 
 model.Snapshot(); 
 model.InsertTags(model.SectionCommand()); 
 ShowLines("section"); 
 model.Snapshot(); 
 model.InsertCharacter('x'); 
 ShowLines("x"); 
 model.Restore(); 
 ShowLines("no x"); 
 model.Restore(); 
 ShowLines("no section"); 
 AssertEquals(99, model.Lines.Count); 
 } 

The zero is correct, and when we put it back, the test runs. I ll remove all that tracing, and while I m at it, I ll add some sorely needed assertions.

 [Test] public void SectionUndo() { 
 ShowLines("empty"); 
 model.Snapshot(); 
 model.InsertTags(model.SectionCommand()); 
 model.Snapshot(); 
 model.InsertCharacter('x'); 
 model.Restore(); 
 AssertEquals("<sect1><title></title>", model.Lines[0]); 
 AssertEquals("</sect1>", model.Lines[1]); 
 model.Restore(); 
 AssertEquals(0, model.Lines.Count); 
 } 

This test also passes . We are left with one customer test, which is saying Stack Empty attempting a Restore(). I got a little too rambunctious when I edited the tracing out of the old Restore() method. It needs to check the stack, and I deleted that when I deleted the Console.WriteLine() calls:

 public void Restore() { 
 if (SavedModels.Count == 0) return; 
 IUndoRestore opt = (IUndoRestore) SavedModels.Pop(); 
 opt.Restore(this); 
 } 

All the tests are running again, using the original Snapshot() that just saves a new copy of the TextModel. Now there are a number of methods that are no longer needed, and I ll remove them to clean up the space further. Removed are ReplaceChangedLines(), RemoveNewLines(), PutBackOldLines(), LinesBeforeCursor, BeforeLines, and some tests that use those: the entire UndoAnalyzer.cs and UndoAnalyzerTest.cs.

Again, I discover to my dismay that Microsoft Visual Studio and the compiler won t tell me which public methods are unreferenced. It will be difficult to be sure that I have found all the excess. I find: LineNumberToInsert, NumberOfLinesToRemove, FirstLineToRemove, NewModelLineContainingCursor and its corresponding member variable, newModelLineContainingCursor. I also remove the Boolean debugIsOptimized and its property. There s LinesToPutBack and NumberOfLinesToPutBack. The tests are all running. We have a good place to begin now.