Inspired by an example from Jonathan Pierce, I typed in a walkthrough of the .NET CollectionBase, a base class used for building typed collections. Chet and I talked about it at Borders today, and it inspired us to look at the ArticleList in the blog software. We tweaked it to make it more like a typed collection, and it's definitely better.

Ideas Ideas Everywhere and Not a Drop to Drink

There’s an interesting if somewhat acerbic conversation going on on the refactoring list, and it inspired me to look at .NET’s CollectionBase. Jonathan Pierce built a typed collection as part of an example, and it looked interesting. It has been my usual practice to build collections which are in practice typed, but not to make them strictly typed, since I don’t really want or need all the help that a compiler like C#.NET insists on giving me. But in the interest of learning something, I looked up the .NET CollectionBase walkthrough and created a simple NUnit Test-Driven typed collection.

When Chet and I got together at the Brighton Borders this morning, I told him a bit about the discussion, which is bordering on the manic, and showed him the CollectionBase example. We have been whining about the blog’s ArticleList object, so we decided to take a look at it. We didn’t convert it to use CollectionBase, but we did improve it quite a bit, and rather simply.

The Old ArticleList

There are two operations in the blog that require us to look at more than one Article: the table of contents on the front page, and creation of the RSS feed. To enable this, we created a simple class, ArticleList, which embeds a list of FileInfo objects pointing to Article files, and with a few methods that produce the data we want. It looked like this:

using System.Collections;
using System.IO;

namespace Blog {
  public class ArticleList {
    private ArrayList list;

    public ArticleList(ArrayList list) {
      this.list = list;
      this.list.Sort(new FileInfoDateComparer());
    }

    public ArticleList(object[] array) : this(new ArrayList(array)) {
    }

    public string Contents() {
      StringWriter s = new StringWriter();
      foreach (FileInfo f in list) {
        // TODO do we hate the FileInfo here?
        s.WriteLine("{0} ''{1}''", f.LastWriteTime.ToShortDateString(), f.Name);
        s.WriteLine(Articles.GetArticle(f).Precis());
      }
      return s.ToString();
    }

    public void WriteRssItems(TextWriter s) {
      foreach (FileInfo f in list) {
        s.WriteLine("<item>");
        s.WriteLine("<title>{0}</title>", WikiNameTagger.SpacedWikiWord(f.Name));
        s.WriteLine(
          "<link>http://www.XProgramming.com/Blog/Page.aspx?display={0}</link>", 
          f.Name);
        s.WriteLine("<description><![CDATA[{0}]]></description>", Description(f));
        s.WriteLine("<pubDate>{0}</pubDate>", f.LastWriteTime.ToString("r"));
        s.WriteLine("</item>");
      }
    }

    private string Description(FileInfo f) {
      // TODO uses FileInfo??
      return Articles.GetArticle(f).Description();
    }

    public string RssItems() {
      StringWriter s = new StringWriter();
      WriteRssItems(s);
      return s.ToString();
    }

    public string RssFeed() {
      StringWriter feed = new StringWriter();
      WriteRss(feed);
      return feed.ToString();
    }

    public void WriteRss( TextWriter feed ) {
      feed.WriteLine("<?xml version=\"1.0\"?>");
      feed.WriteLine("<rss version=\"2.0\">");
      feed.WriteLine("<channel>");
      feed.WriteLine("<title>Hot Needle Of Inquiry</title>");
      feed.WriteLine("<link>http://www.XProgramming.com/Blog/Page.aspx</link>");
      feed.WriteLine("<description>Ron Jeffries' blog</description>");
      WriteRssItems(feed);
      feed.WriteLine("</channel>");
      feed.WriteLine("</rss>");
    }
  }
}

There’s nothing very fancy here, nothing to be particularly proud of. I like that state of mind, and recommend it. If we’re not too proud of the code, it’s easier to see ways to change it, easier to listen to other people’s ideas about it. The ArticleList is used in just one place in Article, and in some tests. In Article, it looks like this:

    private string TableOfContentsText() {
      return ContentsList().Contents();
    }

    private ArticleList ContentsList() {
      DirectoryInfo info = new DirectoryInfo(articleFiler.DirectoryName);
      return new ArticleList(info.GetFiles("*."));
    }

We see that ArticleList is basically just a list of FileInfo objects. Our first thought was that the FileInfo thing has been bothering us for quite a while now. FileInfo is a pretty dull object, and using it so directly isn’t very object-oriented by our standards. We really thought that the ArticleList should be a list of, well, Articles.

Improving the ArticleList

Begin, of course, with a test. Specifically, the following test. Don’t try to read it all, just note the highlighted bits:

using System.Collections;
using System.IO;
using NUnit.Framework;

