How much travel is too much travel?

I’m sitting here on the plane flying from Dallas to St. Louis, the second hop of my trip home from Redmond, and we’re a couple hundred miles out of St. Louis. I just made it to Executive Platinum on American Airlines in the past 100 miles or so. For those of you who don’t know what that is, that’s 100,000 miles flown in one year. Is that too much travel? Sometimes I feel like I’m just visiting when I’m home 🙁

There are pluses and minuses to traveling this much. I have 3 young children, and I miss a few of their special occasions, and I don’t get to see them anywhere as much as I’d like to. Linsey is 10, loves to play sports and talk on AIM to her friends. She plays piano and cello, sings in the choir, dances, plays softball, basketball, and tennis. And gets good grades. She is so sensitive and loving, I enjoy all my time with here. Laurel is 7, and is brilliant. She is what they call highly gifted, and she and I relate very well because of it. As a child, I went through a lot of the same stuff she is going through now in having a hard time relating to her peers on their level. It’s hard for children like that to have normal childhoods,  because their mind is at a different level than their classmates. She and I talk about those kinds of issues a lot, and we like to be silly together. And Andrew is 4, and is going to push Laurel in terms of academics really soon. He’s really bright, too, and loves to play trains. Whenever he sees me, he always asks me to play Thomas with him. I miss all of them when I’m not there.

And I haven’t even mentioned my wife, Sharon. She’s just great. She takes care of all the house management stuff, as well as manages our business, leaving me free to work, develop business, teach, speak, write, and do all those professional things I need to do to make our business a success. And she plays tennis and softball, is very involved in our kids’ schools, has been PTO president for the past 3 years, yet still finds time to spoil our kids silly. I have tremendous respect for the job she does without me for a large part of the time.

On the other hand, I have the best time when I’m working up at Microsoft. The two guys I work with on a daily basis, Scott Densmore and Peter Provost are both incredible. They teach me something new every day. Between them, they know more about windows programming that I ever will. And do we have fun! We have nicknames for each other, we quote movie lines all day long, all like the same music, and just have a ball. Oh, yeah, we also write some really cool software. And getting to work with Jim Newkirk and Ward Cunningham daily is like gravy on top of all of this. Jim and I have some really good talks about where software and TDD are going, and how we can help, and Ward is just Ward. Ward was telling me about this idea he has for a next-generation wiki, possibly written in a language he just showed me called Io. Very cool prototype-based language that took me about 5 minutes to learn 🙂 As a contractor, I know my time at Microsoft is going to come to an end at some point, and I know I’ll miss it and them very much when it does.

I started this post with the intent of talking about what I was doing on the plane, but I got a little off track. My original point that I wanted to make is that I’m spending my time on the plane practicing programming. I remember either Kent Beck or Ron Jeffries talking about how they practice programming. Just like star athletes and musicians practice their craft, programmers should practice theirs. I’ve been thinking and introspect+ing a lot in the past month or so about refactoring and TDD, as well as reading the Refactoring and TDD yahoo groups. From those lists and my own thinking, I’ve developed a few things I’d really like to try out. Jay Bazuzi had a really cool post on the refactoring list a couple of weeks ago about really small methods, and I’m trying to write my code like that, just for practice. Ron posted to the TDD list that he almost never uses exceptions. He changes his design in such a way that their handled at the point at which they’re thrown, so the only exceptions that leak out of his lower level classes are the ones he truly can’t do anything about. I know that sounds a bit vague, but I’m trying to put together some examples of what I think he means that I’m going to write about soon.

If anyone made it this far in this rambling post, I thank you for reading it. It really was just a stream-of-conciousness post about what was on my mind right now. Welcome to my life 🙂

— bab

 

TDD Dive — Chapter 6

In this installment, we’re going to code up the input side. This is going to be very much like the last installment, where we did the output. The key to making this easy is to do everything in terms of TextReaders instead of specific kinds of TextReaders. This will allow us to use StringReaders in our tests, and substitute in a StreamReader later, in main. Armed with this knowledge, we can put our test code and test data in the tests themselves, as opposed to having the data spread out in little files all over the place.

So, our end goal for this part of the story is to read in an input line, parse out the Payroll command on that line, create a Payroll object, and run it. Pretty simple. Let’s start with a test list for the input reader:

  1. If no payroll input, no date is returned.
  2. If single payroll input, only one date is returned
  3. if multiple payrolls, can get all of them one at a time

I know I’ve been ignoring all error checks throughout this example. That’s because its really not the point of the exercise. If I wanted to be completey robust here, I’d stick in some stuff about making sure the input is formatted correctly, etc, but I’m really not interested in that. If this were a real system, I absolutely would be, and I’d write extra tests for those case.

So, I can’t think of any more test cases for this off the top of my head, although I might come back to this later. As usual, I want to implement the simplest test first. Here it is:

You know, I had every intention of writing that first test, but I just realized I don’t know enough to. I have no idea, in code, how this input stuff is going to be used. Proceeding in the face of this unknown seems like a recipe to build the wrong thing, so I really need to take a minute and figure out how the input side is going to be used. For right now, I’ll assume this code is going to go in main. Here is main as I’ve sketched it out. I know it is not complete yet, but this is just a thought experiement:

