Inch by inch, step by step, we work on improving the internals of the ArticleFiler. No master plan, no grand design. Just gently polishing until the object is visibly better. A good way to proceed? You decide.

Ideas for Further Simplification

We made a little progress last time, though not as much as I’d hoped, and not in quite the direction I had intended. It often goes that way: I don’t wind up where I thought I would. In some cases, like last time, I don’t even start out like I thought I would. I let the work guide me. (See the last story here.)

The ArticleFiler, shown just below, includes a number of uses of the FileInfo class. It turns out that there are some simpler ways to do some of those things, in the Path class and the File class. Let’s see whether we can make ArticleFiler a bit simpler. Here’s the current code. Just skim it for now, and refer back as we change things. That’s what I’m going to do.

  public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string directoryName;
    private string articleName;

    public ArticleFiler(string path, string articleName) {
      this.directoryName = path;
      this.articleName = articleName;
    }

    public string DirectoryName {
      get { return directoryName; }
    }

    public string ArticleName {
      get { return articleName; }
    }

    public string CommentText() {
      if (! CommentFile.Exists )
        return "----/r/nComment Here";
      using (StreamReader reader = CommentFile.OpenText())
        return reader.ReadToEnd();
    }

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = FileToRead.OpenText()) 
        return reader.ReadToEnd();
    }

    public void WriteText(string text) {
      using(StreamWriter writer = this.CreateText())
        writer.Write(text);
    }

    public void WriteComment(string text) {
      using(StreamWriter writer = this.CreateCommentFile())
        writer.Write(text);
    }

    public bool Exists {
      get { return BaseFile.Exists || InvisibleFile.Exists || CommentFile.Exists; }
    }

    // TODO not entirely happy about this automatic file finding stuff

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( InvisibleFile.Exists ) 
          return InvisibleFile;
        else if ( BaseFile.Exists )
          return BaseFile;
        else if ( CommentFile.Exists )
          return CommentFile;
        else // none exist
          return BaseFile;
      }
    }

    internal FileInfo FileToWrite { // testing and local only
      get { return FileToRead; }
    }

    private StreamWriter CreateCommentFile() {
      return CommentFile.CreateText();
    }

    private StreamWriter CreateText() {
      return FileToWrite.CreateText();
    }

    private FileInfo BaseFile {
      get { return new FileInfo(FileName(directoryName, articleName, "")); }
    }

    private FileInfo InvisibleFile {
      get { return new FileInfo(FileName(directoryName, articleName, invisibleExtension)); }
    }

    private FileInfo CommentFile {
      get { return new FileInfo(FileName(directoryName, articleName, commentExtension)); }
    }

    private string FileName( string path, string name, string extension ) {
      return path + "/" + name + extension; 
    }
  }

The Path Class

The Path class in .NET has a number of handy static methods. The ones that appeal to me are Combine(), which puts two parts of a file path together, and GetExtension(), HasExtension() and ChangeExtension(). Let’s see what we can do …

Begin with a test. We have some tests on ArticleFiler:

  public class TestArticleFiler : Assertion{

    private ArticleFiler Info(string directory, string file) {
      return new ArticleFiler(directory, file);
    }

    [Test] public void Hookup() {
      Assert(true);
    }

    [Test] public void DoesntExist() {
      Assert(!Info("../Pages", "MyBusinessIsToConfuse").Exists);
    }

    [Test] public void Exists() {
      Assert(Info("../Pages", "MyBusinessIsToCreate").Exists);
    }

    [Test] public void InvisibleExists() {
      Assert(Info("../Pages", "PasswordRequired").Exists);
    }

    [Test] public void WikiExists() {
      Assert(Info("../Pages", "WikiTest").Exists);
    }

    [Test] public void FileToRead() {
      AssertEquals
        ("PasswordRequired.invisible", 
        Info("../Pages", "PasswordRequired").FileToRead.Name);
    }

    [Test] public void WikiFileToRead() {
      AssertEquals
        ("WikiTest.wiki",
        Info("../Pages", "WikiTest").FileToRead.Name);
    }

    [Test] public void FileToWrite() {
      AssertEquals
        ("PasswordRequired.invisible", 
        Info("../Pages", "PasswordRequired").FileToWrite.Name);
    }
  }

