dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

API proposal/RFC - System.Time namespace #14744

Closed Clockwork-Muse closed 4 years ago

Clockwork-Muse commented 9 years ago

(aka "programmers for good dates")

Related: dotnet/runtime#14089 - Adding Date and Time types. dotnet/runtime#14052 - Deprecating DateTime.Now.

See also: What's wrong with DateTime anyway?

Date and Time are foundational concepts in our daily lives, and in programming; it's why we have DateTime.
Unfortunately, DateTime isn't sufficient. Date and time are part of a larger ecosystem of concepts, ranging from an absolute point in time something occurred (an Instant), to the value reported on a wristwatch (LocalTime). They're related, and you can (usually) transition or convert from one to the other, but they are very definitely not the same thing.

But wait! We have NodaTime! Well, the biggest problem with NodaTime is... it's a separate, third-party library. It means we can't take dependencies on it. This results in things like OData having a separate Date type for XML compliance (and there are other examples in different namespaces). There's a lot of stuff that NodaTime does, and does well, and most of it is stuff that the default libraries should be handling (like, say, time zone ids from non-windows sources).

What about patching some common holes with new, standard types (ie, Matt's Date and Time types)? This alleviates some problems for us, but leaves the problem when interacting with NodaTime. That is, "Why do I have to use the deficient framework types/something else to do XML serialization!?". (Note NodaTime has been looking to get the library installable on SQLCLR; I don't think it's made it into a release yet). Oh, and it means we have to keep three time zone databases in sync, instead of two (at least on windows).


So: I'm proposing a new namespace as a modern, good implementation of a date and time library, our own version of JSR310/java.time . We need a good set of narrow value types, and a range of support classes to handle conversions and many other use cases.

Unfortunately... I... don't have an API on hand: I'm creating this issue to get things started. Coming from the Java world, I really like the new java.time library, but don't know if we can (or should, really) do a straight port. It's possible we might just use NodaTime; however that library was deliberately implemented with limited extensibility, something we would need to consider (ie, no custom calendars). It would be good to ask Jon Skeet what version 8.0 would look like, if we went that route.


Known pain points:


End-of-life considerations:

First, even if we had an implementation now, corefx isn't releaseable yet. Then, there's whatever delay is involved in possibly getting such a library into the desktop release, and into wider distribution. So nothing is going anywhere any time soon.


Interested parties (?): @jasonwilliams200OK , @mj1856 , @gafter , @paulirwin , @tarekgh, @eatdrinksleepcode , @michaelstaib , @HaloFour , @ellismg

@jskeet (Jon Skeet - NodaTime lead) @jodastephen (Stephen Colebourne - JodaTime and java.time implementer - informational/possible wisdom only)

mattjohnsonpint commented 9 years ago

WRT VB's aliasing of Date, See mj1856/corefx-dateandtime#29. One can use brackets to prevent the aliasing, as in [Date], but it only works under the new compiler. The previous VS2013 compiler refuses to accept another type named [Date] living in the System namespace other than the one aliased to System.DateTime. (Not sure if that matters or not if the new API is for .NET Core only.)

If this new API lives in a System.Time namespace, that would resolve the compiler issue, but VB devs would still have to remember to use brackets to prevent the aliasing.

HaloFour commented 9 years ago

I think that instead of Date and Time that new types be called LocalDate and LocalTime. That eliminates the problems with VB's alias. This is also the naming convention used by JSR310, presumably because Date was also already taken.

I think that there are a lot of good ideas between Joda/Noda and JSR310. However, before we can really move forward fixing the time issue I think that .NET needs a better answer to the time zone issue. The current TimeZoneInfo class is better but it's still not nearly as flexible as tzdb. I think that this is fundamental to designing a proper date/time library.

Clockwork-Muse commented 9 years ago

@HaloFour - That's one of the reasons I posted the issue; to get extra stuff to look into. In truth, I was already thinking about something like that anyways, if for no other reason than to make it part of a unified namespace. Things like System.Globalization.Calendar would be merged (or otherwise ignored, and eventually removed).

Borrowing and tweaking the table you posted in an earlier issue, we need (at minimum) narrow types for the following areas (ignore names for now):

BCL Type Purpose
Date Represents only a date, ignorant of time or time zone.
Time Represents only a time, ignorant of date or time zone
(Local) DateTime Represents the combination of Date and Time, ignorant of time zone (essentially DateTimeKind.Local similar to DateTimeKind.Unspecified)
TimeZone Represents a named time zone complete with historical and transition rules ZonedDateTime
TimeOffset Represent an offset from UTC (ie, + 2:00)
OffsetDateTime Represents the combination of a Date and Time with an offset from UTC

... a big question at that point is, do we go the JodaTime/NodaTime route and have Date maintain references to a calendar, or go the Java 8 route and have distinct date types for each calendar (and they and Date implement some common interface).

Then there's the common utility types, which are slightly less important but still nice to have .

BCL Type Purpose
Instant An absolute point in time, and knows nothing of dates or calendars (and maybe time?). Usually historical (should log with this)
Period Difference between two Dates
Duration Difference between two Times
Clock Allow repeatable test results for calls to Date.Today, and similar.
jskeet commented 9 years ago

DateTimeKind.Local isn't equivalent to "ignorant of time zone" - it means "in the system default time zone". DateTimeKind.Unspecified is closer to "ignorant of time zone". This may be a good reason to avoid using the term "local" in any new API; I somewhat regret using it in Noda Time, although I don't currently have a better suggestion.

I would also suggest TimeOfDay instead of Time - partly as that enables System.Time as a namespace.

Clockwork-Muse commented 9 years ago

@jskeet - Thanks. Guess I'm too used to the Java 8/JodaTime convention. Haven't had the opportunity to do much in C# yet.
Names being taken sucks. System.Time would be a nice namespace, but there's also the System.Timers namespace, which might cause some confusion (I just picked something easy to understand).

jodastephen commented 9 years ago

For a namespace, might be worth considering System.temporal.

For classes, might be worth considering JSR-310 names as is, simply to aid developers moving between languages. (The "Local" part of LocalDate comes from ISO-8601's terminology).

Step 1 in a process like this is to understand the mistakes of the past design, examine the bugs that get raised. Step 2 is to decide on the value types. Step 3 is to figure out the level of extensibility you want and find a way to make that work. (JSR-310 is extensible in calendar system, fields, units and value types for example)

Clockwork-Muse commented 9 years ago

@jodastephen - re Step 1: any insight WRT java.time?

jodastephen commented 9 years ago

See https://github.com/ThreeTen/threeten/wiki for some captured thoughts.

paulirwin commented 9 years ago

Thanks for tagging me due to my suggestion dotnet/runtime#14089. I'm happy to see this conversation kicked up again.

My only input in looking at the Java world is to make sure that our equivalent of getMonth() isn't zero-based :-) (I kid, I kid... I know that java.time.LocalDate finally righted this wrong.)

I did not know about the LocalDate name coming from ISO-8601, thanks @jodastephen for that info. That changes my mind and makes me okay with using that name for the .NET version.

I'm sorry I don't have much more input here at the moment, I exhausted my input on dotnet/runtime#14089.

ellismg commented 9 years ago

Thanks for tagging me. I'll admit that I have not gone deep on a bunch of the improvements NodaTime brings to .NET, but I'll try to help out however I can.

One question I have to start with (which will probably lead to a bunch of follow up questions) is about this statement:

Well, the biggest problem with NodaTime is... it's a separate, third-party library. It means we can't take dependencies on it.

It's unclear to me what "we" means here. Does this mean that types in CoreFX can't take a dependency on it? Is it that other libraries outside CoreFX don't want to take a dependency on NodaTime? Something else entirely? I'm interested in understanding the differences between some new time APIs in a future version of .NET Core vs a future version of Noda Time that runs on top of the .NET Core surface area and hence can be used by any library targeting .NET Core. What does the former provide that the latter does not?

When all of the libraries in .NET Core ship on NuGet and can version independently from one another and have no ties to the runtime itself, I struggle to see the difference between something in CoreFX vs a popular "third-party" library like NodaTime, JSON.net, etc.

Clockwork-Muse commented 9 years ago

@ellismg - the 'we' here was intended to be corefx. Some in-house devs may also not be allowed to take 3rd party library dependencies, but this is rare and not an interesting reason. The former (doing a new library) would allow us to pull in tertiary types (like OData's Date type), and to allow wider use (OData's Date doesn't work directly with the globalization calendars, you have to take a trip through DateTime). It's enabling corefx to accept or return a better, more specific type than DateTime.

If the future deployment model would allow us to use NodaTime types in corefx, I could probably work/live with that.

mattjohnsonpint commented 9 years ago

Looking at the roadmap, I see System.Data.SqlClient is pending. This is a good example of an assembly that would rely on the changes we're discussing here.

Would it be reasonable for System.Data.SqlClient to depend on a .NET Core version of Noda Time? Does having a Microsoft made System.Time assembly change anything? Does it make a difference?

Or perhaps the time-portions would be broken out into System.Data.SqlClient.Time which then took the dependency on our time library while the main assembly didn't. But that could lead to a lot more splintering... How much fragmentation is too much?

What about backwards compatibility? If we are suggesting that these new types be created independently, are we also suggesting dropping the current date and time types? What about all of the existing code that folks will want to port to .NET Core? It would be quite frustrating to find that you had to make tons of changes because DateTime wasn't there any more.

Besides, there's really very little wrong with DateTimeOffset and TimeSpan. They are perfectly usable and functional. And DateTime works just fine for UTC and Unspecified kinds. It's really only the Local kind that gets folks into trouble (and just not being aware of Kind in the first place). Heck, even TimeZoneInfo isn't all that bad from an API perspective. It's really only the underlying data that is the issue there.

I'm not trying to be a naysayer, but IMHO, thrusting a whole new and different time API on folks is bound to be problematic. Likewise, doing nothing when we know there are issues also has it's consequences. I think the key to success here is balance. Add the missing pieces, while keeping the good parts intact.

Here's a detailed proposal that I think achieves everyone's goals (please correct if I'm mistaken):

Other important changes that would be introduced:

There are really only a few other things that Noda Time provides that are absent from the above. We can debate/discuss these:

There are a few trivial bits also, such as Offset and Interval, but I don't see a whole lot of value in those coming over.

After all is said and done, there would be a few new .NET Core nuget packages, which I would expect assemblies like the aforementioned System.Data.SqlClient to take a dependency on.

@Clockwork-Muse - Do you think this goes far enough? Or are you really set on the death of DateTime? :wink:

@ellismg @tarekgh - does this sound reasonable? Or is it overreaching?

Clockwork-Muse commented 9 years ago

@mj1856 - I think I'd prefer the death of DateTime through neglect (nobody uses it) in favor of something better. I don't want people yelling at us for it being gone - at least at first I'd prefer gentle encouragement to use something better (say, a StyleCop rule or something). Make the new API available but not even deprecate the old one (at least for a while). People could use the existing libraries with minimal/no changes, and update to the new API as time allowed. (All of a sudden I'm wondering if we could distribute a Roslyn syntax rewiter to do gross conversions, based on types and method calls... Tricky though). I mostly proposed a new, separate API for a cleaner break from the old, and to give any new design more potential freedom in implementation.

One of the biggest problems is dealing with DateTimeKind, and the issues it causes. I have little issue with TimeSpan or DateTimeOffset as they're (probably) implemented (although DateTimeOffset.Date returns a DateTime, which isn't really correct). (The purist in me also wants TimeSpan in here, too, but I mostly get what you're saying about the dependency.)

ZonedDateTime doesn't feel to me like it has DateTimeOffset in it. Yeah, you can get it without much trouble, but it's really Date and Time with 'location' attached. Ehhh, semantics really.

Dealing with time zone info providers does sound like an interesting idea. I'm not as up on some of the deployment/management side of things, though.

I personally (just for right now) dislike how you have to use the current calendar system, but don't have problems with it otherwise. A good Period class would help in that regard, anyways.

ashmind commented 9 years ago

Just a few points based on my .NET experience. (I'm on mobile so sorry for any formatting issues)

  1. Whatever the final API is, I suggest it should be harder to use non-timezoned date by default. I've seen so many people not being properly aware of DateTimeOffset, and it is a pain and leads to many hacks when people wake up to the reality of timezones. It would be cool if YourNewNamespace.DateTime was timezoned, and some explicit name like InvariantDateTime was used for a date that should be the same in all timezones.
  2. One thing missing in current API is timezone abbreviations (PDT, GST, etc) and corresponding parsing/formatting. Those are somewhat ambiguous, which complicates parsing/formatting API, but at least having them listed on timezone would be very useful for human-friendly date representation.
tugberkugurlu commented 9 years ago

@ashmind

Whatever the final API is, I suggest it should be harder to use non-timezoned date by default

I believe UTC is not a time zone and its use should be completely encouraged. As far as I can understand, you want the DateTimeKind.Local usage to be discouraged which is a very valid point.

tugberkugurlu commented 9 years ago

In NodaTime, I really like the way how the time zone info (e.g. DST, etc.) gets updated. AFAIK, this information changes based on the government rules. So, I should be able to update this by myself or the official update channel should be very responsive to changes and releases should be frequent.

HaloFour commented 9 years ago

IIRC TimeZoneInfo would require various other internal enhancements as well. I think it only supports the concept of a single "base offset" which doesn't reflect how potentially complex time zone rules have changed over time apart from just DST changes. Then there is TimeZone, would be nice if that class can either get folded into the mix or just go away.

I'm on the fence about System.DateTime. I'd love to find a way to include it within the rest of this family simply because it's so prevalent and because it'll likely remain the first date-related type that any new developer will find. It's not that far off from what a LocalDateTime type would be, although Kind does throw a little bit of a monkey wrench into it.

@ashmind The Microsoft time zone information doesn't contain those abbreviations at all. The IANA tzdb does for supported time zones, but they are mostly useless in parsing scenarios. You'd need a known time zone as a starting point, at which point the abbreviation can aid in resolving ambiguous times at DST boundaries.

tarekgh commented 9 years ago

I believe we need to take some time to think more about the whole space. exposing a lot more date/time classes will create more complexity. I understand the need of such types but we need to think how all of this will fit with what we have today. this is why we suggested to start with what @mj1856 creating and have the experiment to know what most needed types and how simple/complex it is. also we'll see what is wrong in such design. dotnet/runtime#14089 has the detailed discussion about this direction.

I want to second @ellismg point. we need to encourage people to use and consume the third parity packages. Json.Net is very good example of that. not every type should be part of the framework core. we need to understand why people can't take dependency on such Third parity libraries. deploying such third parity library shouldn't be different than deploying the corefx itself.

@mj1856 as we have talked offline a couple of months ago regarding different issues. and I have shared my thoughts with you. mostly what you have mentioned align with what we thought but we need to spend some more time exploring different options and design to make sure everything will fit nicely especially with types like TimeZoneInfo which is shared with the desktop framework.

mattjohnsonpint commented 9 years ago

@HaloFour - The single base-offset issue was described in KB3012229, and has already been fixed in .NET Core and .NET 4.6RC. However, there's still a related API issue open in dotnet/runtime#14457.

WRT the TimeZone class, it's already been removed.

WRT abbreviations, only English abbreviations are in TZDB, and the TZDB list has discussed several times that many were just made up by the group to fill in the gaps. Some non-English ones are also in CLDR, but in general, not every zone has an abbreviation. I've got the CLDR abbreviations and names wrapped nicely in TimeZoneNames, which I plan to do a .NET Core release in a future update. I'm not sure if it would make sense to pull this in as a core dependency - though it would solve the globalization issues with TimeZoneInfo's properties (DisplayName, StandardName, DaylightName) which are affected by machine locale rather than the .NET CurrentCulture.

paulirwin commented 9 years ago

Third party libraries are just fine for non-primitive types or primitive types of niche usage. A calendar date is a primitive type that is universally used. JsonConvert is not a primitive type and not much framework code would call into it. However, quite a bit of framework code could make use of a primitive Date type, in particular SqlClient as @mj1856 mentioned. SqlClient not being able to have a 1:1 mapping to SQL Server's date type is the primary reason why I created dotnet/runtime#14089, but also things like HTML5's input type="date" mapping to an MVC parameter. My team has spent time - this week already, in fact - resolving issues arising from DateTime including time (usually midnight) when the domain doesn't need it.

mattjohnsonpint commented 9 years ago

@paulirwin - From your perspective, can you clarify the meaning of "primitive"? There is some debate on what this term actually means.

The C# Language Specification uses this word only twice, and in an indirect way.

The VB.Net Language Specification defines it more definitively as:

7.3 Primitive Types The primitive types are identified through keywords, which are aliases for predefined types in the System namespace. A primitive type is completely indistinguishable from the type it aliases: writing the reserved word Byte is exactly the same as writing System.Byte. Primitive types are also known as intrinsic types.

Furthermore, VB lists Date as a primitive type, while C# doesn't mention DateTime in the language spec at all. Nor does it list it on the "Built-In Types Table" on MSDN.

If we apply C#'s definition to VB, then DateTime is not a primitive, and DateTimeOffset, TimeSpan, etc. are not primitives in either language.

So when we say "primitive", are we talking about a framework concept of primitive in some way? Or is there some other distinguishing factor in your mind that makes date/time related classes and value types a lower-level feature?

I can understand this lower-levelness at least with TimeSpan, given that it's used along with timers, tasks, threads, etc. But the rest of them seem like they aren't really at the lowest level to be called "primitive".

Clockwork-Muse commented 9 years ago

@tarekgh - I agree we should in general be encouraging people to use 3rd party libraries, but I'm pretty sure JSON.Net is a terrible example to use here:

  1. As near as I can tell, it's packaged with the .Net library (they took a dependency on it).
  2. There were already extension points for doing your own serialization/parsing.

In the case of date and time, there's a lot of stuff that is "stuck" returning DateTime, and can't have a more appropriate type (or custom type) returned instead.

I didn't really have any problem with @mj1856 's experiments. The problem was that the direction at the end of dotnet/runtime#14089 seemed more like, "We'll just do these few types, and tell people to use NodaTime for the rest". I firmly believe that's not a good solution long-term:

  1. It still leaves most of the pain points currently present here, for coders to stumble into (DateTime.Kind Local?)
  2. It forces anybody using NodaTime to need to switch to a completely equivalent (hopefully!) but different type for some common actions.
paulirwin commented 9 years ago

@mj1856 Sorry, I wasn't intending to use the word primitive in any kind of way as to imply something specification-related but I appreciate the digging into the meaning. What I meant by primitive was a value type that is a fundamental building block of framework code (IDataReader, System.Convert, etc), such as Guid, DateTime, TimeSpan, etc., even though these types aren't primitive in the sense that they can be literals (although perhaps they should).

LordZoltan commented 9 years ago

@mj1856 - I'm personally all for sourcing time zone info from the CLDR, it's an excellent resource and, from what I can tell, very accurate; plus it's updated regularly. Its format lends itself to generating the code via a tool.

There's also the possibility (getting ahead of us all, here, but just throwing it out there) that this could pave the way to start driving some of the culture and region information from it, too - the data is much more comprehensive than Windows, certainly.

That said, it would make the most sense to have a provider model (as mentioned earlier) for timezones et al, of which a CLDR-driven source would be just one implementation.

As wonderful as NodaTime is, I'm not entirely comfortable with the framework taking a dependency on it for something as fundamental as date and time. Its reach is pervasive, and I'd wager would very quickly make NodaTime seem to be a core 'part' of the wider framework, when in fact it isn't.

I'm very happy for a new, more flexible, Date/Time API to be introduced into the framework that supersedes the old one, but feel it should fall under the general .Net umbrella, not a third party umbrella.

I realise that that, along with much else besides, is a debatable issue, however, and that is also true for whether anything I've just said here has really added anything to the discussion!

rebizu commented 9 years ago

Hi there i am new here, As far as i can see, nobody mentioned leep second, there is one at 2015-06-30 23:59:60 Have anybody thought about how a leep second can be handled in a new implementation of datetime?

For reference here is some light readings about leep seconds https://en.wikipedia.org/wiki/Leap_second http://www.timeanddate.com/time/leapseconds.html

jskeet commented 9 years ago

For Noda Time, I made a very conscious decision not to include leap seconds. Originally I believe JSR-310 was going to support multiple timelines, but ended up deciding that clock-smearing (with a simple "60 seconds per minute, always" model) was a better fit. My contention is that few applications require understanding of leap seconds, and it's reasonable for those applications to require other libraries or custom handling rather than imposing the complexity of leap seconds on all other applications.

More interesting to me is whether there's a clean way of representing a time-of-day value of 24:00, which has far more practical applications - but still ends up causing pain in various ways.

Clockwork-Muse commented 9 years ago

@rebizu - I'm with @jskeet - we're unlikely to include leap seconds for pretty much exactly that reason. There's leap-second aware stuff for java.time in the 310-extras package (among other things), and I would recommend creating something similar for not-rare-but-not-common-enough use cases.

@jskeet - WRT to 24:00, one of the big problems being "does adding it to a date give the next day?" (when combining a date and time). It certainly would be nice for exclusive upper-bounds checking.
On the one hand I want to say that doing such a combination would be an error...("You should be using tomorrow!"), and maybe just have a special check or item. It would depend on how any new types combine date and time. Something else to consider, though, yeah.

tarekgh commented 9 years ago

I agree with @jskeet regarding the leap seconds too.

@Clockwork-Muse

but I'm pretty sure JSON.Net is a terrible example to use here I just mentioned JSON.Net as example of the third parity library that is used by many application out there and people take dependency on it. I didn't mean we should not follow the packaging mechanism of this component.

I didn't really have any problem with @mj1856 's experiments. The problem was that the direction at the end of dotnet/runtime#14089 seemed more like, "We'll just do these few types, and tell people to use NodaTime for the rest". I firmly believe that's not a good solution long-term

as discussed in dotnet/runtime#14089 Matt's library is experimental components and any type proven in the field we'll consider adding it to the core. it is matter of time. the long term direction is we'll include all needed types in the core as long as it make sense these types to be there and in same time most of apps will need such types. as I mentioned before I am really worried exposing many date/time types which will add more complexity to what we are currently have and in end up not many people will use such new types. we still can add more types to Matt's library as needed. recommending NodaTime also can help telling what apps really need from such library and the volume of the demand too. but in general we want to understand why people cannot take dependency on NodaTime? @jskeet may have more info here to understand the pain points with NodaTime

eatdrinksleepcode commented 9 years ago

At this point I feel the need to remind us all:

Standards

I am honestly very confused by the resistance to using NodaTime itself. @jskeet and @mj1856 (and others) have put a lot of effort into understanding both the (extremely complex) domain and the myriad of ways that it trips up developers every day, and designing a library to help expose these critical concepts as a "pit of success". IMHO they have been quite successful in that effort. I think we would be crazy to throw all of that away just to recreate it again. If there are problems that NodaTime doesn't solve, let's solve them. If there are design issues that make NodaTime less than ideal, let's fix them. But I have yet to see any justification for starting from scratch.

As near as I can tell, [JSON.net is] packaged with the .Net library (they took a dependency on it).

That's the whole idea: when we identify a gap in Core, and find that there is an existing library that solves the problem well, Core should take a dependency on that library rather than building it over again.

In the case of date and time, there's a lot of stuff that is "stuck" returning DateTime, and can't have a more appropriate type (or custom type) returned instead.

This is an unfortunate problem, but re-building a new date/time library in Core does not solve it. Other than taking massive breaking changes in the framework - which I am pretty sure is off the table - the best thing I know to do is ensure that our new date/time library has good interop with the existing types. But we're in luck! NodaTime already has such interop, allowing developers to convert back and forth in a reasonable way.

As a disclaimer, I should acknowledge that I have a couple of small commits in NodaTime from the relatively early days. But I don't consider myself particularly invested in NodaTime, nor is my concern whether my (minor) work in particular survives. My concern is this...

Well, the biggest problem with NodaTime is... it's a separate, third-party library. It means we can't take dependencies on it.

...the apparently still-prevalent philosophy that because a library is not published by Microsoft and doesn't have the "System.*" namespace that it can't be considered "part of the framework". This is the philosophy that has been holding back .NET open source for over a decade, but it has no place in the world of .NET Core. Needlessly re-inventing the wheel - as has already been done so often in the history of .NET - only reinforces the stereotype that .NET open source projects can't have a real impact because only "the framework" matters. We need to kill that stereotype if .NET is going to compete with all of the other popular development ecosystems whose beating heart is community-driven open source.

</soapbox>

HaloFour commented 9 years ago

@eatdrinksleepcode

While I agree with a lot of what you say I do think that if a part of the .NET framework takes a dependency on something that is not a part of the .NET framework then that something must become a part of the framework. It would be impossible for System.Data.dll to have a dependency on NodaTime without NodaTime then being distributed and maintained as a part of the whole because neither would work without the other. CoreFX being more modular might alleviate some of that issue.

I think NodaTime is good but it's also very complicated, moreso than the typical scenario requires. Chronologies throw a lot of common assumptions out the window in order to support a very small minority of use cases. Separation of chronologies is one reason that Java 8 did not just use JodaTime. Mutability is another. I don't doubt that ownership and maintainability was also a part of that conversation.

paulirwin commented 9 years ago

@eatdrinksleepcode If we weren't talking about something as fundamental to applications as a calendar date and time of day then I would agree with you entirely. To me there is an issue of governance as well: if NodaTime introduces a breaking change to their calendar date type, and SqlClient takes a dependency on NodaTime, there's little that can be done for downstream consumers until SqlClient is updated. You wouldn't be able to update NodaTime and SqlClient independently. If calendar date and time of day types are part of corefx, they would be governed the same as other framework types that use them.

There also is the separate issue of how to get started using these types. Say you wanted to have the following code:

var d = new LocalDate(2015, 6, 24);

It seems extreme to have to add a NuGet reference to NodaTime just for that functionality, when to get DateTime you just do using System;.

I think the implied argument in some of these comments of a calendar date and time of day bloating the framework is exaggerated and misdirected. Indeed, the framework shouldn't get bloated, but calendar dates and times of day are simple value types and fundamental to many, many applications. We're not talking about an API or a sub-framework or types that have external dependencies here. What we want is for dates and times to be as fundamental and ready-to-use as Int32, String, Double, Decimal, Guid, and yes, DateTime.

ryaneliseislalom commented 9 years ago

@jskeet +1 Agreed with Jon. Stay practical and tactical. Think about what the typical use-case is for the typical developer. Virtually no one relies on leap seconds, and if they're very concerned, then (great) we've got a NuGet package that solves it. Identical logic applies to other edge cases, like: 3D graphics, legacy hardware, reporting, logging, databases etc. Sure, those are all important topics, but the edge cases should not be used as leverage to (as @jskeet put it) 'impose the complexity' on all developers.

It's definitely an important topic, but frankly, if a dev's CoreFX code doesn't look and behave like all their other .NET code, there's a much more limiting argument for adopting it. If developers want a radically new framework, there are a million out there to choose from, and CoreFX gets lost in the crowd. If the developers want consistent behavior with 16 years of learning behind all major decisions, CoreFX can win without exception. My sense tells me that they want the latter.

Clockwork-Muse commented 9 years ago

@eatdrinksleepcode - FWIW, I personally don't have a problem taking a dependency on NodaTime (although I've identified at least one area where we'd likely need changes). I started this issue to say "we need to include the ability to do stuff similar to NodaTime". If we end up taking the dependency, okay. If the fallout is we want to go with significant design differences, necessitating a different API, then we should learn (steal?) from NodaTime.

The xkcd is actually one reason I felt I had to raise this issue now; I felt it was a possible result of @mj1856 getting his code in a release, then ending up with a richer date/time library down the road. (mj: I'm not trying to dis your code, it's just that you'd be constrained by the current limitations DateTime and other supporting classes impose, design and implementation wise).

mattjohnsonpint commented 9 years ago

Just want to say that I love the feedback so far - soapboxes and all! Keep it coming!

WRT leap seconds, I'll dump some info here.

I agree with leaving them out of .NET, for several reasons.

  1. Leap seconds are part of the domain of clock synchronization. They're important for operating systems and NTP servers, not application frameworks.
  2. Windows doesn't do anything with leap seconds outside of w32tm, of which it generally ignores them. It would be difficult to support leap seconds in the framework if they're not supported in the underlying OS. (I'm not sure 100% about other OS's handling, but I don't think it matters much anyway.)
  3. It's extremely rare to need to parse a leap second. They are allowed by ISO-8601, but in practice, I've never encountered a parser implementation that supports them.
  4. Application contexts that need leap seconds are mostly limited to the already specialized domains of astronomy, astrology, and global positioning (GPS clocks need a leap-second table to get an accurate UTC time). Other than those, there's not a whole lot of actual purpose for them (IMHO).
  5. Most applications have to contend with the fact that computer clocks are imprecise instruments anyway. They can be fiddle with by the user, they drift, and they are "corrected" via NTP sync and other OS-level processes. Applications that need "perfect" timing would have to consider using specialized hardware anyway.

For more about leap seconds on Windows, and why this will be a non-event, see M3's blog post on MSDN.

ghost commented 9 years ago

My vote is to revamp System.Time although it will be a HUGE breaking change, but if there is a discount for ONE breaking change in CoreFX, this should be it.

If it gets elected, should we start a [WIP] PR already; collecting commits from all the interested entities? :sunglasses:

tarekgh commented 9 years ago

I think we cannot get rid of the types we are supporting today. the current plan according to dotnet/runtime#14089 is to introduce the needed types in Matt's library and let's get more info from the people using it and the volume of the usages and then we'll consider these types to the core. this apply to any date/time type we think it is important to be in corefx. I hope we can move forward with this direction.

mattjohnsonpint commented 9 years ago

Thus far, I'm leaning in the same direction as @tarekgh, but let's examine some of the specific issues. Here are two common operations that I think highlight the need for some new features.

Getting a specific time of day on a particular date

Let's say you want a value which is 5:00 AM today. Currently, one would do something like this:

DateTime result = DateTime.Today.AddHours(5);

This seems reasonable, until you consider that adding 5 hours of elapsed time to midnight doesn't necessarily land you at 5:00. On a DST transition day, it could land you at 4:00 or 6:00. Combining a date and time should be more specific with regard to intent.

Noda Time breaks this problem down to the finest grain of detail:

DateTimeZone tz = DateTimeZoneProviders.Tzdb.GetSystemDefault();
Instant instant = SystemClock.Instance.Now; 
ZonedDateTime now = instant.InZone(tz);
LocalDate today = now.Date;
LocalDateTime result = today.At(new LocalTime(5, 0, 0));

As you can see, just getting "today" is complicated. NodaTime 2.0 is working to improve that part. The last line is pretty reasonable, and that's what I'm going for in mj1856/corefx-dateandtime:

DateTime result = Date.Today.At(new TimeOfDay(5, 0, 0));

Adding time in a timezone-aware manner

Similar to above, but this time the intent is actually adding a duration of elapsed time, rather than picking a specific time.

DateTime dt = new DateTime(2015, 3, 8, 1, 0, 0, DateTimeKind.Local);
DateTime result = dt.AddHours(2);

The result is 3:00, but actually it should be 4:00 because in my local time zone (US Pacific Time), the hour from 2:00 to 3:00 is skipped due to the daylight saving time transition.

Again, Noda Time breaks it down to the finest details:

DateTimeZone tz = DateTimeZoneProviders.Tzdb.GetSystemDefault();
LocalDateTime ldt = new LocalDateTime(2015, 3, 8, 1, 0, 0);
ZonedDateTime zdt = ldt.InZoneLeniently(tz);
ZonedDateTime result = zdt + Duration.FromHours(2);
LocalDateTime localResult = result.LocalDateTime;

Sure, we have DateTimeOffset, but that doesn't necessarily help here. The resulting value would be correct in instantaneous time, but would have the wrong local time and offset. It would have to be further adjusted to compensate.

I think most developers would prefer something more obvious, like this:

DateTime dt = new DateTime(2015, 3, 8, 1, 0, 0);
DateTime result = dt.AddHours(2, TimeZoneInfo.Local);

This simple overload of AddHours that takes a TimeZoneInfo object is not as obvious to implement as one might think. Properly taking into account DateTimeKind and converting through offset changes, an extension method implementation looks like this:

public static DateTime AddHours(this DateTime dt, double hours, TimeZoneInfo tz)
{
    DateTimeOffset dto = dt.Kind == DateTimeKind.Unspecified
        ? new DateTimeOffset(dt, tz.GetUtcOffset(dt))
        : new DateTimeOffset(dt);

    DateTimeOffset result = dto.AddHours(hours);
    DateTimeOffset adjusted = TimeZoneInfo.ConvertTime(result, tz);
    return adjusted.DateTime;
}

I'm planning on adding this and others like it to mj1856/corefx-dateandtime shortly...

mattjohnsonpint commented 9 years ago

Actually, thinking through that last one, I had an additional thought. Perhaps what would be most intuitive for developers would be

DateTime dt = new DateTime(2015, 3, 8, 1, 0, 0, TimeZoneInfo.Local);
DateTime result = dt.AddHours(2);

This would be very similar to how things are done in Python. ("naive" vs. "aware" datetimes).

I'd have to work out precisely how to make this work in .NET without breaking existing stuff, but it's something to think about. I'd appreciate any discussion on this concept.

eatdrinksleepcode commented 9 years ago

@HaloFour

It would be impossible for System.Data.dll to have a dependency on NodaTime without NodaTime then being distributed and maintained as a part of the whole because neither would work without the other. CoreFX being more modular might alleviate some of that issue.

Why would that be impossible? Do you mean that it would be impossible for Desktop? That may be, but then I also don't see why shipping NodaTime on Desktop would be a problem. CoreFX is distributed via NuGet, and I see no reason why the System.Data package couldn't take a dependency on the NodaTime package.

I think NodaTime is good but it's also very complicated, moreso than the typical scenario requires.

This has been mentioned a couple of times. What particular complexity are you referring to? For extremely simple scenarios, where time zones and such are not relevant, the Local* types are very simple to use and understand. For anything beyond those extremely simple scenarios, there is inherent complexity that comes into play, and IMHO NodaTime does a good job of exposing that inherent complexity, but no more than necessary. Where do you feel that NodaTime has more complexity than is necessary, that couldn't be addressed in the library itself?

eatdrinksleepcode commented 9 years ago

@paulirwin

if NodaTime introduces a breaking change to their calendar date type, and SqlClient takes a dependency on NodaTime, there's little that can be done for downstream consumers until SqlClient is updated. You wouldn't be able to update NodaTime and SqlClient independently. If calendar date and time of day types are part of corefx, they would be governed the same as other framework types that use them.

Sure you could update them independently, up until the next major version, where the break would occur. This is no different than any other library dependency, and would be no different if we created a System.Time library instead.

It seems extreme to have to add a NuGet reference to NodaTime just for that functionality, when to get DateTime you just do using System;.

Why is that extreme? In the modular .NET Core, everything is a package reference. So unless it is pre-added as part of your project template, you have to add it to get it. And there is no reason NodaTime couldn't be added to the relevant project templates if that makes sense.

calendar dates and times of day are simple value types...What we want is for dates and times to be as fundamental and ready-to-use as Int32, String, Double, Decimal, Guid, and yes, DateTime.

No, this is exactly what we don't want! If all you need is date and time information devoid of any relationship to the real world, then the existing types in the BCL should be good enough. But if what you need is actual date and time information that relates to the real world, then you do not want a simple value type. In fact, treating the extremely complex domain of dates and times as they are represented in the modern (digital and analog) world as if they were simply value types is exactly what got us into our current mess in the first place. Date and time are not primitive values like Int32 and Decimal; even if they are as ubiquitous in application code (which I would dispute), they simply do not have the same trivial semantics, and we should not fall into the trap (again!) of treating them as if they do.

Clockwork-Muse commented 9 years ago

@tarekgh - my biggest fear is, if we went that route, we'd be locking ourselves into using types that we really want to replace or introduce breaking changes to later. For instance, subtracting two DateTimes returns a TimeSpan, or the elapsed ticks between them; however, this is rarely the desired representation, even if the two values were absolute. In the run up to Java.time, @jodastephen looked back at JodaTime and decided it looked at time too much from a computer perspective, and not enough from a human perspective; I think that's happening there, too. If we subtract two Date types, we get back a relative value (year, month, day); returning ticks is like returning the difference in number of days. (Of course, returning just the date-difference value in a Date&Time type would also be incorrect, you'd need either a combined type or two different method calls)

@mj1856 - the biggest problem I have with DateTime is the current DateTimeKind: because sometimes you just don't know what your values represent. Adding the time zone like that implies DateTimeKind changes to have None (local) and Specified (absolute), or goes away or something (and that OffsetDateTime is superseded). It's making me a bit twitchy about not having separate types, but I think I could live with it, if we could throw errors in reasonable places.

HaloFour commented 9 years ago

@eatdrinksleepcode

You're right that CoreFX does open up possibilities of Microsoft-shipped assemblies that have dependencies on third-party dependencies. My concerns there are mostly regarding fragmentation between CoreFX and the full frameworks.

Where do you feel that NodaTime has more complexity than is necessary, that couldn't be addressed in the library itself?

Separation of time zones from chronologies. Many users will need time zones, significantly fewer will need alternate calendars. This is one of the major areas that JSR-310 explicitly differed from JodaTime.

Sure you could update them independently, up until the next major version, where the break would occur. This is no different than any other library dependency, and would be no different if we created a System.Time library instead.

Except that this doesn't happen* with the .NET framework. If System.Time was a part of the BCL then it would be managed and maintained in tandem and go through the rigorous standards of backwards compatibility as demanded by Microsoft.

* Exceedingly rare to the point of insignificant.

No, this is exactly what we don't want! If all you need is date and time information devoid of any relationship to the real world, then the existing types in the BCL should be good enough.

Even in the simplest cases they are far from sufficient, if only because DateTime compounds date and time, which frequently is not what a developer needs or wants. The hoops through which a developer must jump to manually untangle that in the most basic of cases leads to lots of problems. And while it is quite complicated they are also incredibly fundamental.

mattjohnsonpint commented 9 years ago

The more I think about it, the more I like the idea of enhancing DateTime to be optionally timezone-aware. I can envision a design that would keep all existing functionality intact and stay within the 16-byte recommended limit of a struct size. It would add 8-bytes for a pointer to a TimeZoneInfo instance, and repurpose the two bits that Kind currently uses. One bit to control naivety, and one bit to disambiguate values in a fall-back transition.

The question is, is it too much of a breaking change for DateTime to double in size of memory footprint? Or would this have to be a new type (eg, DateTime2 or ZonedDateTime)?

I'd prefer keeping the name the same, if possible - as it address the concern of adding too many new types.

This would effectively "undo" the mess of DateTimeKind without necessarily removing it.

ashmind commented 9 years ago

@mj1856

The more I think about it, the more I like the idea of enhancing DateTime to be optionally timezone-aware.

What do you mean by 'optionally'? As I mentioned above, I would be strongly for making it timezone aware (and dropping DateTimeOffset altogether).

mattjohnsonpint commented 9 years ago

@ashmind - by optionally, I mean that there would continue to be DateTime values with Unspecified kind and null time zone. There are many real-world cases where it is required to not lock a value to a specific time zone or offset.

This wouldn't necessarily drop DateTimeOffset, but rather it would shift the emphasis back to DateTime. One could still use DateTimeOffset, but one could also model a DateTimeOffset as just a DateTime with a fixed-offset time zone. I can see having a method like TimeZoneInfo.WithFixedOffset(TimeSpan) and/or some constructors for DateTime that take a TimeSpan offset as a parameter.

Another reason I like this is that despite the guidance to prefer DateTimeOffset, there's still a natural tendency to just reach for DateTime. Rather than fighting against it, this change would adapt the data type to fit.

There are a couple of interesting questions this model brings up:

  1. Type mapping. For example, a time-zone-aware DateTime map could map to a SQL datetime, in which case the time zone would be stripped away so it wouldn't round-trip. Or, it could be mapped to a datetimeoffset, in which case the offset in effect would be saved, but it would round-trip back as a fixed time zone - not necessarily as the original time zone.
  2. Compatibility of behavior for the local time zone. For example, one would expect that DateTime.Now would return an "aware" DateTime having DateTimeKind.Local and TimeZoneInfo.Local. That would however be different than the current "naive" value that just has DateTimeKind.Local. Consider the AddHours example I gave above. One would get a different result across a DST transition with the new framework than with the old one. This might be acceptable though. It depends on whether or not you consider it "introducing a breaking change", or "fixing a bug". (semantics...)
ashmind commented 9 years ago

@mj1856

By optionally, I mean that there would continue to be DateTime values with Unspecified kind and null time zone. There are many real-world cases where it is required to not lock a value to a specific time zone or offset.

I suggest using a separate type for that, as it is semantically different. E.g. when defining any web APIs I definitely wouldn't want to receive a time in an unspecified time zone. It would be only applicable for some specific use cases and those should be explicit.

And in the common development process, I don't see much issue with always having a timezone -- e.g. if only one local timezone is needed (desktop app?) all DateTimes will be in that timezone anyway.

jskeet commented 9 years ago

@HaloFour

Where do you feel that NodaTime has more complexity than is necessary, that couldn't be addressed in the library itself?

Separation of time zones from chronologies. Many users will need time zones, significantly fewer will need alternate calendars. This is one of the major areas that JSR-310 explicitly differed from JodaTime.

Yes, and it was removed from Noda Time over a year before 1.0 shipped. It's important to understand that despite the name, Noda Time is different from Joda Time in quite a few ways - now more than ever. (I made the API significantly different before 1.0, and since then I've been changing a lot of the underlying machinery. The underlying code for 2.0 is almost unrecognizable from Joda Time's...)

HaloFour commented 9 years ago

@jskeet That's certainly my mistake, then. :frowning:

I have been spending an unfortunate amount of time in Java land lately having to deal with a project spanning java.util.Date, JodaTime and JSR-310.

jodastephen commented 9 years ago

FWIW I'm not that aware of the subtleties of the existing API. I can talk about what a language-level library should provide - the key concepts:

These five are the base model that are needed for business applications. They need to be different types (not a "kind" flag) as developers need to be confident that each method/field holds the data types they expect it to. While Instant and ZonedDateTime overlap in meaning, they are distinct. Why? Well, when recording a simple timestamp, you typically do not care or even have a relevant time-zone. It is far better to just store the instant.

In usage terms, my experience is that the most used types are LocalDate and Instant, then LocalTime and ZonedDateTime with LocalDateTime least used of the five. While it may be tempting to omit one of the five, doing so would be a big mistake, as it would cause users to use a less appropriate type for their needs. ie. if the user really needs to store a local date+time, they should not be forced to use a zoned date+time simply because the library doesn't have it. (And optional elements are equally nasty at this level).

JSR-310 has other types - Year, YearMonth, MonthDay, Month, DayOfWeek, OffsetTime and OffsetDateTime which all have their place, but are a step down in importance.

Because there are a number of different types, conversion between them is important. JSR-310 provides atXxx and toXxx methods to achieve this:

LocalDateTime ldt = date.atTime(time);
ZonedDateTime zdt = date.atStartOfDay(zone)

To complement the date/time value types, amount values are also needed - Period and Duration concepts. Period captures an amount in years, months and days. Duration captures an amount in seconds (and fractions of second). This separation in JSR-310 (as opposed to the precise approach in Joda-Time) makes the classes simpler and more effective. Note that Period should be aware of the calendar system used - a month is not the same in different calendar systems.

On time-zones, the TZDB data should be used with support from CLDR for localization. While TZDB is not perfect, a single source for TZ data would be beneficial for computing in general.

On the data types, JSR-310 has three:

Separating the ZoneId and ZoneOffset classes into a small hierarchy models the concept well. If a method can only handle offsets (eg. most databases) then it is useful to be able to distinguish.

JSR-310's approach to leap seconds is pragmatic and recommended.

Bear in mind that other calendar systems add a lot of complexity for a rarely needed use case. Keep the main classes solidly in the de facto world civil calendar (ISO-8601).

Finally, look at JSR-310, not Joda-Time. JSR-310 essentially fixes things in Joda-Time.

jskeet commented 9 years ago

I agree with @jodastephen's summary except that I think OffsetDateTime should be included in the "primary" types. Aside from anything else, it's more common in my experience to pass around a date/time with a specified UTC offset than it is to pass around a date/time with an actual time zone ID. (As an example, ISO-8601 has no representation with a time zone ID, but does have a representation with an offset.) While that can be faked using a ZonedDateTime with a fixed-offset time zone, I think that's misrepresenting the information provided.

OffsetDateTime was a late entry into Noda Time 1.0 - just weeks before the release, IIRC - but when I'd introduced it, suddenly a lot of ugliness went away. I would be loathe to introduce a new date/time API which didn't include it as a type. (It matches .NET's DateTimeOffset, just as another point of reference. It's the one "extra" type that the BCL maintainers did think was worth introducing!)