public static void Main(string[] args)
{
    StreamReader inputStream = new StreamReader(args[0]);
    InputReader reader = new InputReader(inputStream);

    StreamWriter outputStream = new StreamWriter(args[1]);
    CheckWriter writer = new CheckWriter(outputStream);

    EmployeeList employees = new EmployeeList();
    employees.Add(“Bill”, 144000);
    employees.Add(“Frank”, 100000);

    Payroll payroll = new Payroll(employees);

    string payDate = null;
    while((payDate = reader.GetNext()) != null)
    {
        IList payRecords = payroll.Pay(payDate);
        writer.Write(payRecords);
    }
}

As far as I can tell, that’s about how main is going to look. As of right now, the InputReader is going to produce a payDate for each line in its input. I’m pretty sure that that is going to change a bit later, but I’ll worry about that later. I don’t want to take the time now to guess about what that class’s generic behavior is going to be, as I’ll know for certain in a subsequent story.

Now I can go back to my test:

[TestFixture]
    public class InputReaderFixture
    {
        [Test]
        public void NoPayrollDateReadFromEmptyInputStream()
        {
            StringReader inputStream = new StringReader(“”);
            InputReader reader = new InputReader(inputStream);

            Assert.IsNull(reader.GetNext());
        }
    }

Very simple, and, like a similar test for the output, worked without me having to actually write any application code beyond just creating the classes and methods.

public class InputReader
{
    public InputReader(TextReader inputStream)
    {
    }

    public string GetNext()
    {
        return null;
    }
}

Next test is to include one payday command on the input and make sure I get the right date out:

[Test]
public void WillReadSinglePayrollDateFromInput()
{
    StringReader inputStream = new StringReader(“Payday,05/01/2004” + System.Environment.NewLine);
    InputReader reader = new InputReader(inputStream);

    Assert.AreEqual(“05/01/2004”, reader.GetNext());
}

At this point, I’m getting a little nervous about my idea of just returning a date string from GetNext(). I’m beginning to think I’m going to need an IList of tokens from the input line. I don’t actually need that yet, but the shape of this system is a transaction processor, where the input comes in, one transaction per line, and we act on it. To make that happen, I think we’ll need a list of tokens per line. The question now is whether I should do this now, or wait until I actually need it.

As a side point, this kind of self-examination of your code and flashes of insight are what good TDDers do all the time. It is no longer as simple as drawing up a design in Visio and implementing it. We, as TDD-programmers, have to be constantly listening to our code, re-evaluating choices we’ve made, waiting for those fundamental insights, and acting on them. That is what makes TDD more difficult than Big Design Up Front (BDUF) programming, but it is also what makes it much better.

I’m really torn about whether I should put this insight into the code now, or whether I should wait until later. The lazy programmer in me says that I would have less code to change in my tests if I made the change now. There is a very slight risk that I could get the abstraction wrong, as I don’t actually need it right now, but I don’t think it would be too tough to get it right, now. I’ll do it, as soon as I get this test working.

public class InputReader
{
    private readonly TextReader inputStream;

    public InputReader(TextReader inputStream)
    {
        this.inputStream = inputStream;
    }

    public string GetNext()
    {
        string entireLine = inputStream.ReadLine();
        if(entireLine == null) return null;

        string [] tokens = entireLine.Split(‘,’);
        return tokens[1];
    }
}

Now the test works, but the GetNext() method is a bit ugly. I have the magic character and magic number in there, and the special case stuff. Don’t like it at all. Then again, we’re about to refactor it…

This turned out to be a really simple change, and I think I’m glad I did it now. The test changed a bit, and I think I like it more now:

[Test]
public void WillReadSinglePayrollDateFromInput()
{
    StringReader inputStream = new StringReader(“Payday,05/01/2004” + System.Environment.NewLine);
    InputReader reader = new InputReader(inputStream);

    IList commandTokens = reader.GetNext();
    Assert.AreEqual(2, commandTokens.Count);
    Assert.AreEqual(“Payday”, commandTokens[0]);
    Assert.AreEqual(“05/01/2004”, commandTokens[1]);
}

Now we can assert something about the entire input line. We can confirm that it only had 2 tokens on it, and we can confirm what they were. I just like that better — it seems more complete. And, in InputReader, all I had to do now was to return the array of tokens returned from Split, rather than pull the array apart and just returning one piece of it. Very simple.

All that is left to do now is to get more than one set of tokens from the InputReader, and I think this class is done.

[Test]
public void WillReadAllSetsOfTokensInInputStream()
{
    StringReader inputStream =
        new StringReader(“Payday,05/01/2004” + System.Environment.NewLine +
                        “Payday,06/01/2004” + System.Environment.NewLine);
    InputReader reader = new InputReader(inputStream);

    IList firstCommandTokens = reader.GetNext();
    Assert.AreEqual(2, firstCommandTokens.Count);
    Assert.AreEqual(“Payday”, firstCommandTokens[0]);
    Assert.AreEqual(“05/01/2004”, firstCommandTokens[1]);

    IList secondCommandTokens = reader.GetNext();
    Assert.AreEqual(2, secondCommandTokens.Count);
    Assert.AreEqual(“Payday”, secondCommandTokens[0]);
    Assert.AreEqual(“06/01/2004”, secondCommandTokens[1]);

    IList emptyCommandTokens = reader.GetNext();
    Assert.IsNull(emptyCommandTokens);
}