This class was called TestWikiFileInfo until just a moment ago. I should have renamed it before we looked at it last night but I didn’t notice the anomaly. Its name is left over from an earlier name for the ArticleFiler. Anyway, it’s all better now. These tests look pretty solid, at least enough to allow some refactoring. But we’ll look at them a bit later to see if we would like to change them around a bit. One thing I’d like to do is move them to focus on a fixed sample directory instead of the Pages directory, which can change as we run the Blog on my local computer. But first, let’s see what we can do to simplify some of the code in ArticleFiler itself. Our first target is this:

    private string FileName( string path, string name, string extension ) {
      return path + "/" + name + extension; 
    }

We’re doing this string manipulation ourselves, and Path can do it for us in a more robust fashion. Let’s try it:

    private string FileName( string path, string name, string extension ) {
      string newPath = Path.Combine(path, name);
      return Path.ChangeExtension(newPath, extension);
    }

Is that an improvement? Not clear, it now takes two lines rather than one. We could have done it by appending name and extension outside the Path, like this:

    private string FileName( string path, string name, string extension ) {
      return Path.Combine(path, name+extension);
    }

That’s shorter and I think I’ll leave it that way. I have an inkling that we’re going to fiddle with the extensions a bit real soon now anyway. The tests ran both ways, by the way, which tells me that passing an extension with a dot included into ChangeExtension works in the way that we’d hope.

Now let’s look at our three uses of FileName, now that we have Path figured out and under control:

    private FileInfo BaseFile {
      get { return new FileInfo(FileName(directoryName, articleName, "")); }
    }

    private FileInfo InvisibleFile {
      get { return new FileInfo(FileName(directoryName, articleName, invisibleExtension)); }
    }

    private FileInfo CommentFile {
      get { return new FileInfo(FileName(directoryName, articleName, commentExtension)); }
    }

Each of these guys is really just setting the extension of a standard file name. That’s interesting. Why not just save a single file and path name, and create the ones with different extensions as we use them. Like this:

  public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string directoryName;
    private string articleName;
    private string basicPath;

    public ArticleFiler(string path, string articleName) {
      this.directoryName = path;
      this.articleName = articleName;
      basicPath = Path.Combine(path, articleName);
    }

		...

    private FileInfo BaseFile {
      get { return new FileInfo(Path.ChangeExtension(basicPath,"")); }
    }

    private FileInfo InvisibleFile {
      get { return new FileInfo(Path.ChangeExtension(basicPath, invisibleExtension)); }
    }

    private FileInfo CommentFile {
      get { return new FileInfo(Path.ChangeExtension(basicPath, commentExtension)); }
    }
  }

Those are a bit simpler, and now, if I’m not mistaken, the FileName method is no longer used. My current refactoring tool, JetBrains ReSharper, agrees with me … and so do the tests.

Could we have done that in one step, removing the FileName, and then putting the Path code directly into the three methods above? Surely we could have. It’s my practice to go in these tiny steps, and at least in a tiny program, it takes no more time and prevents me from going down ratholes. It would have been possible, though, to have changed the three methods above one at a time, if we were smart enough to see where we were going. The way I did it, I just improved something, which drew my attention to another thing. We’ll keep going like that. Now my attention is drawn to the fact that those three methods are returning a FileInfo, which is a fairly heavy object. Why is that happening?

Distracted again. In looking at the FileInfo stuff, I noticed that the article name and path are stored as member variables, and returned by two properties. Let’s drop the storage, and change the properties. It’ll just take a moment:

  public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string basicPath;

    public ArticleFiler(string path, string articleName) {
      basicPath = Path.Combine(path, articleName);
    }

    public string DirectoryName {
      get { return Path.GetPathRoot(basicPath); }
    }

    public string ArticleName {
      get { return Path.GetFileNameWithoutExtension(basicPath); }
    }
    ...

We removed the member variables and their init, and changed DirectoryName and ArticleName properties to use the basicPath member.

