Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: System.Date type #14089

Closed
paulirwin opened this issue Feb 9, 2015 · 72 comments
Closed

Proposal: System.Date type #14089

paulirwin opened this issue Feb 9, 2015 · 72 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.DateTime
Milestone

Comments

@paulirwin
Copy link

Currently, trying to use System.DateTime to represent just calendar dates is overkill and is an easy way to introduce bugs. SQL Server supports a native date-only type, and having a date-only type in .NET to match would be very handy. But not only from SQL: accepting an MVC action method parameter from an HTML5 input type="date" field would be simplified with a native Date type.

Examples:

Date d = new Date(2015, 2, 9);
Date d2 = Date.UtcToday;
Date d3 = Date.LocalToday;

// helper methods for adding/subtracting days, months, years, or TimeSpan
Date d4 = d.AddDays(7);
Date d5 = d.Subtract(TimeSpan.FromYears(2));

string s = d.ToString();
//--> "2015-02-09"

// properties:
int y = d.Year;
int m = d.Month;
int day = d.Day;
DayOfWeek weekday = d.DayOfWeek;
int doy = d.DayOfYear;

// convert from DateTime:
Date d6 = DateTime.UtcNow.ToDate();
Date d7 = (Date)DateTime.UtcNow;

// convert to DateTime:
DateTime dt = d.ToDateTime();
DateTime dt2 = (DateTime)d; // equivalent to new DateTime(d.Year, d.Month, d.Day)
@HaloFour
Copy link

Wouldn't it be more appropriate for Date.Add(TimeSpan) and Date.Subtract(TimeSpan) to return DateTime since TimeSpan contains a time portion?

Beyond little details, though, I fully agree. There needs to be a type that conveys the concept of a date without time and vice versa. I actually like how Java 8 revamped their date/time libs so that date, time and zone are individual immutable components which can be combined into composite types representing DateTime, ZonedDateTime, etc.

@mattjohnsonpint
Copy link
Contributor

The idea is sound. In fact, you'll find a similar type called LocalDate in the Noda Time library that is designed exactly for this.

There are a few general problems to consider though:

  • Date is a keyword in VB.Net, and it's already mapped to System.DateTime. Creating System.Date would confuse the heck out of VB programmers.
    • I suggest CalendarDate, as it describes exactly the purpose for the type - a date on a calendar.
  • Math with a Date and a TimeSpan wouldn't work cleanly. TimeSpan is a discrete unit of time (usually, elapsed time), with "tick" precision. Whole dates should work only with whole days or larger units. (Consider that not all calendar days are exactly 24 hours, due to the effects of daylight saving time.)
    • I suggest methods such as AddDays, AddMonths, and AddYears - but no interaction with TimeSpan.
  • One might also consider how System.Globalization.Calendar could play into this. Perhaps there is a constructor overload that takes a Calendar instance. The default would of course be GregorianCalendar.
    • This would solve a problem that DateTime has, in that it is always Gregorian internally. If you use a non-Gregorian calendar, that only works during formatting and parsing. (In other words, you can't actually represent a date on a non-Gregorian calendar with a DateTime presently)

There are other related date/time deficiencies that could be addressed at the same time - (without borrowing all of the ideas of Noda Time):

  • .NET could use a TimeOfDay type. The current TimeSpan type is not ideal, as it can track negative time and times >= 24:00 - which just don't make sense for a time-of-day. Also, TimeSpan doesn't handle 12-hour meridem (am/pm).
    • This type would be a better fit for DateTime.TimeOfDay, and would align with SQL Server's time type. Though, I'm not sure how to go about remapping those in a backwards-compatible way.
  • .NET could also use a ZonedDateTime type, which would essentially combine DateTimeOffset with TimeZoneInfo. The goal would be a fully time-zone aware data type representing an instant in time with reference to a specific time zone.
    • However, one might argue that if you have need for such a type, you probably have need for the whole Noda Time library. :)

@mattjohnsonpint
Copy link
Contributor

