Runtime: Deprecate DateTime.Now

[ellismg]

(The original issue was at dotnet/coreclr#45), but I've moved it over here as the meat of the issue is about framework design.

From @mj1856

I would like to propose that DateTime.Now be deprecated. This is most developer's first introduction to working with time, and is also a root cause of bugs related to daylight saving time and time zones.

Developers should get in the habit of thinking globally, rather than locally - especially when designing for the web. Even on desktop and mobile applications that only run in a single time zone, this can create the kind of bugs that pull developers out of bed at 2AM on the morning of a daylight saving time transition.

More supporting arguments:

• The Case Against DateTime.Now - Matt Johnson (me)
• What's Wrong with DateTime Anyway - Jon Skeet

Given the widespread nature of this API, I suggest _not_ removing it from coreclr - but rather marking it with an [Obsolete] attribute. I can send a PR if the suggestion is approved.

[jakesays-old]

I'm sorry but I sincerely believe this is a HORRIBLE idea. I read your "Case Against" argument - you're making all sorts of assumptions that are just not valid (or perhaps more accurately push your somewhat myopic view on time.)

While I agree there are many instances where capturing the timezone would be appropriate, it certainly isn't necessary in all cases.

Your argument that local time is without context is just false. There are many systems that operate purely within a single timezone, and the 'local' context is adequate.

I work in an industry that really pays no attention whatsoever to timezones, and we interact with data shipped from around the country. We deal with dozens of standards that do not even have room for timezone data, so having to manage it would be a complete waste of time.

Not to mention the fact that I'll now have to deal with hundreds of obsolete warnings.

[ronnyek]

Two people's ideas of what datetimes should be, automatically mean this is a bug/issue? I would strongly disagree with this change. The reality is that while I think Utc time is probably more valid/relevant date time value/format, I think the reasoning for this change is trivial. As jake points out.. .there are many .net systems out there with no concept/context of timezones or needing to run on servers.

I'd much rather have things stay the way they are... my .02c

[mj1856]

I'm not arguing for everyone using UTC in all cases. I'm well aware of the use cases of using local time, and that many people work within a single time zone.

Let me restate the problem another way. Imagine an application that's developed offshore, as many applications are. Perhaps it was developed in a location that does not use daylight saving time, so the developers may not even be familiar with the concept. (As an example, consider UTC+05:30 that is used in India.) The application records some transactions in the local time zone on a fairly regular basis. It gets the current time using DateTime.Now, and stores it in a datetime field in a SQL Server.

Everything works well, and the application is delivered to a client in the US, or Europe, or elsewhere in the world. As an example, let's say it's installed on a server that is set to US Eastern Standard Time.

It continues to work well, until the date of the fall-back daylight saving time transition. When the clock was approaching 2:00 AM, it jumped backward to 1:00 AM and repeated the hour. How does the application behave? Well, it depends on what is done with that data. Say, for example, there is part of the application that returns records sorted by time. For this hour, it will return twice the usual amount of data, and the values from the _first_ one-o'clock hour will be interleaved with the _second_ one-o'clock hour. What happens from there is highly dependent on the application. Perhaps just totals are off on some report. But perhaps it triggers a false alarm on some critical tripwire.

This is just one example where ignoring the behavior of the local time zone can have unintended consequences. There doesn't need to be any concept of time zone in the application, nor does it need to have users in multiple time zones in order to encounter these types of problems. The fact is, _local time_ is an artifact of the local time zone. In fact, we can see in the implementation of DateTime.Now that TimeZoneInfo is used with the local time zone.

In the example given, and in many of the other examples, the problem is mitigated by using DateTimeOffset.Now to get the local time _including the offset_ - and persisting the entire DateTimeOffset object.

The only cases where you would not have these types of problems is if one of the following is true:

• The application will never run in a time zone with daylight saving time
• The application never runs in the middle of the night (near the transition times)
• The application has low activity during the transition time
• The data isn't recorded or isn't used to filter, sort, or group

In the case where you really want the current local DateTime, and one of the above is true, then you could use DateTimeOffset.Now.DateTime to be explicit that you don't care about the offset.

And yes - I agree that this is opinionated. However, I believe it's an opinion that is well informed and would help guide developers to making the right decisions. I bring it here as a _proposal_ for discussion for the betterment of the framework - not as a bug.

[jakesays-old]

So you're suggesting the framework be optimized for offshore development? You're not, obviously, but that sure is an absurd example.

You're also dismissing 10+ years of code written around DateTime.Now. You're going to have developers maintaining code bases and arbitrarily changing Now to UtcNow to fix build warnings. Now we have a whole new raft of bugs to fix, thank you very much.

[SamuelEnglard]

The problem is less the idea of using local time and more the fact that a developer can easily use DateTime.Now and not realize the issues. I think though that is should be more of a documentation issue than an API one. Just like string concatenation vs. StringBuilder you allow the developer to chose which is the best for their situation.

I vote for clearer documentation.

[Suchiman]

So you suppose i should always have to write DateTime.UtcNow.ToLocalTime() or DateTimeOffset.Now.DateTime everytime i need to present my user on a local operating desktop system the current time? Developers developing enterprise server applications accessed around the world should be aware of the concept of Utc.

Marking DateTime.Now as [Obsolete] will trigger a build warning with the implications of people having warnings as errors to fail the build which would be undesired and also not accurate since the concept of a local time will never fade away, the world will always have local times and requiring everyone to add explicit code to access local time would be contradictory to the basic principle of .NET to be local aware first (e.g.: string comparisons).

[mikedn]

This proposal is anything except reasonable. Yes, ideally UTC should be used but obsoleting DateTime.Now won't do that, the only thing this would achieve is making old code produce a bunch of warnings that needs to be fixed, at the cost of introducing bugs, or silenced. Or ignored, I've seen plenty of warnings ignored just like I've seen plenty of bad uses of local time.

This should be handled like other globalization issues, with code analysis tools. For example FxCop has CA1303 for string literals, something like that could probably be added for DateTime.Now.

[Clockwork-Muse]

Personally, I'd prefer a new date/time API in general (whether we ask John Skeet for NodaTime, or go with something closer to Java's new JSR310, or some third option, I don't much care - although I think NodaTime would likely need to have extensible calendars).

This is more about providing things scoped for least-use, and clearing up ambiguity.
Have a "business date", that is used globally (ie, "all sales for the 25th") - that's a LocalDate, with only year/month/day values (and note that they usually aren't strictly aligned to other calendars, either).
Have a log? Everything should be an instant in UTC, and then formatted when necessary.