A point of technique here, more to drill it into my brain than into yours. ReSharper has a nice feature, “Find Usages”. It opens a new window that points to all the usages of whatever you point at. You can click on each usage, and Visual Studio opens the source file containing it, and positions the cursor on it. You can edit that usage, then carry on. It woks a bit like the Smalltalk “Senders” browser, and it’s a very convenient way to cycle through all the usages of something, edit them, and then be in a position to remove whatever it was you were pointing at. In this case, it was those two member variables that are now all gone. Nice feature, nearly good. Now ArticleFiler is a bit simpler, yet again. Take a look:

  public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string basicPath;

    public ArticleFiler(string path, string articleName) {
      basicPath = Path.Combine(path, articleName);
    }

    public string DirectoryName {
      get { return Path.GetPathRoot(basicPath); }
    }

    public string ArticleName {
      get { return Path.GetFileNameWithoutExtension(basicPath); }
    }

    public string CommentText() {
      if (! CommentFile.Exists )
        return "----/r/nComment Here";
      using (StreamReader reader = CommentFile.OpenText())
        return reader.ReadToEnd();
    }

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = FileToRead.OpenText()) 
        return reader.ReadToEnd();
    }

    public void WriteText(string text) {
      using(StreamWriter writer = this.CreateText())
        writer.Write(text);
    }

    public void WriteComment(string text) {
      using(StreamWriter writer = this.CreateCommentFile())
        writer.Write(text);
    }

    public bool Exists {
      get { return BaseFile.Exists || InvisibleFile.Exists || CommentFile.Exists; }
    }

    // TODO not entirely happy about this automatic file finding stuff

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( InvisibleFile.Exists ) 
          return InvisibleFile;
        else if ( BaseFile.Exists )
          return BaseFile;
        else if ( CommentFile.Exists )
          return CommentFile;
        else // none exist
          return BaseFile;
      }
    }

    internal FileInfo FileToWrite { // testing and local only
      get { return FileToRead; }
    }

    private StreamWriter CreateCommentFile() {
      return CommentFile.CreateText();
    }

    private StreamWriter CreateText() {
      return FileToWrite.CreateText();
    }

    private FileInfo BaseFile {
      get { return new FileInfo(Path.ChangeExtension(basicPath,"")); }
    }

    private FileInfo InvisibleFile {
      get { return new FileInfo(Path.ChangeExtension(basicPath, invisibleExtension)); }
    }

    private FileInfo CommentFile {
      get { return new FileInfo(Path.ChangeExtension(basicPath, commentExtension)); }
    }
  }

Now there are those three FileInfo-returning methods right in our face. Let’s examine how they are used. Well, we see that they are used in the method Exists, in FileToRead, and in the various methods like CreateText() and CreateCommentFile(). I rather like the code that checks for existence: it really likes to be talking to a FileInfo for clarity. But let’s change one of these last three methods to return a string instead and see what we have to fix and how ugly it becomes. It might be worth it. Using Find Usages again, I discover that InvisibleFile has only three usages. BaseFile has four, and CommentFile has six! I’ll change InvisibleFile first.

I can see two ways to go. One is just to change the return from Invisible file directly to string, and repair the three usages. A more conservative approach might be to create a new method that returns string, say InvisibleFileName, then change the usages to refer to it rather than the other. When I started this paragraph, I was going to do it the first way but I think it’ll be safer and a bit more educational to do the second. In addition, I like the name InvisibleFileName a bit better than InvisibleFile. So first we extract the new method:

    private FileInfo InvisibleFile {
      get { return new FileInfo(InvisibleFileName); }
    }

    private string InvisibleFileName {
      get { return Path.ChangeExtension(basicPath, invisibleExtension); }
    }

Resharper did the Extract Method for me. Now I’ll change the usages, one at a time:

    public bool Exists {
      get { return 
              (BaseFile.Exists || 
              File.Exists(InvisibleFileName) || 
              CommentFile.Exists); }
    }

Now that’s not lovely, but it’s easy. I’m tentatively accepting it to see how things turn out. Next:

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return InvisibleFile;
        else if ( BaseFile.Exists )
          return BaseFile;
        else if ( CommentFile.Exists )
          return CommentFile;
        else // none exist
          return BaseFile;
      }
    }

That was cut and dried. I notice a bit of duplication there. Let’s make a note of it with a TODO and move on.

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return new FileInfo(InvisibleFileName);
        else if ( BaseFile.Exists )
          return BaseFile;
        else if ( CommentFile.Exists )
          return CommentFile;
        else // none exist
          return BaseFile;
      }

Easy enough. I’m not sure I love it but it does allow me to remove the InvisibleFile method. Let’s do that (the tests all run), then push on to BaseFile. I could get brave and do it by changing the BaseFile method to BaseFileName, or do it in two steps. This time I’ll do it the other way and tell you why. As soon as I change BaseFile to BaseFileName:

    private string BaseFileName {
      get { return Path.ChangeExtension(basicPath,""); }
    }