@HaloFour - Regarding Date.Add(TimeSpan) == DateTime - That would be making the assertion that all days start at midnight - which is not true (again, due to DST). That is another problem DateTime has presently - DateTime.Today can actually return a DateTime with Local kind, that doesn't exist in the local time zone.

@mattjohnsonpint
Copy link
Contributor

Also - It might make sense to consider extracting all date/time related stuff to it's own Nuget package. (Microsoft.Bcl.Time perhaps?) I'm not sure if TimeSpan could go, given that it's used for elapsed-time measurement in many other areas - but probably DateTime, DateTimeOffset, TimeZoneInfo, *Calendar and whatever new types come out of this discussion could all go in there.

@michaelstaib
Copy link

@mj1856 I wouldn't name it CalenderDate. I would prefer System.Date but for vb one could add an language alias that is named CalenderDate.

@michaelstaib
Copy link

@HaloFour I also think Date.Add(TimeSpan) and Date.Subtract(TimeSpan) would be more consistent than Date.AddDays(int) but if there is an need for shortcuts so that you do not have to provide all the time a TimeSpan object there should be also a SubtractDays(int).

@tarekgh
Copy link
Member

tarekgh commented Feb 10, 2015

@michaelstaib I agree CalendarDate name will be confusing as it can indicate the date is related to some calendar (Japanese, Gregorian, Hijri...).
maybe we can come up with a name which can work and not conflict with VB. something like AbsoluteDate, PureDate...etc.

@paulirwin
Copy link
Author

@mj1856 Thanks for the detailed comments! Regarding VB, I assume an "Option" flag a la "Option Strict" is off the table? :-) Alternatively, couldn't we leave VB's Date keyword to mean System.DateTime, but allow them to do Imports Date = System.Date as a type alias?

@mattjohnsonpint
Copy link
Contributor

Imports Date = System.Date won't work since Date is a language keyword. If you were to escape it, Imports [Date] = System.Date, you would also have to escape all usages, Dim dt as [Date] = [Date].Today

@tarekgh How do you feel about CalendarDate if the type actually has a Calendar reference?

using System.Globalization;

public struct CalendarDate
{
    private readonly Calendar _calendar;

    public CalendarDate(int year, int month, int day)
    {
        _calendar = new GregorianCalendar();
        // ...
    }