Once I wrote this test, to my surprise, it worked. I guess I wasn’t thinking about it clearly enough, but the initial implementation of GetNext() was sufficient. I really expected to have to write a while look in that method, to let me get each line of tokens. Looking back at the code, I understand completely why I didn’t need to, but I expected to. Had I just sat down and written the code, I may very well have written that loop, only to discover that I didn’t need it! Not real bright of me, but sometimes that happens. My tests saved me from doing something dumb.

This is the finished InputReader class:

public class InputReader
{
   private readonly TextReader inputStream;

   public InputReader(TextReader inputStream)
   {
       this.inputStream = inputStream;
   }

   public IList GetNext()
   {
       string entireLine = inputStream.ReadLine();
       if(entireLine == null) return null;

       return entireLine.Split(‘,’);
   }
}

The main program

Since that was so easy, let’s look at main now and make sure that it works correctly. After our earlier refactoring to change to IList of tokens, here is main:

public class PayrollMain
{
    public static void Main(string[] args)
    {
        StreamReader inputStream = new StreamReader(args[0]);
        InputReader reader = new InputReader(inputStream);

        StreamWriter outputStream = new StreamWriter(args[1]);
        CheckWriter writer = new CheckWriter(outputStream);

        EmployeeList employees = new EmployeeList();
        employees.Add(“Bill”, 144000);
        employees.Add(“Frank”, 100000);

        IList tokens = reader.GetNext();
        while(tokens.Count > 0)
        {
            Payroll payroll = new Payroll(employees);
            string payDate = tokens[1] as string;

            IList payRecords = payroll.Pay(payDate);
            writer.Write(payRecords);
        }
    }
}

After running this, there were a few minor problems that I had to find through debugging, which tells me that I really should have written this code the same way I wrote the rest of it, driving it through TDD. This is also a bit complex for a main, and the while loop at least should be refactored out, but I think I can stand leaving this for now right where it is. I’m pretty sure it will get refactored out during the next story into a factory or something, as soon as we start creating other kinds of commands, but I’ll leave that for another day.

At this point, I consider this first story to be finished. It works, there are no more refactorings I can think of, we have our main built. Main worked pretty much the first time I tried it. The only two changes I had to make were to change the loop condition to checking to see if tokens was null instead of checking that the count of tokens was 0, and I had to add a TextWriter.Flush() call into CheckWriter.Write(). Let’s pretend that this last change was a bug that slipped through our testing that was found in production. In the next, and last, installment for this story, we’ll talk about how to fix bugs in TDD.

Here is the final code for main and the InputReader and its tests:

[TestFixture]
public class InputReaderFixture
{
    [Test]
    public void NoPayrollDateReadFromEmptyInputStream()
    {
        StringReader inputStream = new StringReader(“”);
        InputReader reader = new InputReader(inputStream);

        Assert.IsNull(reader.GetNext());
    }

    [Test]
    public void WillReadSinglePayrollDateFromInput()
    {
        StringReader inputStream = new StringReader(“Payday,05/01/2004” + System.Environment.NewLine);
        InputReader reader = new InputReader(inputStream);

        IList commandTokens = reader.GetNext();
        Assert.AreEqual(2, commandTokens.Count);
        Assert.AreEqual(“Payday”, commandTokens[0]);
        Assert.AreEqual(“05/01/2004”, commandTokens[1]);
    }

    [Test]
    public void WillReadAllSetsOfTokensInInputStream()
    {
        StringReader inputStream =
            new StringReader(“Payday,05/01/2004” + System.Environment.NewLine +
            “Payday,06/01/2004” + System.Environment.NewLine);
        InputReader reader = new InputReader(inputStream);

        IList firstCommandTokens = reader.GetNext();
        Assert.AreEqual(2, firstCommandTokens.Count);
        Assert.AreEqual(“Payday”, firstCommandTokens[0]);
        Assert.AreEqual(“05/01/2004”, firstCommandTokens[1]);

        IList secondCommandTokens = reader.GetNext();
        Assert.AreEqual(2, secondCommandTokens.Count);
        Assert.AreEqual(“Payday”, secondCommandTokens[0]);
        Assert.AreEqual(“06/01/2004”, secondCommandTokens[1]);

        IList emptyCommandTokens = reader.GetNext();
        Assert.IsNull(emptyCommandTokens);
    }
}

public class InputReader
{
    private readonly TextReader inputStream;

    public InputReader(TextReader inputStream)
    {
        this.inputStream = inputStream;
    }

    public IList GetNext()
    {
        string entireLine = inputStream.ReadLine();
        if(entireLine == null) return null;

        return entireLine.Split(‘,’);
    }
}