ReSharper highlights the lines in the file that no longer work because they refer to BaseFile, which no longer exists. It does this with little orange lines in the scroll bar, and when I click on them, it takes me to the broken lines. There might even be another way to do it. Hold on a moment. Sure enough, there is. There’s “Go to Next Error” on the menu, or you can type F2. Cool, this will click me right through the changes:

    public bool Exists {
      get { return 
              (File.Exists(BaseFileName)     || 
              File.Exists(InvisibleFileName) || // TODO duplicated elsewhere?
              CommentFile.Exists); }
    }

… and …

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return new FileInfo(InvisibleFileName);
        else if ( File.Exists(BaseFileName) )
          return new FileInfo(BaseFileName);
        else if ( CommentFile.Exists )
          return CommentFile;
        else // none exist
          return new FileInfo(BaseFileName);
      }
    }

That was easy enough. We see a little more duplication, though. But let’s wait and see what happens with the final change. I’ll do it the same way, replacing CommentFile with CommentFileName:

    private string CommentFileName {
      get { return Path.ChangeExtension(basicPath, commentExtension); }
    }

… and going through the F2 process again, resulting in:

    public string CommentText() {
      if (! File.Exists(CommentFileName) )
        return "----/r/nComment Here";
      using (StreamReader reader = File.OpenText(CommentFileName))
        return reader.ReadToEnd();
    }

    public bool Exists {
      get { return 
              File.Exists(BaseFileName)     || 
              File.Exists(InvisibleFileName) || // TODO duplicated elsewhere?
              File.Exists(CommentFileName); }
    }

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return new FileInfo(InvisibleFileName);
        else if ( File.Exists(BaseFileName) )
          return new FileInfo(BaseFileName);
        else if ( File.Exists(CommentFileName) )
          return new FileInfo(CommentFileName);
        else // none exist
          return new FileInfo(BaseFileName);
      }
    }

    private StreamWriter CreateCommentFile() {
      return File.CreateText(CommentFileName);
    }

The tests all still run. Take a look at the whole class:

  public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string basicPath;

    public ArticleFiler(string path, string articleName) {
      basicPath = Path.Combine(path, articleName);
    }

    public string DirectoryName {
      get { return Path.GetPathRoot(basicPath); }
    }

    public string ArticleName {
      get { return Path.GetFileNameWithoutExtension(basicPath); }
    }

    public string CommentText() {
      if (! File.Exists(CommentFileName) )
        return "----/r/nComment Here";
      using (StreamReader reader = File.OpenText(CommentFileName))
        return reader.ReadToEnd();
    }

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = FileToRead.OpenText()) 
        return reader.ReadToEnd();
    }

    public void WriteText(string text) {
      using(StreamWriter writer = this.CreateText())
        writer.Write(text);
    }

    public void WriteComment(string text) {
      using(StreamWriter writer = this.CreateCommentFile())
        writer.Write(text);
    }

    public bool Exists {
      get { return 
              File.Exists(BaseFileName)     || 
              File.Exists(InvisibleFileName) || // TODO duplicated elsewhere?
              File.Exists(CommentFileName); }
    }

    // TODO not entirely happy about this automatic file finding stuff

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return new FileInfo(InvisibleFileName);
        else if ( File.Exists(BaseFileName) )
          return new FileInfo(BaseFileName);
        else if ( File.Exists(CommentFileName) )
          return new FileInfo(CommentFileName);
        else // none exist
          return new FileInfo(BaseFileName);
      }
    }

    internal FileInfo FileToWrite { // testing and local only
      get { return FileToRead; }
    }

    private StreamWriter CreateCommentFile() {
      return File.CreateText(CommentFileName);
    }

    private StreamWriter CreateText() {
      return FileToWrite.CreateText();
    }

    private string BaseFileName {
      get { return Path.ChangeExtension(basicPath,""); }
    }

    private string InvisibleFileName {
      get { return Path.ChangeExtension(basicPath, invisibleExtension); }
    }

    private string CommentFileName {
      get { return Path.ChangeExtension(basicPath, commentExtension); }
    }

Again, it’s a bit simpler. But there’s some really irritating duplication going around. Here’s the main thing that catches my eye:

    internal FileInfo FileToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return new FileInfo(InvisibleFileName);
        else if ( File.Exists(BaseFileName) )
          return new FileInfo(BaseFileName);
        else if ( File.Exists(CommentFileName) )
          return new FileInfo(CommentFileName);
        else // none exist
          return new FileInfo(BaseFileName);
      }
    }

