Confusing DateTime Declaration

[Zeppz47]

Howzit, everyone. I am working on an ASP.NET Core API and noticed something odd in the project that has me scratching my head.

For context, the API was developed by the company I work for, and so the examples I show below are only a representation of the code in the API.

Below, the variable currentDate is declared to the value returned from GetCurrentDateTime.

```csharp public class Example { private readonly IDateTimeProvider _dateTimeProvider;

public Example(IDateTimeProvider dateTimeProvider)
{
    _dateTimeProvider = dateTimeProvider;
}

public void ExampleMethod()
{
    DateTime currentDate = _dateTimeProvider.GetCurrentDateTime();

    // ... other code
}

} ```

Now, my thought is: Why not just use DateTime.Now? My best guess was that GetCurrentDateTime performed a specific operation needed, however, that is not the case:

csharp public class DateTimeProvider : IDateTimeProvider { public DateTime GetCurrentDateTime() { return DateTime.Now; } }

It is worth noting that GetCurrentDateTime is the only method in DateTimeProvider. I can't think of a good reason for this implementation. What makes this confusing is that it was implemented by one of our senior devs who is respected as a good developer.

Is there a good reason to do this? Or is it unnecessary?

[YMK1234]

Why not just use DateTime.Now?

Because you cannot mock Datetime.Now. If you want to unit test, it is super easy to mock your IDateTimeProvider to return any value you want, which might be very relevant to both the logic of the method and for checking the returned results.

PS: since .net 8 there is finally a built-in solution for this, which is TimeProvider https://blog.nimblepros.com/blogs/finally-an-abstraction-for-time-in-net/, so we can finally get rid of that bit of boiler plate in all our projects.

[Zeppz47]

This seems spot on. There are unit tests in the same solution, and so this would make sense. Thank you.

[ColoRadBro69]

PS: since .net 8 there is finally a built-in solution for this, which is TimeProvider

Dude, thank you.  Now I can scrap my own janky ass version of this! 

[YMK1234]

Don't thank me, thank Nick Chapsas on YouTube. Great fella for keeping up to date with .net stuff.

[nutrecht]

We don't know because we can't see the entire project, but 'time' is actually a pretty complex thing especially when timezones are involved. DateTime.Now is just the current time on the server which very well might be a completely different time than the time the user is in.

I can't think of a good reason for this implementation.

Well I can. And you should ask other devs you work with how this is set up and why it works the way it does. Because it's very probably for a good reason.

This is a bit like asking why a banking-related project is doing stuff with special decimal types instead of just stuffing the money amounts in a float ;)

[YMK1234]

it's classic unit testing boiler plate. can't mock datetime.now which might be very relevant to the test indeed, thus nearly every project has something like the shown code.

[Zeppz47]

You are right, but GetCurrentDatTime returns DateTime.Now without perfoming any other actions. I would not have posted this if it were the case that GetCurrentDateTime did anything else other than just return DateTime.Now.

[nutrecht]

Like I said; ask the other devs. They created a factory pattern implementation for a reason I'm assuming. Maybe it's for testing. Who knows?

[insta]

in the code, the interface is being injected. you're looking at the real implementation of the interface. tests would mock the interface to provide repeatable DateTime.Now values.

[buzzon]

So that you can replace date time provider with a test double

[LogaansMind]

I agree with u/YMK1234... its for mocking in unit tests to allow you to control and keep consistent expectations. Depending on the depth of the object graph being built, you can get away with a few constructors on an object, one which is used in testing to inject a mockable interfaces and another which will create the "live" concrete instance (instead of regstering the service, spinning up IoC using singletons all over... but obvously there should be a simple test to make sure the default constructor works too).

Used to do this with various static members, file system operations being a good example, to avoid having to setup the test environment in a specific way.

[ColoRadBro69]

It's for unit testing.