Why using DateTime.Now directly is a bad idea
Tags: csharp, testing, cleancode, dotnet
Published: 2020-11-29 11:08:10

###Introduction###

In this post I'm explaining why using DateTime.Now directly in the code is not the best idea and what we can do in order to ensure testability of dates / time in the code.

###Our code is often dependent on time###
That is the reality - I worked in many projects and in every single project we used dates / time to implement some logic. It is hard to imagine a business domain free of date / time dependency.

On the other hand I've seen a lot of bad practices regarding to using dates / time in the code. Let's focus now on the most obvious code smell in my opinion. That thing is using DateTime.Now static property directly in the code. Let's get our hands dirty and write some code:

```C#
public class Greeting
{
    public string SayHello()
    {
        var hour = DateTime.Now.Hour;
        if (hour < 12)
            return "Good Morning!";
        if (hour < 18)
            return "Good Afternoon!";
        return "Good Evening";
    }
}
```

The logic in our class is very simple. The method SayHello returns appropriate greeting depending on the current time. At the moment when I'm writing this post, it's 10:40 am in Poland so I'm expecting that the method is going to return a string "Good Morning!". I don't want to rely on my guesses, I'm going to create an unit test and cover the logic with a test.

```C#
[Fact]
public void SayHello_Test()
{
    // Arrange
    var greeting = new Greeting();
    // Act
    var result = greeting.SayHello();
    // Assert
    Assert.Equal("Good Morning!", result);
}
```

After running the test I got confirmation - my method works as expected. So far so good! Unfortunately there is one big issue in our code which leads to a serious problem in the test. Our test is not *repeatable*. It works now but if I wait for two hours, the test will be red. It will expect 'Good Morning!' to be returned but the result will be 'Good Afternoon!'. We've got that issue because we don't have control over date and time in our code. Let's fix that. What can we do to take control over date and time? We have many ways to resolve the problem:
1. [Create a fake assembly using Microsoft Fakes](https://docs.microsoft.com/en-us/visualstudio/test/isolating-code-under-test-with-microsoft-fakes?view=vs-2019)
2. [Wrap DateTime.Now with custom class and expose Now using Func<DateTime> delegate](https://ayende.com/blog/3408/dealing-with-time-in-tests)
3. Wrap DateTime.Now with an interface and inject implementation during object creation.

Let's describe shortly all three approaches. 
(1) has one only big disadvantage for me. Microsoft Fakes is available only with Visual Studio Ultimate. That was always the biggest issue for me, so I haven't used that approach in my solutions. 

However there are some open source solutions and maybe it is worth taking a look at them:
[Pose](https://github.com/tonerdo/pose), [Prig](https://github.com/urasandesu/Prig)

(2) This is the simplest way to take control over the time dependency I must say. The only one thing you need to have testable solution is creating a service class with the implementation:

```C#
public static class SystemTime
{
    public static Func<DateTime> Now = () => DateTime.Now;
}
```

And use that in your code instead of using DateTime.Now directly. For testing purposes you can set whatever fits your needs by calling:

```C#
SystemTime.Now = () => new DateTime(2020, 11, 11);
```

It's so simple, isn't it? My view on this is - that's almost perfect solution. Why almost perfect? I can imagine that a developer in production code sets some date which is not the current date not intentionally and it'll be hard to find. After such change the code is buggy and all tests are green! I don't like it but I understand that someone can prefer this solution because of simplicity.

Number (3) is a best choice for me. It requires a bit more code but it's worth doing that in my opinion. Let's introduce additional interface as that was mentioned. 

```C#
public interface ISystemTime
{
    DateTime Now { get; }
}
```

and the implementing class:

```C#
public class SystemTime : ISystemTime
{
    public DateTime Now => DateTime.Now;
}
```
We need to modify our Greeting class:

```C#
public class Greeting
{
    private ISystemTime _time;
    public Greeting(ISystemTime time)
    {
        _time = time;
    }
    public string SayHello()
    {
        var hour = _time.Now.Hour;
        if (hour < 12)
            return "Good Morning!";
        if (hour < 18)
            return "Good Afternoon!";
        return "Good Evening";
    }
}
```
As you can see it doesn't use DateTime.Now directly, we have a proxy class and keep a reference to it using an interface. It is the critical point from unit testing perspective. It gives us the ability of replacing the implementation for testing purposes.

Let's continue our work and update the test to get rid of unrepeatable behaviour. 

First step is to create our FakeSystemTime class for testing purposes:

```C#
public class FakeSystemTime : ISystemTime
{
    private DateTime _now;
    public FakeSystemTime(DateTime now)
    {
        _now = now;
    }
    public DateTime Now => _now;
}
```

And update the test:
```C#
[Fact]
public void SayHello_Test()
{
    // Arrange
    ISystemTime time = new FakeSystemTime(new DateTime(2020, 11, 11, 10, 00, 00));
    var greeting = new Greeting(time);
    // Act
    var result = greeting.SayHello();
    // Assert
    Assert.Equal("Good Morning!", result);
}
```
Now I can ensure you that this test is repeatable and it will be green even if you run it at 9:00 pm. What is more, we don't have the ability of setting custom value in our production code, that was the biggest disadvantage for me in approach (2). Happy days!

What we could do more? 
* We could add tests to cover two other scenarios (Good Afternoon, Good Evening) or use [xUnit Theory](https://andrewlock.net/creating-parameterised-tests-in-xunit-with-inlinedata-classdata-and-memberdata/#using-the-theory-attribute-to-create-parameterised-tests-with-inlinedata-) in order to parametrise the test.
* We could configure IoC containers for our production code to use SystemTime class and use FakeSystemTime for tests instead of creating these objects manually. I skipped that part for the sake of simplicity,
* We could use mocking framework like [NSubstitute](https://nsubstitute.github.io/) or [Moq](https://github.com/Moq/moq4/wiki/Quickstart) instead of creating FakeSystemTime manually.

###A summary:###
1. We shouldn't use external dependencies like date / time directly in our code. It makes testing harder and sometimes even impossible,
2. There are many ways to resolve the issue of external dependencies in the code from testing point of view,
3. Using the interface should give us more control over the external dependency, however we'll need to write some additional code.

Thank you for reading to the end! I hope that my tips will help you to create more reliable code. 

If you like my posts, please follow me on [Twitter](https://twitter.com/pnieweglowski) :)