We can make it a bit simpler by computing FileNameToRead and then just returning its FileInfo:

    private string FileNameToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return InvisibleFileName;
        else if ( File.Exists(BaseFileName) )
          return BaseFileName;
        else if ( File.Exists(CommentFileName) )
          return CommentFileName;
        else // none exist
          return BaseFileName;
      }
    }

    internal FileInfo FileToRead {
      get { return new FileInfo(FileNameToRead); }
    }

That’s somewhat better. I wonder if there’s a good way to get rid of the duplicate occurrences of, for example, InvisibleFileName in FileNameToRead. Mind you, I’m not worried about performance, just about the fact that the word appears twice. I don’t see a way, do you?

Now what about those FileInfo things like FileToRead. We could just return the name, depending on who’s using them. This is a job for … Find Usages! Well, one of them is this:

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = FileToRead.OpenText()) 
        return reader.ReadToEnd();
    }

That’s readily changed, in the style we used elsewhere:

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = File.OpenText(FileNameToRead)) 
        return reader.ReadToEnd();
    }

We find a test:

    [Test] public void FileToRead() {
      AssertEquals
        ("PasswordRequired.invisible", 
        Info("../Pages", "PasswordRequired").FileToRead.Name);
    }

… where …

    private ArticleFiler Info(string directory, string file) {
      return new ArticleFiler(directory, file);
    }

That helper method needs to be renamed to Filer, while we’re at it. Consider it done, because it is. Now to fix the test. I tried this:

    [Test] public void FileToRead() {
      AssertEquals
        ("PasswordRequired.invisible", 
         Filer("../Pages", "PasswordRequired").FileNameToRead);
    }

But that returns the whole path, not just “PasswordRequired”. For now, we’ll use Path to extract what we want:

    [Test] public void FileToRead() {
      AssertEquals
        ("PasswordRequired.invisible", 
         Path.GetFileName(Filer("../Pages", "PasswordRequired").FileNameToRead));
    }

Now that’s not pretty, but when we’re done, it’ll move the ugliness to the test and we can probably improve it even there. Or we can put a test-helper method on the class if we need it. Let’s press on. There’s another test needing the same treatment:

    [Test] public void WikiFileToRead() {
      AssertEquals
        ("WikiTest.wiki",
         Filer("../Pages", "WikiTest").FileToRead.Name);
    }

And we’ll give it to him:

    [Test] public void WikiFileToRead() {
      AssertEquals
        ("WikiTest.wiki",
         Path.GetFileName(Filer("../Pages", "WikiTest").FileNameToRead));
    }

That works. Now we have:

    internal FileInfo FileToWrite { // testing and local only
      get { return FileToRead; }
    }

Which we’ll fix, for now, like this:

    internal FileInfo FileToWrite { // testing and local only
      get { return new FileInfo(FileNameToRead); }
    }

This means that there are now no accesses to FileToRead, so we can delete the method. And the tests still run. We can replace FileToWrite with a string FileNameToWrite written this way:

    internal string FileNameToWrite {
      get { return FileNameToRead; } 
    }

And find and change the usages:

    private StreamWriter CreateText() {
      return File.CreateText(FileNameToWrite);
    }

    [Test] public void FileToWrite() {
      AssertEquals
        ("PasswordRequired.invisible", 
         Path.GetFileName(Filer("../Pages", "PasswordRequired").FileNameToWrite));
    }

… just as before. We can delete FileToWrite now, and the tests run. Let’s look at ArticleFiler in toto (the little dog).

  public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string basicPath;

    public ArticleFiler(string path, string articleName) {
      basicPath = Path.Combine(path, articleName);
    }

    public string DirectoryName {
      get { return Path.GetPathRoot(basicPath); }
    }

    public string ArticleName {
      get { return Path.GetFileNameWithoutExtension(basicPath); }
    }

    public string CommentText() {
      if (! File.Exists(CommentFileName) )
        return "----/r/nComment Here";
      using (StreamReader reader = File.OpenText(CommentFileName))
        return reader.ReadToEnd();
    }

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = File.OpenText(FileNameToRead)) 
        return reader.ReadToEnd();
    }

    public void WriteText(string text) {
      using(StreamWriter writer = this.CreateText())
        writer.Write(text);
    }

    public void WriteComment(string text) {
      using(StreamWriter writer = this.CreateCommentFile())
        writer.Write(text);
    }

    public bool Exists {
      get { return 
              File.Exists(BaseFileName)      || 
              File.Exists(InvisibleFileName) || // TODO duplicated elsewhere?
              File.Exists(CommentFileName); }
    }

    // TODO not entirely happy about this automatic file finding stuff

    internal string FileNameToRead { // testing and local only
      get {
        if ( File.Exists(InvisibleFileName) ) 
          return InvisibleFileName;
        else if ( File.Exists(BaseFileName) )
          return BaseFileName;
        else if ( File.Exists(CommentFileName) )
          return CommentFileName;
        else // none exist
          return BaseFileName;
      }
    }

    internal string FileNameToWrite {
      get { return FileNameToRead; } 
    }

    private StreamWriter CreateCommentFile() {
      return File.CreateText(CommentFileName);
    }

    private StreamWriter CreateText() {
      return File.CreateText(FileNameToWrite);
    }

    private string BaseFileName {
      get { return Path.ChangeExtension(basicPath,""); }
    }

    private string InvisibleFileName {
      get { return Path.ChangeExtension(basicPath, invisibleExtension); }
    }

    private string CommentFileName {
      get { return Path.ChangeExtension(basicPath, commentExtension); }
    }
  }

