DateTime.Now is Evil

Introduction

Unit testing is a core concept of following agile processes. However, it can seem difficult and even a waste of time to write tests for all but the most simple of functionality. This tends to be caused by deceptively simple looking code which turns out to be hiding complexities. What seems like a simple method may in fact be using several different subsystems to get its work done – subsystems which must be factored out of the code under test. This post details one of my experiences in that area.

First Experiences with Unit Testing

I remember one of my first experiences of learning unit testing. I decided to develop a Person class and write some tests for it using NUnit – something along the lines of:

  1. class Person
  2. {
  3.     public String Name { get; private set; }
  4.     public DateTime DateOfBirth { get; private set; }
  5.  
  6.     public Person(String name, DateTime dob)
  7.     {
  8.         if (String.IsNullOrEmpty(name))
  9.         {
  10.             // Please excuse the fact name might be String.Empty –
  11.             // it's just an example!
  12.             throw new ArgumentNullException("name");
  13.         }
  14.  
  15.         Name = name;
  16.         DateOfBirth = dob;
  17.     }
  18. }
  19.  
  20. class PersonTest
  21. {
  22.     [Test, ExpectedException(typeof(ArgumentNullException))]
  23.     public void TestCtorFail()
  24.     {
  25.         new Person(null, new DateTime(1950, 7, 3));
  26.     }
  27.  
  28.     [Test]
  29.     public void TestCtor()
  30.     {
  31.         var name = "Bill";
  32.         var dob = new DateTime(1950, 7, 3);
  33.         var p = new Person("Bill", dob);
  34.         Assert.AreEqual(name, p.Name);
  35.         Assert.AreEqual(dob, p.DateOfBirth);
  36.     }
  37. }

That was all well and good. Next I came to write an Age property:

  1. class Person
  2. {
  3.     // … rest of Person class.
  4.  
  5.     public Int32 Age
  6.     {
  7.         get
  8.         {
  9.             var now = DateTime.Today;
  10.             var age = now.Year DateOfBirth.Year
  11.             if (DateOfBirth > now.AddYears(age))
  12.             {
  13.                 age;
  14.             }
  15.             return age;
  16.         }
  17.     }
  18. }
  19.  
  20. class PersonTest
  21. {
  22.     public void TestAge()
  23.     {
  24.         // TODO: Test the Age property.
  25.     }
  26. }

I now had a problem, and it wasn’t the fact that I wasn’t following strict TDD.

Problems

I wanted to write a test for Age which was repeatable, and was runnable at any time – properties that any half decent unit test should have. However, this wasn’t possible due to my perfectly reasonable implementation of Age. I had two options for writing the tests:

  • I could select a constant value for the date of birth to test with, but the correct age would be different depending upon the day the test was run. The person in the test would age.
  • I could calculate a value for the date of birth to use based upon the current date. The expected age would then be the same no matter when the test was run. The date of birth would increase with the day the unit test was run on.

Both of these options have severe problems. Neither result in tests that run the same way every time they’re run. There’s potential for them to fail on some runs, but pass on others. Implementing either option would involve writing code similar to what’s already in the Age property. Even if we decided to implement one of them, there’s still the matter of what happens when the test is run close to midnight. Assuming a person was born at midnight, our test could determine an expected age just before midnight, and the Age property get accessed just after, resulting in a failure.

To me, it seemed like unit testing had failed at the second hurdle. For anything more complex than a simple calculation or constructor call, it just simply wouldn’t work, or would give flaky tests that could fail at any moment. Unit testing evangelists were quite simply talking absolute garbage that was only appropriate for their overly simplified examples.

Welcome to the Real World

Imagine writing a unit test for DateTime.Now. It would be pretty difficult. After all, how else does your code know what time it is? What are you going to do? Launch the test at a known time, start a System.Diagnostics.Stopwatch, stop it, and compare? Sure, that’s gonna work. It’s just as difficult to write a test for something that’s dependant upon DateTime.Now, DateTime.Today, or for that matter any other mutable global variable. They don’t come much more global and mutable than DateTime.Now (although they do come more unpredictable, unless someone’s screwing with the system time).

In order to write stable unit tests, you have to have control over dependencies. That includes those external systems your application interacts with – the database you use, the file system you read from, and even your system’s clock.

The reason my Age property was hard to test was because not only had I included a highly volatile global variable, I’d also implicitly linked my code to an external system I had little control over. Sure I can set the time, but can I stop it while I run a test? Can I guarantee that some time sync operation’s not going to screw my test over?

Let’s summarize the issues:

  • We’ve linked our Person class directly to an external system.
  • We’re trying to test code involving a highly volatile global variable we have no control over.

It’s no wonder I found writing tests difficult.

Taking Control Over Time

We’re linking Person directly to an external system. Interfaces are extremely good at defining interactions with external systems. Rather than accessing the current date and time through DateTime.Now, we should be going through an interface, so that our core classes need no direct knowledge of an implementation:

  1. public interface ISystemClock
  2. {
  3.     DateTime Now { get; }
  4. }

We can then create an implementation which uses DateTime.Now. A class like this is often called a boundary class because it exists on the boundaries of a system in order to interact with another system.

  1. public class RealSystemClock : ISystemClock
  2. {
  3.     public DateTime Now { get { return DateTime.Now; } }
  4. }

We can now provide a custom implementation of ISystemClock for testing, which always returns the time we want it to be (a mocking framework such as Moq makes this easy). If the system was more complex and required the testing, we could even write an implementation which accelerated time or simulated clocks going forward for the Summer if we needed to. I’ve done this in other projects and it can be extremely powerful if you’re project does a lot of time based stuff.