namespace Blog {
  [TestFixture] public class TestRSS : Assertion {

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

    [Test] public void Date() {
      FileInfo f = new FileInfo("../TestPages/MyBusinessIsToCreate");
      Assert(f.Exists);
      AssertEquals("9/7/2004", f.LastWriteTime.ToShortDateString());
    }

    [Test] public void FormatEmptyList() {
      ArticleList list = new ArticleList(EmptyList());
      string contents = list.Contents();
      string expected = "";
      AssertEquals(expected, contents);
    }

    [Test ] public void FormatList() {
      ArticleList list = new ArticleList(SampleArticles());
      string contents = list.Contents();
      string expected = "9/7/2004 ''MyBusinessIsToCreate''\r\n\r\n9/2/2004 ''BlogStories''\r\n\r\n";
      AssertEquals(expected, contents);
    }

    [Test]  public void RssItems() {
      ArticleList list = new ArticleList(SampleArticles());
      string[] rssItems = TestHtmlCalculator.SplitLines(list.RssItems());
      int line = 0;
      AssertEquals("<item>", rssItems[line++]);
      AssertEquals("<title>My Business Is To Create</title>", rssItems[line++]);
      AssertEquals(
        "<link>http://www.XProgramming.com/Blog/Page.aspx?display=MyBusinessIsToCreate</link>",
        rssItems[line++]);
      AssertEquals(
        "<description><![CDATA[<B>My Business Is To Create</B>",
        rssItems[line++]);
      line += 7;
      AssertEquals("]]></description>", rssItems[line++]);
      Assert(rssItems[line++].StartsWith("<pubDate>"));
      AssertEquals("</item>", rssItems[line++]);
      AssertEquals("<item>", rssItems[line++]);
      AssertEquals("<title>Blog Stories</title>", rssItems[line++]);
      AssertEquals(
        "<link>http://www.XProgramming.com/Blog/Page.aspx?display=BlogStories</link>",
        rssItems[line++]);
      AssertEquals(
        "<description><![CDATA[<B>Blog Stories</B>",
        rssItems[line++]);
      line += 13;
      AssertEquals("]]></description>", rssItems[line++]);
      Assert(rssItems[line++].StartsWith("<pubDate>"));
      AssertEquals("</item>", rssItems[line++]);
    }

    [Test] public void RssFeed() {
      ArticleList list = new ArticleList(SampleArticles());
      string[] rssItems = TestHtmlCalculator.SplitLines(list.RssFeed());
      int line = 0;
      AssertEquals("<?xml version=\"1.0\"?>", rssItems[line++]);
      AssertEquals("<rss version=\"2.0\">", rssItems[line++]);
      AssertEquals("<channel>", rssItems[line++]);
      AssertEquals("<title>Hot Needle Of Inquiry</title>", rssItems[line++]);
      AssertEquals(
        "<link>http://www.XProgramming.com/Blog/Page.aspx</link>", 
        rssItems[line++]);
      AssertEquals("<description>Ron Jeffries' blog</description>", rssItems[line++]);
      line = rssItems.Length-3;
      AssertEquals("</channel>", rssItems[line++]);
      AssertEquals("</rss>", rssItems[line++]);
      AssertEquals("", rssItems[line++]);
    }

    private ArrayList EmptyList() {
      return new ArrayList();
    }

    private ArrayList SampleArticles() {
      ArrayList result = new ArrayList();
      result.Add(new FileInfo("../TestPages/MyBusinessIsToCreate"));
      result.Add(new FileInfo("../TestPages/BlogStories"));
      return result;
    }
  }
}

So the tests here create only one interesting ArticleList, the one with the two entries in the TestPages directory. These are used to check the Contents, and to check the RSS results, using a bunch of straightforward string compares which were tedious to write, and even more tedious to read. Chet and I just made one change:

    private ArrayList SampleArticles() {
      ArrayList result = new ArrayList();
      result.Add(Articles.GetArticle("MyBusinessIsToCreate", "../TestPages"));
      result.Add(Articles.GetArticle("BlogStories", "../TestPages/"));
      return result;
    }

ArticleList’s constructor takes an ArrayList or Array, formerly of FileInfo. We change the input ArrayList in the test to be a list of Article, using our handy-dandy Article factory class, Articles. Then we ran the tests. They broke, no surprise:

    public ArticleList(ArrayList list) {
      this.list = list;
      this.list.Sort(new FileInfoDateComparer());
    }

This code blew up because FileInfoDateComparer didn’t like Articles. It looked like this:

using System;
using System.Collections;
using System.IO;

namespace Blog {
  public class FileInfoDateComparer : IComparer {
    public FileInfoDateComparer() {
    }