...At least this isn't Java (%shudder%...that original date/time API)...

[aggsol]

What would be the alternative then? Calls like DateTime.LocalNow and DateTime.UtcNow?

[Clockwork-Muse]

You end up with different types, depending on what you're doing, usually. So stuff like LocalDate.Today (no time portion, so "now" isn't quite right), or Instant.Now (which incorporates no notion of timezones, or even year/month/day divisions - it's an offset from a specific earlier instant).

This doesn't prevent somebody from being an idiot while misusing a type, but it does allow conscientious people to be more specific and clear about what they plan to allow.

[nydehi]

it would be nice if there was a rating thumbs up or down to get the general sense of the community. oblivating aol me too message like this.

I dont think this is a good idea.

But as an aside @aggsol DateTime.LocalNow and DateTime.UtcNow are great additions but as extension methods.. there is opensource .net library for date time that is fairly popular. that would the thing to reccomend

[mj1856]

@Suchiman - No, I'm proposing you would write DateTimeOffset.Now for that. Converting back to a DateTime defeats the purpose.

[mj1856]

@Clockwork-Muse NodaTime is great. But I don't think introducing new types into the clr is going to stop people from misusing the existing ones.

[mj1856]

@mikedn - yes - it would and should produce a warning. Using DateTime.Now is very easy - and is almost always in error. (Especially in an ASP.net app)

I like your idea though that perhaps this should be a Code Analysis warning instead of a compiler warning. That feels like a better balance.

[jakesays-old]

@mj1856 Can you justify your assertion that DateTime.Now is almost always in error with empirical evidence? Because honestly I have rarely come across the issues you raise, and when I have the fixes have been trivial. You're making this out to be a huge issue, and I'm just not seeing it. @SamuelEnglard pretty much hit the nail on the head - Let the developer make the decision. Contrary to popular belief we know what we're doing.

[mj1856]

@jakesays - Thank you for the valuable feedback. I appreciate your viewpoint. I will work on gathering empirical evidence, if that will help the argument. I'm not saying this is a _huge_ issue - if it was, we likely would have seen offset as part of the original DateTime type instead of introduced later. But it is indeed an issue that can be avoided with proper guidance.

I think a good compromise is @mikedn's idea of making it a code analysis warning instead of marking it with [Obsolete]. The message would be something similar to:

CA####: Using a DateTime to capture time in the local time zone can lead to globalization errors because the time zone offset is not persisted in the data. Instead, use DateTimeOffset.Now to capture the current local time. When working with UTC, use DateTime.UtcNow or DateTimeOffset.UtcNow.

I'd want to use a similar message for DateTime.Today, and perhaps there are other CA warnings that should be considered around date/time issues.

@ellismg - Is there any open-source effort for Code Analysis?

[Suchiman]

@mj1856 for this https://github.com/DotNetAnalyzers/StyleCopAnalyzers might be a good starting point

[aggsol]

@jakesay When ever you store DateTime.Now in a database in some other format than ticks you loos the DateTime.Kind value. The DateTime is then local or UTC only by convention. This is especially bad when machines from different timezones store local timestamps.

[MartinJohns]

@aggsol He is well aware of this. DateTime.Now can be bad for some situations (like the one you mentioned), but the point is that it's not _always_ bad. There are legitimate uses for it.

Let the developer make the decision. Contrary to popular belief we know what we're doing.

[mj1856]

@ellismg - I think the feedback here is valid. This would be a better fit for code analysis than for the framework. Feel free to close if you agree.

[ellismg]

Makes sense.

[liddellj]

I have witnessed countless bugs over the years caused by developers using DateTime.Now where they _should_ have used DateTime.UtcNow. I would therefore love to see DateTime.Now marked as obsolete, and DateTime.LocalNow introduced in it's place. A simple, non-breaking change that increases clarity and reduces the chance of bugs.

Any chance of re-opening this bug on that basis?

[Suchiman]

@liddellj As already mentioned:

Marking DateTime.Now as [Obsolete] will trigger a build warning with the implications of people having warnings as errors to fail the build which would be undesired

Also it's pretty much default that .NET APIs are by default locale aware unless specified otherwise.

The overall settlement of this issue was to opt-in for this in a less intrusive way by using roslyn analyzers which could optionally auto-fix all occurrences.

[mj1856]

Also, changing the name of the property _would_ be a breaking change, and wouldn't necessarily solve the problem.

[liddellj]

@Suchiman I'm aware about the warnings as errors issue discussed above, but I maintain that in this scenario the benefits outweigh the costs - since the fix for such a warning would be trivial (replace usages of DateTime.Now with DateTime.LocalNow. In fact - addressing such warnings may actually result in undiscovered bugs being fixed by forcing developers to consider whether they actually wanted to use local time in the first place.

I also don't think this change has any bearing on whether the DateTime API is locale aware or not. It's simply a case of being clearer about what the properties represent.

@mj1856 I'm not proposing that the name of the property should be _changed_, I'm proposing that DateTime.Now is marked as obsolete, and a new property is introduced called DateTime.LocalNow with the exact same value. For example:

    [Obsolete("Use DateTime.LocalNow instead.")]
    public static DateTime Now
    {
        ...
    }

    public static DateTime LocalNow
    {
        ...
    }

You are correct that it may not solve the problem, but it certainly increases the clarity of what the value actually represents, and would force the developer to consider whether local time is actually what they mean.

I'm based in the UK, where we recently entered BST (British Summer Time). In the past 2 days I have witnessed 2 separate projects encounter issues directly caused by using DateTime.Now where DateTime.UtcNow should have been used. These are generally easy to fix and you could argue that they could have been avoided in the first place with better coding or test coverage, but the reality is that such issues will continue to crop up - and I've seen plenty of them in my time as a .NET developer. I fail to see how this change could do anything but improve matters, even considering the warnings as errors problem.

[mj1856]

@liddellj - Yep. I hear you loud and clear. However, I raised the issue and the community responded quite clearly that this change would have too far reaching of an impact without a whole lot of direct benefit. Say we did as you proposed. All it would do is create a lot of warnings, and many people would either ignore them or just change the name without considering the impact.

I'm still advocating for the best practice to be: Never use DateTime.Now in a web application. (Or any of the other methods that assume the local time zone). I am investigating where to fit this into code analysis and other guidance. But it's not going to be obsoleted in the framework.

[ryanbnl]

The language + framework should be designed to support the entire set of programs which will be built upon them; they should not be optimized for a specific (but important) subset.

Your goal is to avoid an extremely common error that is made in globalized "information" systems. This is a very common use case on the .Net platform, so it's clear that there should be some way improve the handling for this situation.

Luckily the new .Net compiler offers just that: you can create a custom Analyzer to identify these issues in your project at compile time. You can catch the bug without having to change anything in the .Net framework. Why wait 1-2 years for a framework-level fix when you have the power to do that now?

This is something that we, the community, should pick up and deliver.

[aggsol]

@ryanbnl The framework is the root and needs to be fixed regardless how long it takes of changes to emerge. I find arguing that way risky because you are arguing for "custom Analyzer" bandaid that can only be used by the knowing,

[ronnyek]

Strongly disagree... besides "needing to be fixed" and what constitutes the fix are totally subjective... To me this still feels an awful lot like someone latched on to one teeny tiny little concept, and is suggesting big breaking changes to get that one little thing working how THEY want it.

Frankly, I like the idea of analyzer over just changing objects, and deprecating (or just removing them all together).

Its worked, and worked better than Dates on any other platform I've dealt with dates, and we've got 10yrs 14'ish years of information on the net that talks about using datetime way x, or way y.

In any case the issue is closed, lets hope this stays that way.

[DaveVdE]

The whole DateTime type needs to be made obsolete IMHO.