Removing the Dependency from Person

The next part sometimes takes a little more thought. We could go down the dependency injection route, by passing an instance of ISystemClock to a Person constructor, and have the Age property use it in its calculation. Dependency injection is often the first thing people think of when factoring out a statically defined dependency. However, taking a step back, that seems a little wrong for this example. Why should a person have to know the time in order to be created? A person and time seem quite unrelated. Why couldn’t you have a person without knowing the time? This yields more architectural questions:

  • Rather than an Age property, should we use an Age(ISystemClock clock) method?
  • Why does an age calculation need time-telling abilities? Couldn’t we do Age(DateTime now)?
  • Should Age even be a member on Person? Sure, from an object-oriented perspective, a person has an age, but we might want to calculate the age of other things. Should it go in some base class?
  • Could the age calculation go on the ISystemClock interface?
  • Should an Age calculation be more like a function of two DateTimes? Why should it belong to any class?

This is typical of the thought process one goes through when devising ways to unit test their code. The power of unit testing isn’t just the fact that you end up with a suite of tests which add a layer of checking over your compiler, it’s that you have to put a lot more thought into class dependencies and architecture. You have to make things simple and clean, or else you can’t test things well. In fact, some studies show that simply thinking about how to test code and skipping the actual writing of tests can still have big benefits regarding bug rates.

In my opinion, the logic to calculate someone’s age should be as simple as possible. That is, a function of two DateTimes – that’s all the input that’s needed. What should it belong to? It depends on the system. For now, because of its purity and the fact that we’re not going to want to swap it out for some other implementation, it’s fine as a static method in a static class. Statics are usually evil, especially where unit testing is concerned. However, as this is currently a pure piece of logic which we’re not going to want to swap out any time soon, a static is fine. If we need to change it in the future, perhaps to support age calculations for different cultures, it should be easy to refactor appropriately.

  1. public static class DateTimeUtils
  2. {
  3.     public static Int32 GetAgeInYears(DateTime startDate, DateTime endDate)
  4.     {
  5.         var now = DateTime.Today;
  6.         var age = endDate.Year startDate.Year
  7.         if (startDate.Year > endDate.AddYears(age))
  8.         {
  9.             age;
  10.         }
  11.         return age;
  12.     }
  13. }

In my opinion, we’re better off dropping the Age property on Person altogether, leaving Person a much simpler class. If I had to include Age on Person, I’d go for an Age(DateTime now) method which called our method defined above.

I’d like to re-iterate however that defining this as a static method is only acceptable because it’s not dependant upon any mutable globals. Calling code must take a snapshot of the current DateTime and pass it in explicitly. GetAgeInYears represents a pure function, where the same inputs always give the same outputs.

The new GetAgeInYears method can be tested easily using NUnit’s TestCaseSource attribute for the test cases:

  1. [TestCaseSource("GetAgeInYearsTestCases")]
  2. public Int32 TestGetAgeInYears(DateTime startDate, DateTime endDate)
  3. {
  4.     return DateTimeUtils.GetAgeInYears(startDate, endDate);
  5. }
  6.  
  7. public IEnumerable<ITestCaseData> GetAgeInYearsTestCases()
  8. {
  9.     yield return new TestCaseData(new DateTime(1970, 10, 13), new DateTime(2011, 3, 25)).Returns(40);
  10.     // etc…
  11. }

What About ISystemClock?

Note that we’ve not got ISystemClock involved at this moment. We’ve pushed the responsibility for determining the current time and a Person’s age to consuming code. In doing so we’ve kept our Person class and other logic simple and unit-testable. This is another common result of writing testable code. The definitions of external systems and dependencies get pushed back towards the entry point of the application. This leaves the rest of the system easy to test. For a big system, this can become an issue, and is exactly the problem that dependency injection frameworks such as ninject were created to solve.

If we’ve just got a form to display people and ages, it wouldn’t be difficult to calculate the ages there, close to the the system’s boundaries, leaving what’s internal to the system easily testable. The ISystemClock could be passed to the form’s constructor by calling code. That’s no problem at all for a little application like that. Larger applications, where large object graphs are constructed up front may need a little more thought, and use of one of the aforementioned dependency injection frameworks can ease the burden.

Conclusions

Unit testing can seem like a daunting exercise, especially when you’re introducing tests to an existing system not written with testability in mind. You can work around this by refactoring in order to isolate external systems. However, just as it’s important to isolate external systems, it’s also important to consider whether or not dependencies on those systems are actually needed at all. Once a developer has got their head around the technique of dependency injection, it’s easy to fall into the trap of having every class in an application accept dependencies on systems a better architecture would have them know nothing about.

To summarize:

  • Reduce dependencies your code has on external systems and mutable static state by refactoring.
  • Inject dependencies on external systems only when it makes sense at a design level to do so. Don’t get too DI-Happy.

The concepts of unit testing and dependency injection contain a lot of material to get opinionated about. “It depends” applies very much here, especially when we all work on systems of massively varying scale. However, I hope this post has highlighted to some why they may not appreciate unit testing and encourage them to give it another chance.

Share and Enjoy:
  • Print
  • Digg
  • StumbleUpon
  • del.icio.us
  • Facebook
  • Yahoo! Buzz
  • Twitter
  • Google Bookmarks
  • email
  • LinkedIn
  • Technorati

Leave a Reply

Your email address will not be published. Required fields are marked *