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

Duration doesn't support years, months #1426

Closed
DartBot opened this issue Jan 31, 2012 · 6 comments

Comments

@DartBot
Copy link

@DartBot DartBot commented Jan 31, 2012

This issue was originally filed by martinp...@google.com


Dart's built in Duration does not support years or months. That's particularly nasty as adding months to a date is hard (add 30, 31, 28, or 29 days to get to the next month?).

@dgrove

This comment has been minimized.

Copy link
Member

@dgrove dgrove commented Feb 1, 2012

cc @floitschG.
Added Area-Library, Triaged labels.

@whesse

This comment has been minimized.

Copy link
Member

@whesse whesse commented Feb 1, 2012

This would be hard to support without making the Duration class more complicated. It would have to have separate fields for the amount of time in seconds, which would be counted as days when it was very large, and for the number of months. This seems like an easy class to write, and it would work best with a CalendarDate class that did not include a time or time zone.

So I don't think Date and Duration from corelib should be changed in this way, and that a CalendarDate and CalendarDuration class should be used, and perhaps offered in a publicly available library (but not core?).

@DartBot

This comment has been minimized.

Copy link
Author

@DartBot DartBot commented Feb 1, 2012

This comment was originally written by martinp...@google.com


Right, I was mistakenly thinking Duration would keep track of the difference in separate fields, not sum them all up. So Duration is basically a wrapper around a millisecond value that makes it a bit easier to display a time difference, but not necessarily a way to do date calculations (that typically need things like "5 days later", regardless of DST etc).

I think we do need something like a CalendarDuration. I wonder if there's a good reason for Duration to exist if we have a CalendarDuration; it seems like a Duration in this sense is more or less just a CalendarDuration that operates with only milliseconds.

We could have different constructors for these:

interface CalendarDuration {
  // This sums up all the components into milliseconds.
  CalendarDuration.millisecondDuration(days, hours, minutes, ...);
  // This creates the separate component style duration.
  CalendarDuration(days, hours, ...);

  // returns the days component.
  final int days;
  // calculates the (rounded down) total days this duration spans.
  final int totalDays;
}

Does that make sense?

@DartBot

This comment has been minimized.

Copy link
Author

@DartBot DartBot commented May 4, 2012

This comment was originally written by @seaneagan


It would only require adding fields for days and months, since weeks can be determined from days, and years from months. The in{Milliseconds,Seconds,Minutes,Hours} methods would still be exact when only a subset of {milliseconds,seconds,minutes,hours} are passed to the constructor, and similarly for in{Days,Weeks}/{days,weeks}, and in{Months,Years}/{months,years}. In other scenarios, the in{...} fields all serve as approximations. When adding and subtracting Durations, the Date implementation would only care about the milliseconds, days, and months fields.

Here's how I would do it:

https://gist.github.com/2595325

@floitschG

This comment has been minimized.

Copy link
Contributor

@floitschG floitschG commented May 4, 2012

The current Duration class is really just a very simple wrapper around an integer that represents the duration in milliseconds.
If we want something more complicated (either by extending the Duration class, or by creating a new class) we need to consider the following use-cases:
2012-03-02 to 2013-03-03: The difference here should yield 1 year, and 1 day. Independent of any possible leap-year, or even summer-time change. This is interesting because 2011-03-02 to 2012-03-03 should still yield 1 year and 1 day, even though the number of days are different. Given that some countries change their summer-savings behavior from time to time the number of hours are usually not consistent either.
2012-02-28 to 2012-03-01 must yield 2 days (obvious), which shows that this class would need to know about leap years.

Similar thoughts about time differences: assuming that there is a summer-savings time change at 2h, then 1h55 to 2h05 should not just yield 10 minutes, but 1h10.

To wrap it up: I currently believe (but i haven't put too much thought into it) that the more complicated class would need to contain at least one Date instance, which would allow it to do correct leap-year and summer-savings time computations.

@floitschG

This comment has been minimized.

Copy link
Contributor

@floitschG floitschG commented Dec 18, 2012

Closing as won't fix. A duration with days, months and years is useful but won't be in core. It is an obvious candidate for a calendar library, though.


Added WontFix label.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.