OK, what now? well, we can get rid of those last three private methods entirely by storing those strings as member variables. That would also reduce our tension about the duplication in Exists. Let’s do that:

 public class ArticleFiler {

    const string invisibleExtension = ".invisible";
    const string commentExtension = ".wiki";
    private string basicFileName;
    private string invisibleFileName;
    private string commentFileName;

    public ArticleFiler(string path, string articleName) {
      basicFileName = Path.Combine(path, articleName);
      invisibleFileName = Path.ChangeExtension(basicFileName, invisibleExtension);
      commentFileName = Path.ChangeExtension(basicFileName, commentExtension);
    }

    public string DirectoryName {
      get { return Path.GetPathRoot(basicFileName); }
    }

    public string ArticleName {
      get { return Path.GetFileNameWithoutExtension(basicFileName); }
    }

    public string CommentText() {
      if (! File.Exists(commentFileName) )
        return "----/r/nComment Here";
      using (StreamReader reader = File.OpenText(commentFileName))
        return reader.ReadToEnd();
    }

    public string ArticleText() {
      if ( ! Exists )
        return "Cannot find article text";
      using (StreamReader reader = File.OpenText(FileNameToRead)) 
        return reader.ReadToEnd();
    }

    public void WriteText(string text) {
      using(StreamWriter writer = this.CreateText())
        writer.Write(text);
    }

    public void WriteComment(string text) {
      using(StreamWriter writer = this.CreateCommentFile())
        writer.Write(text);
    }

    public bool Exists {
      get { return 
              File.Exists(basicFileName)      || 
              File.Exists(invisibleFileName) || // TODO duplicated elsewhere?
              File.Exists(commentFileName); }
    }

    // TODO not entirely happy about this automatic file finding stuff

    internal string FileNameToRead { // testing and local only
      get {
        if ( File.Exists(invisibleFileName) ) 
          return invisibleFileName;
        else if ( File.Exists(basicFileName) )
          return basicFileName;
        else if ( File.Exists(commentFileName) )
          return commentFileName;
        else // none exist
          return basicFileName;
      }
    }

    internal string FileNameToWrite {
      get { return FileNameToRead; } 
    }

    private StreamWriter CreateCommentFile() {
      return File.CreateText(commentFileName);
    }

    private StreamWriter CreateText() {
      return File.CreateText(FileNameToWrite);
    }
  }

Now then. Take a look back at the original and at this version. Much shorter and, I think, rather more clear. There’s a bit more we could do, and now some of the tests are a bit trickier, but I think it’s a good time to take a break.

Observations

So. What happened here? It should be pretty clear, if nothing else, that whatever happened happened without a grand plan. We just kept looking at things that we didn’t quite like, and moving them in directions we liked better. The net result is a reduction of the class from 91 lines to 80, and from 204 words to 181, from 16 methods to 12. More interesting, we removed all the occurrences of the FileInfo class, some four occurrences. That makes me happy as well.

The work was very rhythmic and there wasn’t more than a moment when anything was broken. Did it take a little longer than I might like? Perhaps, but on the other hand, I was writing this article as well as programming. Article plus programming only took two hours, and I took out time to have a root beer float with my wife Ricia. Not bad at all.

The lesson? You get to decide what it means to you. What it means to me is that there is this very simple, very stress-free way of improving the code, a tiny bit at a time, that keeps the program always working, that keeps the stress down, and that leads surely and not all that slowly to a better final situation. Could I have gotten to this state or a better one with more planning and thinking? In two hours, with time out for a float? Frankly I don’t think so. Will it work for you? Try it and let me know.