    public int Compare(Object d1, Object d2) {
      return DateTime.Compare( 
        ((FileInfo) d2).LastWriteTime, 
        ((FileInfo) d1).LastWriteTime);
    }
  }
}

The cast to FileInfo was failing, because Article can’t be cast to FileInfo. So we modified the Sort call and the comparer:

    public ArticleList(ArrayList list) {
      this.list = list;
      this.list.Sort(new ArticleDateComparer());
    }

  public class ArticleDateComparer : IComparer {
    public ArticleDateComparer() {
    }

    public int Compare(Object d1, Object d2) {
      return DateTime.Compare( 
        ((Article) d2).LastWriteTime, 
        ((Article) d1).LastWriteTime);
    }
  }

Sweet. Now it explodes because Article doesn’t understand LastWriteTime. Easily fixed:

... in Article:
    public DateTime LastWriteTime {
      get { return articleFiler.LastWriteTime ; }
    }

... in ArticleFiler:
    public DateTime LastWriteTime {
      get { return File.GetLastWriteTime(FileNameToRead); }
    }

We proceed like this just a bit more, changing references to FileInfo to article, and wind up with a new ArticleList looking like this:

  public class ArticleList {
    private ArrayList list;

    public ArticleList(ArrayList list) {
      this.list = list;
      this.list.Sort(new ArticleDateComparer());
    }

    public ArticleList(object[] array) : this(new ArrayList(array)) {
    }

    public string Contents() {
      StringWriter s = new StringWriter();
      foreach (Article article in list) {
        s.WriteLine("{0} ''{1}''", article.LastWriteTime.ToShortDateString(), article.Name);
        s.WriteLine(article.Precis());
      }
      return s.ToString();
    }

    public void WriteRssItems(TextWriter s) {
      foreach (Article article in list) {
        s.WriteLine("<item>");
        s.WriteLine("<title>{0}</title>", WikiNameTagger.SpacedWikiWord(article.Name));
        s.WriteLine(
          "<link>http://www.XProgramming.com/Blog/Page.aspx?display={0}</link>", 
          article.Name);
        s.WriteLine("<description><![CDATA[{0}]]></description>", Description(article));
        s.WriteLine("<pubDate>{0}</pubDate>", article.LastWriteTime.ToString("r"));
        s.WriteLine("</item>");
      }
    }

    private string Description(Article article) {
      return article.Description();
    }

    public string RssItems() {
      StringWriter s = new StringWriter();
      WriteRssItems(s);
      return s.ToString();
    }

    public string RssFeed() {
      StringWriter feed = new StringWriter();
      WriteRss(feed);
      return feed.ToString();
    }

    public void WriteRss( TextWriter feed ) {
      feed.WriteLine("<?xml version=\"1.0\"?>");
      feed.WriteLine("<rss version=\"2.0\">");
      feed.WriteLine("<channel>");
      feed.WriteLine("<title>Hot Needle Of Inquiry</title>");
      feed.WriteLine("<link>http://www.XProgramming.com/Blog/Page.aspx</link>");
      feed.WriteLine("<description>Ron Jeffries' blog</description>");
      WriteRssItems(feed);
      feed.WriteLine("</channel>");
      feed.WriteLine("</rss>");
    }
  }

So far so good. At this point all the tests are running. But in writing this article, I noticed a problem. My current refactoring tool, JetBrains ReSharper, has this neat “Find Usages” tool, and when I used it just now on ArticleList, I found this in the Article class:

    private ArticleList ContentsList() {
      DirectoryInfo info = new DirectoryInfo(articleFiler.DirectoryName);
      return new ArticleList(info.GetFiles("*."));
    }

Hello, what’s this about? This usage of the ArticleList is still passing in FileInfo instances. That’s not going to work … or is it? It might, because we basically implemented all the necessary methods on Article. But I expect that the casts to Article will fail. I’ll have to run the blog to find out. … and right, the casts fail.

This is easy enough to fix, but it does mean that my tests aren’t strong enough. I do have the one that launches the actual blog turned off, and it would find the problem, but it’s really slow and I’m not likely to run with it turned on. So now the moral dilemma. I can fix the defect easily enough, any number of ways. One easy one would be to add a constructor to ArticleList that accepts the FileInfo instances. There is probably a cleaner way.

But I need a test! Or do I … if I converted the ArticleList to a typed collection, then this code wouldn’t compile, and I’d be forced to fix it. No, it’s still a sin. My rules require me to write a test for this defect. How will I do it? Tune in next week for the thrilling conclusion …

Reflection

Still, so far, it has gone pretty well. We changed a test, ran it until it broke three or four times, each time progressing through the code until the tests all ran. Very simple, very calm, no muss no fuss. Except for this nagging test thing, it was a good day. Thanks to everyone involved.