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);
            }
        }
    }
}

<p>
  <!--EndFragment-->
</p>