public static void Main(string[] args)
{
    if(args.Length != 2)
    {
        Console.Out.WriteLine(“usage: PayrollMain <inputFile> <outputFile>”);
        return;
    }

    StreamReader inputStream = new StreamReader(args[0]);
    InputReader reader = new InputReader(inputStream);

    StreamWriter outputStream = new StreamWriter(args[1]);
    CheckWriter writer = new CheckWriter(outputStream);

    EmployeeList employees = new EmployeeList();
    employees.Add(“Frank Furter”, 120000);
    employees.Add(“Howard Hog”, 144000);

    IList tokens = reader.GetNext();
    while(tokens != null)
    {
        Payroll payroll = new Payroll(employees);
        string payDate = tokens[1] as string;

        IList payRecords = payroll.Pay(payDate);
        writer.Write(payRecords);

        tokens = reader.GetNext();
    }
}

A Great Refactoring Experience

This post is going to be entirely non-technical. Instead, I want to relate a refactoring experience I had yesterday with Peter Provost.

Peter is working on something that he has never worked on before, Event Tracing in Windows (ETW). This is a very low-level, kernel-provided facility to track events in Windows. Accessing these APIs is hard, because you have to P/Invoke to get to all the calls, and it involves messy logic in .Net to make it all work. So, to let him go faster, I’m pairing with him (I’ve never seen this before either, but between us, we’re making good progress).

Well, we had written one test, and we had all the logic in this one single test. We didn’t know how to do any of this ETW stuff yet, so we were using our tests to explore the API. So, we got this first test to work, and then started to refactor out the non test-specific code into another class that we called a TraceEventGuidCollection. Not to hard to do, so we finished that pretty quickly. Then we started looking at that new class, and both of us realized that it was just too complex. It knew how to handle the specific TraceEvent stuff that we were working with, and it also knew about how to create, destroy, and manage unmanaged arrays of pointers in .Net. It seemed like two classes to us, but we had no idea how to split it in half. So we just did it. We had no idea where we were going, we had no idea what our plan should be — we were pretty clueless. So we just started doing an ExtractClass refactoring by the book.

We pulled out our IntPtr [] array into the new class we had just created, and changed the original TraceEventGuidCollection class to use the array in its new home. Then we started moving over all the methods that used the array into the new class, one by one, addressing problems as we found them. The interesting part that we both noticed at this point is that we still had no idea of where we were going, but we knew we liked it. We’ve only known each other for a couple of months, but we’ve built up a level of respect and trust between us, so we were able to proceed together without fear. We knew we’d get somewhere good.

As we proceeded through the class, we discovered that we had two methods that were 89% identical, but there was just enough different to make our lives difficult. This was the outline of each function:

public void DoSomething()
{
    // setup stuff
    // AllocateArrays
    // Fill Arrays
    // if(error occurs) throw win32exception
    // Cleanup
}

The problems that we had were that the way we detected an error was different, and what the method returned was different. In one case, we were in the constructor, so we didn’t want to return anything. In the other case, we were using the API to figure out how many slots to allocate into our array, using some API tricks, and that had to return a number.

So our problem was that the code was really, really close to the same, but just enough different to make refactoring them both to look the same. We spun on this for about a half hour, trying different things to make them look the same, refactor them over to the other class, etc, and ended up backing out each of them. Finally, we decided to solve the smaller problem of just moving the constructor logic over. But in the process of doing this, we had to make the code much uglier than it was in its current home. We held our noses and did it, again confident that we could clean it up later.

We refactored this first piece of code over, got everything to work, and then went to work on the second method. Once we finally finished this, Peter was still unhappy with how the classes were laid out. Because we had refactored them and paid attention to where the responsibilities should lie, we had this totally subjective feeling that something was wrong. Peter and I looked at it for a bit, and finally realized that the second method was totally unneeded. We got rid of it, changed our constructor a bit to pick up a little of that responsibility, and checked in the code.

This was one medium sized piece of our day, and we were exhausted at this point. But the two classes we had come up with were now factored much better, we had each learned something about how the worked, and we had a great time

If people only realized what a great creative process refactoring was, there would be a lot more people doing it!

– bab

 

The Never-ending TDD Story — Part 5

[Note: I’m using Copy as HTML to transfer code from VS.Net to BlogJet. This tool embeds style sheets into the posted blog entry to provide the nice formatting for the source code. Unfortunately, the version of .Text used by Dot Net Junkies won’t let me post the embedded style sheets. This means that viewing this article in an aggregator will look different, and worse, than viewing it on the DotNetJunkies site. I recommend viewing it there, as the code will be more easily readable than in an aggregator — bab]

As promised, we are finally adding the input and output handling stuff. If you can recall from way back in installment one, I chose to implement this stuff last, because it wasn’t challenging and it wasn’t the core of the problem. In other words, I was pretty sure I could make it work when I had to, so I wasn’t worried about it. Well, since the meat of the story is now finished, I need to do my I/O, and here it is.

Output

I’m going to start working on the output side first, just for the fun of it. I’m not exactly sure why I’m doing this, but it may be because the output side already has a fully formed user, the Payroll class, while the input is a little foggier. Should the input reading drive the system, should there be something else out there that drives the whole thing… I don’t know yet, so I’m going to do the output side first.

So, as we discussed in the first installment, there is this company that will print the checks for me, but I have to give them a properly formatted batch file. This was the sample output that we had:

Check,100,Frank Furter,$10000,05/01/2004
Check,101,Howard Hog,$12000,05/01/2004
Check,102,Frank Furter,$10000,06/01/2004
Check,103,Howard Hog,$12000,06/01/2004