    public CalendarDate(int year, int month, int day, Calendar calendar)
    {
        _calendar = calendar;
        // ...
    }
//...

@tarekgh
Copy link
Member

tarekgh commented Feb 10, 2015

@mj1856 CalendarDate will make sense if we are going to support all calendars with that type (as you have demonstrated in your code). I think this is not the intend from the original request to have a simple light weight Date type. I am seeing Date and CalendarDate can be used in different scenarios

@mattjohnsonpint
Copy link
Contributor

@michaelstaib - I don't think there's anything wrong with SubtractDays and similar methods, though they are essentially the same as AddDays(-days). Having them wouldn't hurt.

But with TimeSpan, consider the case Paul gave at the top of this proposal:

Date d5 = d.Subtract(TimeSpan.FromYears(2));`

That just can't work, because TimeSpan has no FromYears method. It can't have FromYears or FromMonths because those are not discrete measurements. (Months can be 28,29,30, or 31 days, and Years can be either 365 or 366 days).

Even with TimeSpan.FromDays - that assumes a standard day of 24 hours in length. That doesn't always align with calendars, as DST transition days can be 23 or 25 hours long.

@mattjohnsonpint
Copy link
Contributor

@tarekgh - Ok. If the intention is to keep the type lightweight, meaning it's only field would be an integer representing the whole number of days since some epoch, then I understand why CalendarDate would be a potential confusion.

I actually like System.Date - I'm just not sure how the VB folks would reconcile. Perhaps there are some VB devs that are fluent with idiomatics that could weigh in here?

@paulirwin
Copy link
Author

@mj1856 @michaelstaib Good point re: TimeSpan. Further reading: reference source of DateTime.AddYears/AddMonths: http://referencesource.microsoft.com/#mscorlib/system/datetime.cs,0b982f3a57816426,references

@eatdrinksleepcode
Copy link

I would strongly suggest we not re-invent the wheel here. The NodaTime project has already done a lot of work building a solid date/time API and working through the myriad of issues that naturally arise in such an effort. It incorporates many of the ideas from JodaTime and the newer date/time JSRs, while remaining idiomaticly .NET and actually improving on many of the flawed and overly complex Joda designs.

@paulirwin
Copy link
Author

@eatdrinksleepcode I'd be happy to see NodaTime in the BCL directly, like what Java 8 did by working with the Joda-Time developer. But having a separate library as a dependency just to have a calendar date type is painful. Opinion and rationale: If DateTime is in "mscorlib" or whatever the equivalent is with Core CLR, it feels like a calendar date type should be, too. Slippery slope acknowledged.

@paulirwin
Copy link
Author

@eatdrinksleepcode Also, isn't it true that the overwhelming majority of issues that NodaTime aims to solve (and the bugs they encounter) are related to times and time zones, rather than calendar dates? Calendar dates is a relatively easy problem to solve compared to times.

@mattjohnsonpint
Copy link
Contributor

As an active contributor to Noda Time, I would hope for better integration support rather than embedding it in the BCL. For example, it would be great if we could override the default mapping of SQL's date to System.DateTime to use NodaTime.LocalDate instead, in ADO.Net and Entity Framework.

At the same time, I think there's a valid need for some minimal expansion in the BCL also. System.Date and System.TimeOfDay are prime examples. Consider that a lot of time-based business logic is based around floating calendar dates (such as the closing date of a purchase order), or around operating hours (such as the close-of-business for a nationwide restaurant chain).

@paulirwin - Noda Time does solve a lot of time zone related issues, but is also comprehensive around calendar dates. Jon Skeet is the primary author, and has done a ton of research into calendars - (even non-Gregorian calendars such as the Hebrew calendar).

@michaelstaib
Copy link

@mj1856 @paulirwin I also would prefere a simple and small calender type in the BCL which would solve the 80%. When I just want to map some date data from the database the type is just there at my fingertips. Whenever there is need for more one can opt for NotaTime ist just an install-package nodatime away ;).

@michaelstaib
Copy link

@mj1856 regarding the AddDays(-days). I know that you can subtract days from the DateTime-object by passing in a negative ints but I think the contract is much cleaner by providing a clearly named method. This makes it also more discoverable for developers that just start with .net. So, if we could give the date type methods for adding/subtracting Days/Weeks/Months/Years it would make the type very easy to use and the contract very clean.

@michaelstaib
Copy link

@paulirwin do you plan to provide the date type with similar formatting options like DateTime?

@paulirwin
Copy link
Author

@michaelstaib Yes, I would expect Date.LocalToday.ToString("MMMM d, YYYY") to work as well. The format string options would be a subset of the ones for DateTime, obviously removing the time-based ones.

@mattjohnsonpint
Copy link
Contributor

👍 Also, it should support Parse, ParseExact, TryParse, TryParseExact, and perhaps also Convert.ToDate for completeness.

I wonder if it's too late to consider remapping DateTime.Date, DateTimeOffset.Date and DateTime.Today. Likewise if System.TimeOfDay were to be added, perhaps consider remapping DateTime.TimeOfDay and DateTimeOffset.TimeOfDay. This could probably be done if there were some clever use of implicit casting operators. Though I'm not sure if that would be kosher.

@paulirwin
Copy link
Author

👍 As well as implement the same interfaces as DateTime: IComparable, IFormattable, IConvertible, ISerializable, IComparable<Date>, IEquatable<Date>

@Clockwork-Muse
Copy link
Contributor

I think I'd prefer getting a complete, integrated date/time library (NodaTime, port of JSR310, or whatever), rather than little things piecemeal. Personally, I think that's what got us into some of the current mess in the first place - people adding things as they were requested, instead of a comprehensive whole. Date/time stuff is generally more complicated than people expect, with pieces related in occasionally strange ways.

@svick
Copy link
Contributor

svick commented Feb 12, 2015

Date d2 = Date.UtcToday;
Date d3 = Date.LocalToday;

Why does Date have the concept of local or UTC date? Wouldn't it make more sense to specify that when converting to DateTime?

E.g.:

Date date = Date.Today;

DateTime dateTime = date.ToLocalDateTime();

@paulirwin
Copy link
Author

@svick - The Date object itself doesn't store the timezone, but to determine which day is "today" depends on the timezone. At 10pm Eastern Time tonight, LocalToday would be 2015-02-12 while UtcToday would be 2015-02-13. If Date.Today assumes "local" time, that leads to many of the same bugs that DateTime.Now causes.

@tarekgh
Copy link
Member

tarekgh commented Feb 12, 2015

@paulirwin DateTime already have "Now" and "UtcNow" properties. so I think there shouldn't be any confusion with this one. may be you mean Now should be called LocalNow but I never see anyone confused when using "Now". DateTime.Today can be confusing if don't know it is always returns local date. In general I think having LocalToday and UtcToday would be a good idea as it is explicit.

@paulirwin
Copy link
Author

The SqlDataReader case @mj1856 outlines above (and note that SqlDataReader is used by ORMs like Entity Framework) was precisely my original motivation for creating this issue. I am not in favor of bloating the BCL for the sole reason of making more features available, but in this case, a calendar date is such a primitive function of so many business applications, and so many bugs and hacky workarounds exist to deal with the tacked-on time in DateTime, that a calendar date in the BCL would be beneficial in many aspects. I am also in favor of making things more open for extensibility for outside libraries like NodaTime, but "just use NodaTime instead" is inefficient (in the SqlDataReader case in particular), adds an extra dependency for just a fundamental chronological type, and requires significant wiring-up of that dependency for cases like deserialization of MVC parameters, for example. Also having parity between HTML5 input types and .NET types would be beneficial given that the overwhelming use of .NET is to create web-based applications.

@mattjohnsonpint
Copy link
Contributor

HTML5 parity is a very good point.

Another point of parity mismatch is with W3C XML Schema Definition (XSD), which has xsd:dateTime, xsd:date, and xsd:time.

Currently we can only use xsd:dateTime, and only with DateTime. There are serialization problems when you try to use it with DateTimeOffset. See here and here.

@HaloFour
Copy link

HaloFour commented Mar 7, 2015

@KrzysztofCwalina

I do think that for the most part additional functionality should be filled-in by external libraries. But the CLR already provides an out-of-the-box implementation of chronology that is a fairly poor. Remaining in the BCL in their current state is not a pit of success, particularly for new developers. It also precludes the ability for other BCL assemblies from improving their use of chronology types. System.Data and System.Xml would benefit considerably from having better date/time/offset/timezone support, and those assemblies cannot take dependencies on NodaTime, and helper methods to bridge the two would be more likely to introduce bugs where they are not necessary.

@eatdrinksleepcode
Copy link

If I understand correctly, just adding an improved Date/Time API to core would not improve the SqlDataReader scenario at all; SqlDataReader would also have to be updated to understand the new API. It would be similar for any other existing functionality that currently maps to the existing Date/Time APIs; it would all have to be updated.

On Mar 6, 2015, at 10:59 AM, Paul Irwin [email protected] wrote:

The SqlDataReader case @mj1856 outlines above (and note that SqlDataReader is used by ORMs like Entity Framework) was precisely my original motivation for creating this issue. I am not in favor of bloating the BCL for the sole reason of making more features available, but in this case, a calendar date is such a primitive function of so many business applications, and so many bugs and hacky workarounds exist to deal with the tacked-on time in DateTime, that a calendar date time in the BCL would be beneficial in many aspects. I am also in favor of making things more open for extensibility for outside libraries like NodaTime, but "just use NodaTime instead" is inefficient (in the SqlDataReader case in particular), adds an extra dependency for just a fundamental chronological type, and requires significant wiring-up of that dependency for cases like deserialization of MVC parameters, for example. Also having parity between HTML5 input types and .NET types would be beneficial given that the overwhelming use of .NET is to create web-based applications.


Reply to this email directly or view it on GitHub.

@HaloFour
Copy link

HaloFour commented Mar 8, 2015

@eatdrinksleepcode

If I understand correctly, just adding an improved Date/Time API to core would not improve the SqlDataReader scenario at all; SqlDataReader would also have to be updated to understand the new API.

Correct, just adding the new types doesn't fix the situation with the other assemblies at all. What it does is open the door to updating those assemblies to support the new API officially. Since System.Data, System.Xml and the like cannot take dependencies on third-party polyfill libraries like NodaTime they cannot be improved without such underlying BCL support.

@tarekgh
Copy link
Member

tarekgh commented Mar 9, 2015

@eatdrinksleepcode can't System.Data and System.Xml changed to adapt for custom conversions? I mean to make it work with any third party library through a type conversion pattern that the library can implement and register with System.Data and System.Xml

@HaloFour
Copy link

HaloFour commented Mar 9, 2015

@tarekgh Probably, but that would be inherently more complex, and anyone new to the framework are just going to see System.DateTime and how System.Data/Xml work with that and will continue to follow poor patterns for lack of knowing better. Chronology is fundamental, I think the BCL should do it right.

@KrzysztofCwalina
Copy link
Member

This thread is getting long. I would love to agree on some plan of action. Here is a straw man I would like to propose:

  1. People who feel passionate about fixing DateTime should design an initially OOB library that is "the right" library for dates and times.
  2. In parallel, we should all work on extensibility features in XML, data APIs, etc. to help an arbitrary OOB library to integrate more nicely with these APIs. As I can guarantee you that even if the do design "the right" date and time APIs, some people will want to use noda time and others.
  3. Once "the right" library for dates and times is proven in the filed, we will consider adding it (or parts of it) to CoreFx.

@HaloFour
Copy link

HaloFour commented Mar 9, 2015

@KrzysztofCwalina

I'm curious as to how 2 would play out. In those APIs you often have specifically typed methods, e.g. DbDataReader.GetDateTime or you have methods that deal in just Object of a boxed DateTime. Adding new methods like DbDataReader.GetDate would be seemingly off limited unless/until said types were officially added to CoreFX, and I can't imagine that there would be a clean way to "register" that the boxed value be of a type other than DateTime that wouldn't have potentially sweeping ramifications. You could potentially have a separate API altogether but that seems dirty and would probably lead to more confusion.

Given that Noda seems to be the preferred date/time library (and that Java used Joda as a basis for their chronology enhancements) I wonder how it would feel to instead explore extending System.Data, System.Xml et al to support the Noda types directly as a strawman.

@paulirwin
Copy link
Author

Would it be helpful if I fork NodaTime, remove the parts that aren't in the scope of this issue, normalize the API against conventions, and present it as a reference for what a System.Date (and perhaps System.TimeOfDay) type could look like?

@tarekgh
Copy link
Member

tarekgh commented Mar 9, 2015

@paulirwin I think this will be a good start here. if you share the initial type definition we can collect some design feedback too before finalizing the implementation.

@mattjohnsonpint
Copy link
Contributor

I have a near-complete implementation of System.Date and System.Time I've been working on for awhile. I'll post them to a GH repo by tomorrow.

@mattjohnsonpint
Copy link
Contributor

I've posted the prototype to mj1856/corefx-dateandtime.

It still needs lots more unit tests, but I think I got most of the desired functionality.

It also needs to be optimized for performance. Currently it relies on DateTime and TimeSpan quite a bit internally. It could probably be better self-contained.

For the sake of not spamming this thread - please just use that repo's tracker for any issues, suggestions, pull requests, etc. until such time which it might be merged in. We should keep this thread for general comments about the idea rather than my proposed implementation.

@paulirwin
Copy link
Author

Awesome work @mj1856! I will help out with your repo rather than trying to create my own prototype. I like the direction here.

@tarekgh
Copy link
Member

tarekgh commented Mar 10, 2015

@mj1856 Thanks Matt for your effort. I took a quick look at the Date type and here is some notes and questions:

  • does Date need to implement IConvertible interface. I think it is needed
  • Date will be exactly like DateTime in term of supporting Gregorian dates only. this can be ok if this satisfy all scenarios we are introducing Date for (SQL, Xml...etc.). if there is any other scenarios need to work with other calendars we'll need to either add such support here or we'll have another type like CalendarDate or so.
  • Did you think in supporting B.C dates? just raising the thought
  • does it make it easier if you store ticks instead of daynumber? avoid converting to ticks in many places
  • ToString(string format), would be ok if some one passing format include time? not a problem but just wondering
  • In general from my experience with DateTime, Parse/TryParse is really problem and always causing problems especially when changing the culture default date/time patterns. I suggest we don't support Parse/TryParse and force only ParseExact/TryParseExact
  • renaming DateFromDateTime to FromDateTime would be better I guess
  • if we have Add/Subtract methods, then I think we need to limit the parameters to positive values. otherwise we should have Add only. I am trying to limit the confusion when using it
  • I like the implicit cast operator :-)

Thanks again Matt

@mattjohnsonpint
Copy link
Contributor

@tarekgh - I'm very glad I can do this in the open source space now. It helps to include feedback from other community members. :)

I'll address the specific points over here. Thanks!

@paulirwin
Copy link
Author

As @tarekgh mentioned on @mj1856's repo, the idea is that this issue should be closed until it is proven in the field. I did not pick up on that from earlier comments here but if that's how it should be handled then that's acceptable. Personally, I think that if there is an intention to implement this in the future, then the issue should be left open, perhaps given a future milestone or tag, because otherwise will be difficult to determine when, exactly, it is "proven" in the field. Is there a way to quantify that? Should I re-open this issue when I've deployed a single app to production successfully using @mj1856's library, or does it take X users/projects/months/stars/forks before it is proven? That's an honest question, I'm not trying to be inflammatory. I just really, sincerely care about this issue - the headaches having to shoe-horn calendar dates into DateTime has wasted numerous hours of my life - and I'd be sad if it falls by the wayside due to a "won't fix" closed status and I'd like to re-open it if it legitimately meets some criteria for being proven. If there is no objective criteria for this being proven, I'd say we should keep this issue open until it is given a definitive "fixed" or "won't fix" status.

@paulirwin
Copy link
Author

Also another reason for not closing the issue - if it is closed, someone else could come along, see no open issues for this, and create a new open issue, starting this conversation all over again.

@tarekgh
Copy link
Member

tarekgh commented Mar 13, 2015

@paulirwin this means we cannot close any issue :-) when someone open same issue again we'll direct him/her to the old issue. this is regular process for any issue. they will not have to re-discuss it again. and even they will have the freedom to reopen the old issue

@paulirwin
Copy link
Author

Fair enough, then we can close the issue if that's the consensus here. But please either follow @mj1856's library closely going forward or let me know what the rough criteria would be for the library being considered proven.

@tarekgh
Copy link
Member

tarekgh commented Mar 13, 2015

we should be in contact with @mj1856 moving forward anyway and we'll recommend this library for anyone run into to this issue. yes we should keep our eye on this library.
personally, I think this library will grow in good way as we have more scenarios related to date/time which may need similar solutions.

Thanks Paul for your thoughts and bringing such issue. and thanks of course to Matt helping with the design and implementation

@mattjohnsonpint
Copy link
Contributor

Thanks @tarekgh! Yes - I'm fine with closing the issue for now.

We'll keep iterating on mj1856/corefx-dateandtime. I'll also put together examples of corefx/coreclr changes that would be compelling use cases for should it be merged.

@tarekgh tarekgh closed this as completed Mar 15, 2015
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.DateTime
Projects
None yet
Development

No branches or pull requests