# Simple DateTime abstraction

Some of my tests require that I need to test the date time results (like time stamps etc.). In order to be able to test the date time string I created a simple DateTime abstraction that I'm going to use in other projects later. It should replace direct calls to DateTime.(Utc)Now. Or should I rather call it TestValue instead of ConstValue?

public abstract class DateTimeProvider
{
public abstract DateTime Value { get; }

public DateTime? ConstValue { get; set; }

public static implicit operator DateTime(DateTimeProvider dateTimeProvider)
{
return dateTimeProvider.Value;
}
}

public class NowDateTimeProvider : DateTimeProvider
{
public override DateTime Value
{
get { return ConstValue ?? DateTime.Now; }
}
}

public class UtcNowDateTimeProvider : DateTimeProvider
{
public override DateTime Value
{
get { return ConstValue ?? DateTime.UtcNow; }
}
}


The idea of abstracting DateTime.Now and DateTime.UtcNow for testability is definitely a good one. There are some improvements I'd suggest:

## Naming

FooProvider is a name I usually pick when I absolutely can't think of a better name for something that provides Foo. In this case, a much more natural name than DateTimeProvider would be Clock.

I also don't really like the name ConstValue. For one thing it's an unnecessary shortening of ConstantValue. But probably Override would be more descriptive.

## Implicit Operator

I tend to lean towards being pretty permissive with the use of the implicit operator. It's often a good way to be able to write more expressive code without cluttering things up with constructor calls.

But in this case I think the benefit is very marginal. The difference in readability between calling e.g. AddTimestamp(clock) and AddTimeStamp(clock.Value) is very minor, and the latter is actually more expressive of that fact that you're stamping it with the time at the point that the AddTimeStamp method was called (rather than at some arbitrary point during its procedure).

## Design

Splitting Now and UtcNow into different classes seems weird, for a couple of reasons:

• It implies that you need both in your system, but never in the same place. If you did need them in the same place, you'd need to handle two different clocks, which would be cumbersome.

• Similarly, it's quite possible you'll have a class which should make its own decision about whether to use Now or UtcNow. It may be a natural part of a class's responsibility to make that decision, not something that should be chosen in bootstrapping. In that case, you'd need to give the class some kind of DateTimeProviderProvider so it could get the right one!

So with that in mind I'd suggest an intermediary step of changing to:

public abstract class Clock
{
public abstract DateTime Now { get; }

public DateTime? NowOverride { get; set; }

public abstract DateTime UtcNow { get; }

public DateTime? UtcNowOverride { get; set; }
}


## Override logic

Admittedly, the above actually looks worse than what we started with, but there's one more problem to tackle: the use of override-type logic. The issues with this:

• It's hard to test. Your NowDateTimeProvider exists so tests don't have to deal with the real current date time. But that class itself can't be tested without dealing with the real time. Although it's a simple class, it still has logic, which means not being able to test it is a problem.

• Your production code has to deal with testing concerns. Every clock you write can have the time overridden, even though there's no scenario in real code where this should be allowed. It's unsafe, and even if you're careful, it degrades the expressiveness of your code to have "Don't call me!" methods.

• It's more code. Instead of a simple Now, you need a Now, a NowOverride, and logic to glue them together.

Fortunately, none of these are hard to fix:

public abstract class Clock
{
public abstract DateTime Now { get; }

public abstract DateTime UtcNow { get; }
}

public class RealClock : Clock
{
public override DateTime Now { get { return DateTime.Now; } }

public override DateTime UtcNow { get { return DateTime.UtcNow; } }
}

public class TestClock : Clock
{
public override DateTime Now { get; set; }

public override DateTime UtcNow { get; set; }
}


## And finally...

From that last version, you can see that once we've made these changes, there's no need to have an abstract class anymore. We're left with just signatures, so we can change to an interface. This also means we can consider not writing TestClock at all and using a mocking library instead.

• As an addendum, you might also consider using DateTimeOffset instead of DateTime, which would get rid of the need to handle Now and UtcNow separately in your abstraction – Ben Aaronson Jan 21 '16 at 10:59
• You could also use NodaTime which includes an IClock interface and can make dealing with TimeZones much much easier (+1 though). – RobH Jan 21 '16 at 11:01
• I have just had another idea but because you've posted your review at the same time I rolled it back... It addressed one of the issues you mention namely the ConstValue. The new code implemented it via a separate class that I called ConstDateTimeProvider. However I have still another idea, perhaps I just pass a Func<DateTime> to the constructor and have only one class that fulfills all requirements. – t3chb0t Jan 21 '16 at 11:03
• @RobH I've heard about it but I really dislike the naming convention it uses and I find it totally over engineered ;-) – t3chb0t Jan 21 '16 at 11:04
• @t3chb0t Heh yes, I saw that change and rollback after I posted. Hopefully the rest is still helpful! And yes, Func<DateTime> serves the same purpose, though still has the issue of not letting the class itself control whether it gets Now or UtcNow. Func<DateTimeOffset> wouldn't have that problem. – Ben Aaronson Jan 21 '16 at 11:06
1. I'd use a simple delegate instead of an interface:

public delegate DateTime GetUtcNow();


You can then implement it with () => DateTime.UtcNow or () => SomeConstant.

2. Converting to a different time zone is a separate responsibility from getting the time, so I wouldn't solve it by injecting different clock services. Instead you should pass in a TimeZoneInfo and convert the utc time to that timezone before printing it.

3. You should probably go one step further and first create an instance of a LogEntry class that contains an UTC timestamp and other information (message, severity,...) and then convert it to a string using some kind of LogFormatter which handles time zone conversion and different string formats for date times.

• @t3chb0t I don't like using a property for a value that differs every time you access it. I'd rather use a method for that. – CodesInChaos Jan 21 '16 at 13:48
• It's hard to use a delegate with dependency injection. Interfaces are much easier to use. – t3chb0t Dec 3 '17 at 14:08

Since this question has a lot of views I'll post my current implementation.

I changed the interface to have only a single method Now:

public interface IDateTime
{
DateTime Now();
}


which is implemented by a couple of other classes that return different date and times. So basically there is a Local and Utc date-time:

public class LocalDateTime : IDateTime
{
public DateTime Now() => DateTime.Now;
}

public class UtcDateTime : IDateTime
{
public DateTime Now() => DateTime.UtcNow;
}


And for testing purposes I use a MockDateTime which allows me to specify any date-time I want. E.g. it could be a fixed timestamp or some other predictable generator.

public class MockDateTime : IDateTime
{

private int _counter;

public MockDateTime(IEnumerable<DateTime> nows)
{
if (nows is null)
{
throw new ArgumentNullException(nameof(nows));
}

_nows = nows.ToList().GetEnumerator();
}

public DateTime Now()
{
_counter++;
return
_nows.MoveNext()
? _nows.Current
: throw new InvalidOperationException(
$"There {(_counter - 1 == 1 ? "was" : "were")} " +$"only {_counter - 1} {(_counter == 1 ? "timestamp" : "timestamps")} " +
\$"but more were requested.");
}
}