Out output class just has to mimic this exactly. No problem!

Let’s start implementing this new class. As before, we need to create a test list before we can start working on the code. This helps me get my thoughts straight about what this class needs to do. Here is my first shot at a test list:

  1. Should print no output if there are no pay records
  2. Should print exactly one line if there is a single pay record
  3. Check numbers always start at 100 after object created
  4. Check numbers keep incrementing throughout lifetime of check writer object
  5. Should print two lines if there are two pay records

Armed with that test list, I’m going to start writing my CheckWriter class. I choose to write test number 1 first, as it seems to be the most basic, and will force me to write at least a stub for all the classes I’ll need. Here’s that test:

     1: [TestFixture]
     2: public class CheckWriterFixture
     3: {
     4:     [Test]
     5:     public void NoOutputWrittenIfNoPayRecords()
     6:     {
     7:         IList payRecords = new ArrayList();
     8:         StringWriter checkData = new StringWriter();
     9:  
    10:         CheckWriter writer = new CheckWriter(checkData);
    11:         writer.Write(payRecords);
    12:  
    13:         Assert.AreEqual(string.Empty, checkData.ToString());
    14:     }
    15:  
    16:     private class CheckWriter
    17:     {
    18:         public CheckWriter(TextWriter checkData)
    19:         {
    20:         }
    21:  
    22:         public void Write(IList payRecords)
    23:         {
    24:         }
    25:     }
    26: }

Now the interesting thing here is that I intended to just stop here so I could watch my test fail. But I ran it and it worked… It turns out that I don’t need to do anything inside the Write() method to make the test pass. But I consider this test to still be very valuable. First of all, it got me focused on the interface I intended to support without having to worry about the details of how I would implement that interface. Secondly, the CheckWriter class has to respond in a predictable way when the list of PayRecords it gets is empty. Now, no matter how the code mutates over time, I can still confirm that the empty case does the right thing. So I guess I’m finished with this first test.

The next test tries to write a single entry into the output stream for a single PayRecord. I do this simple case so I can get the line formatting correct. I could have jumped straight to passing in a bunch of PayRecords, but then I’d have had to worry about details about iteration as well as formatting, and that step is too big. The smaller step I chose to take will make me focus strictly on the formatting of a single line. Here is that test:

     1: [Test]
     2: public void SinglePayRecordFormattedCorrectlyWhenWritten()
     3: {
     4:     IList payRecords = new ArrayList();
     5:     payRecords.Add(new PayRecord("Bill", 1000));
     6:  
     7:     StringWriter checkData = new StringWriter();
     8:  
     9:     CheckWriter writer = new CheckWriter(checkData);
    10:     writer.Write(payRecords);
    11:  
    12:     Assert.AreEqual("Check,100,Bill,$1000,05/01/2004" + 
                            System.Environment.NewLine, checkData.ToString());
    13: }

Well, isn’t this interesting… To format the output properly, I need the date for a PayRecord. I completely missed this! I’ll have to go back to the PayRecord class and change that class to hold the pay date, and fix up any tests and calling code to handle the date. Before I do this, though, is there any way I could have avoided this error? I suppose I could have started with the CheckWriter as my first class, establishing how the output would be written and what data it would contain. But I’d have done that in kind of a vacuum, since I wouldn’t have had the essence of the problem solved het. Would I have come up with the PayRecord class had I started with the CheckWriter first? I don’t know. I almost certainly wouldn’t have called it a PayRecord yet, because I woudn’t have known I needed something like that. To be honest, I’m pretty sure that there would be no sure-fire way to avoid an error like this. What I do know is that I have my tests to back me up, so this change should be pretty easy to make. So I’ll make it.

To make this change, I went into PayrollFixture and changed the first test I found that did any assertions against the contents of a PayRecord. I included an assertion to check the pay date. Then I changed PayRecord to take a pay date in its constructor and added a getter property to get the pay date back out. Compiled, found where I had to change the calls to the constructor to PayRecord and ran my tests. The test I just updated passed, so I went back and updated the rest of the tests in PayFixture to check the pay date as well. The key here is that I changed a test to direct me to what to change in my application code. I changed it in the test, then followed that to the place I had to change. Then the compiler was able to tell me what else needed to change. Moral of the story is to listen to your tools — they’ll tell you where to go to make these changes.

So, back on track now, I finish implementing the test I was originally writing…

     1: public class CheckWriter
     2: {
     3:     private TextWriter checkData;
     4:  
     5:     public CheckWriter(TextWriter checkData)
     6:     {
     7:         this.checkData = checkData;
     8:     }
     9:  
    10:     public void Write(IList payRecords)
    11:     {
    12:         string name = ((PayRecord)payRecords[0]).Name;
    13:         int pay = ((PayRecord)payRecords[0]).Pay;
    14:         string date = ((PayRecord)payRecords[0]).PayDate;
    15:  
    16:         string payData = "Check,100," + name + ",$" + pay + "," + date;
    17:         checkData.WriteLine(payData);
    18:     }
    19: }

This implementation suffices to make this new test pass. I know there is no iteration in it, I know I’ll need that eventually, but by not putting it in, I was able to focus in getting the formatting just right. Before I can move on, though, I need to make sure that my first test still passes. So I run it, and sure enough, it fails! So already, the test that was really trivial to write and to implement turns out to be somewhat valuable. To make this first test pass, I have to bail out of my Write method if there are no payRecords in the parameter. Simple change to make, and then all my tests run again.

For the next test, I’ll implement test #3. In this test, I need to make sure that whenever I create and use a new CheckWriter, the check number in the output always starts at 100. Here’s the test:

     1: [Test]
     2: public void CheckRecordsAlwaysStartAt100ForNewInstances()
     3: {
     4:     IList payRecords = new ArrayList();
     5:     payRecords.Add(new PayRecord("Bill", 1200, "05/01/2004"));
     6:  
     7:     StringWriter firstCheckData = new StringWriter();
     8:     CheckWriter firstWriter = new CheckWriter(firstCheckData);
     9:     firstWriter.Write(payRecords);
    10:  
    11:     Assert.AreEqual("Check,100,Bill,$1200,05/01/2004" + System.Environment.NewLine, firstCheckData.ToString());
    12:  
    13:     StringWriter secondCheckData = new StringWriter();
    14:     CheckWriter secondWriter = new CheckWriter(secondCheckData);
    15:     secondWriter.Write(payRecords);
    16:  
    17:     Assert.AreEqual("Check,100,Bill,$1200,05/01/2004" + System.Environment.NewLine, secondCheckData.ToString());
    18: }
    19:  

And it passed the first time, since we’re hardcoding in the check number in the output string. We could go back and extract that hardcoded value from the output string in CheckWriter right now, or we could implement the next test to make us do that. I think I’d rather do the second thing, just because it sounds more interesting to me. You could certainly go back in now and refactor the hardcoded value into an instance variable now if you choose to, but I think I want to wait until I have a test that demands it, so I’m certain that I implement it the right way. Here’s the next test:

     1: [Test]
     2: public void CheckNumbersIncrementEachTimeWriteMethodIsCalled()
     3: {
     4:     IList payRecords = new ArrayList();
     5:     payRecords.Add(new PayRecord("Bill", 1200, "05/01/2004"));
     6:  
     7:     StringWriter checkData = new StringWriter();
     8:     CheckWriter writer = new CheckWriter(checkData);
     9:     writer.Write(payRecords);
    10:     writer.Write(payRecords);
    11:  
    12:     Assert.AreEqual("Check,100,Bill,$1200,05/01/2004" + System.Environment.NewLine +
    13:                     "Check,101,Bill,$1200,05/01/2004" + System.Environment.NewLine, checkData.ToString());
    14: }

Now, on lines 9 and 10, I call the Write() method twice, and I expect the check number to change. I know that it doesn’t make sense to pay Bill twice for the same payday, but, hey, I own this business, and I can do as I please! The important thing here is to make the check number increment. I can handle validation logic of what is being printed somewhere else if I choose to. The responsibility of the CheckWriter class is just to write the output. If a PayRecord is in its input, it should show up in its output. So, let’s go make that change inside the CheckWriter class.

Step one is to ignore the test we are trying to implement now. This test isn’t going to work until we make this refactoring in CheckWriter, and I don’t like refactoring with broken tests. So, I ignore it, and then move on to the refactoring.

The smallest step I can take here at first is to extract the hardcoded “100” from the string in Write() and make it a local variable:

     1: public void Write(IList payRecords)
     2: {
     3:     string name = ((PayRecord)payRecords[0]).Name;
     4:     int pay = ((PayRecord)payRecords[0]).Pay;
     5:     string date = ((PayRecord)payRecords[0]).PayDate;
     6:  
     7:     int checkNumber = 100;
     8:  
     9:     string payData = "Check," + checkNumber + "," + name + ",$" + pay + "," + date;
    10:     checkData.WriteLine(payData);
    11: }

Compile and run tests, see that they all work, and then promote the local variable into an instance variable. Compile again, run tests, all works. The key in this step is that I didn’t make the jump to an instance variable all in one big leap. I’m pretty sure I could have, and gotten it right, but the challenge in refactoring is to make changes in small steps. This was an extreme example, but there are certainly times when I do things in steps that are this small. Here is the current CheckWriter code, after this change:

     1: public class CheckWriter
     2: {
     3:     private TextWriter checkData; 
     4:     private int checkNumber = 100;
     5:  
     6:     public CheckWriter(TextWriter checkData)
     7:     {
     8:         this.checkData = checkData;
     9:     }
    10:  
    11:     public void Write(IList payRecords)
    12:     {
    13:         string name = ((PayRecord)payRecords[0]).Name;
    14:         int pay = ((PayRecord)payRecords[0]).Pay;
    15:         string date = ((PayRecord)payRecords[0]).PayDate;
    16:  
    17:         string payData = "Check," + checkNumber + "," + name + ",$" + pay + "," + date;
    18:         checkData.WriteLine(payData);
    19:     }
    20: }

Now, lets un-ignore the test we were implementing, and make that work. I won’t bore you with seeing the code again after this change. All I had to do was to add ++ after the checkNumber on line 17, and the incrementing works just fine. On to our next test, paying multiple PayRecords in a single call to Write().

Here is the code for that test:

     1: [Test]
     2: public void WriteOutputForAllPayRecordsInGivenList()
     3: {
     4:     IList payRecords = new ArrayList();
     5:     payRecords.Add(new PayRecord("Tom", 100, "01/01/2004"));
     6:     payRecords.Add(new PayRecord("Sue", 500, "02/01/2004"));
     7:  
     8:     StringWriter checkData = new StringWriter();
     9:     CheckWriter writer = new CheckWriter(checkData);
    10:  
    11:     writer.Write(payRecords);
    12:  
    13:     Assert.AreEqual("Check,100,Tom,$100,01/01/2004" + System.Environment.NewLine +
    14:                     "Check,101,Sue,$500,02/01/2004" + System.Environment.NewLine, checkData.ToString());
    15: }

Now that this test is written, we finally have an excuse to add the iteration to the Write() method, and we do so very simply:

     1: public void Write(IList payRecords)
     2: {
     3:     foreach(PayRecord payRecord in payRecords)
     4:     {
     5:         string name = payRecord.Name;
     6:         int pay = payRecord.Pay;
     7:         string date = payRecord.PayDate;
     8:  
     9:         string payData = "Check," + checkNumber++ + "," + name + ",$" + pay + "," + date;
    10:         checkData.WriteLine(payData);
    11:     }
    12: }

Once again, we rerun all of our tests, and everything passes, so we’re just about finished. The last thing I want to do is to get rid of all the local variables in the Write() method. Now that we have the foreach loop, we don’t have to do the nasty casting we had in the code previously, so I’m not sure the locals are really helping me out any more. And I can get rid of payData as well, I think, putting everything into the checkData.WriteLine() line.

The final point I want to make about this is that it was interesting that I was testing code whose job it was to write to an output file, yet I never touched a single file in any of my tests. There is one big reason I can do this — polymorphism. .Net doesn’t care what kind of TextWriterI’m writing to, and I’m taking advantage of that. I’ve written my code to the base TextWriter class, which allows me to use StringWriters in my tests for convenience and StreamWriters in my final version to write to the file system. This way I don’t have to clutter my test code with file handling logic, including file creation, deletion, reading, etc. Everything is neatly wrapped up in a StringWriter that I can access with a simple ToString(). It makes the tests much easier to read and understand, reducing all that extra plumbing noise. I do this where ever possible — write my interfaces to the most base class I can. That gives me the most freedom at runtime to choose between different kinds of implementations to use.

Here is the final code for this episode:

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

namespace PayrollExercise
{
    [TestFixture]
    public class PayrollFixture
    {
        [Test]
        public void NoOnePaidIfNoEmployees()
        {
            EmployeeList employees = new EmployeeList();
            Payroll payroll = new Payroll(employees);

            IList payrollOutput = payroll.Pay(“05/01/2004”);

            Assert.AreEqual(0, payrollOutput.Count);
        }

        [Test]
        public void PayOneEmployeeOnFirstOfMonth()
        {
            EmployeeList employees = new EmployeeList();
            employees.Add(“Bill”, 1200);

            Payroll payroll = new Payroll(employees);

            IList payrollOutput = payroll.Pay(“05/01/2004”);

            Assert.AreEqual(1, payrollOutput.Count);

            PayRecord billsPay = payrollOutput[0] as PayRecord;
            Assert.AreEqual(“Bill”, billsPay.Name);
            Assert.AreEqual(100, billsPay.Pay);
            Assert.AreEqual(“05/01/2004”, billsPay.PayDate);
        }

        [Test]
        public void PayAllEmployeesOnFirstOfMonth()
        {
            EmployeeList employees = new EmployeeList();
            employees.Add(“Bill”, 1200);
            employees.Add(“Tom”, 2400);

            Payroll payroll = new Payroll(employees);

            IList payRecords = payroll.Pay(“05/01/2004”);

            Assert.AreEqual(2, payRecords.Count);

            PayRecord billsPay = payRecords[0] as PayRecord;
            Assert.AreEqual(“Bill”, billsPay.Name);
            Assert.AreEqual(100, billsPay.Pay);
            Assert.AreEqual(“05/01/2004”, billsPay.PayDate);

            PayRecord tomsPay = payRecords[1] as PayRecord;
            Assert.AreEqual(“Tom”, tomsPay.Name);
            Assert.AreEqual(200, tomsPay.Pay);
            Assert.AreEqual(“05/01/2004”, tomsPay.PayDate);
        }

        [Test]
        public void NoEmployeePaidIfNotFirstOfMonth()
        {
            EmployeeList employees = new EmployeeList();
            employees.Add(“Johnny”, 3600);

            Payroll payroll = new Payroll(employees);
            IList emptyList = payroll.Pay(“05/02/2004”);

            Assert.AreEqual(0, emptyList.Count);
        }
    }

    public class Payroll
    {
        private EmployeeList employees;
        private ArrayList payRecords;
        private string payDate;

        public Payroll(EmployeeList employees)
        {
            this.employees = employees;
        }

        public IList Pay(string payDate)
        {
            this.payDate = payDate;
            payRecords = new ArrayList();

            employees.Apply(new EmployeeList.ApplyAction(PayEmployee));

            return payRecords;
        }

        private void PayEmployee(Employee employee)
        {
            employee.Pay(payDate, payRecords);
        }
    }

    public class EmployeeList
    {
        private ArrayList employees = new ArrayList();

        public delegate void ApplyAction(Employee employee);

        public void Apply(ApplyAction action)
        {
            foreach(Employee employee in employees)
            {
                action(employee);
            }
        }

        public void Add(string employeeName, int yearlySalary)
        {
            employees.Add(new Employee(employeeName, yearlySalary));
        }

        public int Count
        {
            get { return employees.Count; }
        }
    }

    public class Employee
    {
        private string name;
        private int salary;

        public Employee(string name, int yearlySalary)
        {
            this.name = name;
            this.salary = yearlySalary;
        }

        private string Name
        {
            get { return name; }
        }

        private int CalculatePay()
        {
            return salary/12;
        }

        public void Pay(string payDate, IList payRecords)
        {
            if(IsPayday(payDate))
            {
                PayRecord payRecord = new PayRecord(Name, CalculatePay(), payDate);
                payRecords.Add(payRecord);
            }
        }

        private bool IsPayday(string payDate)
        {
            string [] dateParts = payDate.Split(new char[] {‘/’});
            return dateParts[1] == “01”;
        }
    }

    public class PayRecord
    {
        private string name;
        private int pay;
        private string payDate;

        public PayRecord(string name, int pay, string payDate)
        {
            this.name = name;
            this.pay = pay;
            this.payDate = payDate;
        }

        public string Name
        {
            get { return name; }
        }

        public int Pay
        {
            get { return pay; }
        }

        public string PayDate
        {
            get { return payDate; }
        }
    }

    [TestFixture]
    public class CheckWriterFixture
    {
        [Test]
        public void NoOutputWrittenIfNoPayRecords()
        {
            IList payRecords = new ArrayList();
            StringWriter checkData = new StringWriter();

            CheckWriter writer = new CheckWriter(checkData);
            writer.Write(payRecords);

            Assert.AreEqual(string.Empty, checkData.ToString());
        }

        [Test]
        public void SinglePayRecordFormattedCorrectlyWhenWritten()
        {
            IList payRecords = new ArrayList();
            payRecords.Add(new PayRecord(“Bill”, 1000, “05/01/2004”));

            StringWriter checkData = new StringWriter();

            CheckWriter writer = new CheckWriter(checkData);
            writer.Write(payRecords);

            Assert.AreEqual(“Check,100,Bill,$1000,05/01/2004” + System.Environment.NewLine, checkData.ToString());
        }

        [Test]
        public void CheckRecordsAlwaysStartAt100ForNewInstances()
        {
            IList payRecords = new ArrayList();
            payRecords.Add(new PayRecord(“Bill”, 1200, “05/01/2004”));

            StringWriter firstCheckData = new StringWriter();
            CheckWriter firstWriter = new CheckWriter(firstCheckData);
            firstWriter.Write(payRecords);

            Assert.AreEqual(“Check,100,Bill,$1200,05/01/2004” + System.Environment.NewLine, firstCheckData.ToString());

            StringWriter secondCheckData = new StringWriter();
            CheckWriter secondWriter = new CheckWriter(secondCheckData);
            secondWriter.Write(payRecords);

            Assert.AreEqual(“Check,100,Bill,$1200,05/01/2004” + System.Environment.NewLine, secondCheckData.ToString());
        }

        [Test]
        public void CheckNumbersIncrementEachTimeWriteMethodIsCalled()
        {
            IList payRecords = new ArrayList();
            payRecords.Add(new PayRecord(“Bill”, 1200, “05/01/2004”));

            StringWriter checkData = new StringWriter();
            CheckWriter writer = new CheckWriter(checkData);
            writer.Write(payRecords);
            writer.Write(payRecords);

            Assert.AreEqual(“Check,100,Bill,$1200,05/01/2004” + System.Environment.NewLine +
                “Check,101,Bill,$1200,05/01/2004” + System.Environment.NewLine, checkData.ToString());
        }

        [Test]
        public void WriteOutputForAllPayRecordsInGivenList()
        {
            IList payRecords = new ArrayList();
            payRecords.Add(new PayRecord(“Tom”, 100, “01/01/2004”));
            payRecords.Add(new PayRecord(“Sue”, 500, “02/01/2004”));

            StringWriter checkData = new StringWriter();
            CheckWriter writer = new CheckWriter(checkData);

            writer.Write(payRecords);

            Assert.AreEqual(“Check,100,Tom,$100,01/01/2004” + System.Environment.NewLine +
                “Check,101,Sue,$500,02/01/2004” + System.Environment.NewLine, checkData.ToString());
        }
    }

    public class CheckWriter
    {
        private TextWriter checkData;
        private int checkNumber = 100;

        public CheckWriter(TextWriter checkData)
        {
            this.checkData = checkData;
        }

        public void Write(IList payRecords)
        {
            foreach(PayRecord payRecord in payRecords)
            {
                checkData.WriteLine(“Check,” + checkNumber++ + “,” +
                    payRecord.Name + “,$” +
                    payRecord.Pay + “,” + payRecord.PayDate);
            }
        }
    }
}