-
Notifications
You must be signed in to change notification settings - Fork 153
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
Inconsistency in Duration days in duration.total() vs instant.add(). #2768
Comments
The reasoning was that if a developer wants to add days to a In the Duration case, there's no other type to push users to use instead. This was one of the compromises we made when we decided to offer a single duration type, rather than do what Java and some others do where there's a "date duration" and a "time duration". Merging all units into the same duration type had some advantages (e.g. simpler for users) but also some disadvantages, like the inconsistency you found. With only a single Duration type, it's a valid (and very common) use case to create durations that contain only dates, e.g. Therefore, we decided to make Hopefully this explains the compromise solution we chose. |
(As an aside, while I understand Duration was a deliberate decision, IMO it was a poor one. Java/Joda/js-joda absolutely got this right by (1) distinguishing between Period and Duration and (2) not worrying about balancing. I predict Duration to be extremely troublesome forever.) The oddity is that add/subtract/total needs a reference point for weeks, months, years, (because their lengths vary) but not for days (even though its length likewise varies). For consistency, a reference point should be required for days as well. (Although...I would argue that add/subtract don't need to balance. So they shouldn't need a reference, while total and round do need a reference.) |
I've gotten some feedback for Jiff (a Rust datetime library heavily inspired by Temporal) that has essentially echoed this point, including an example of a footgun: they parsed some ISO 8601 durations from some data file, wanted to convert them to seconds and assumed that duration rounding would return an error if there was any ambiguity about what the units meant. But since days are implicitly treated as 24 hours even when there is no relative datetime, it looks like it might have wound up doing the wrong thing in this case. (I don't have the full use case mapped out yet.) I'm considering what the impact would be of, in Jiff at least, always requiring a relative date when durations contain non-zero units of days for operations like |
(Also, meta note, please let me know if my use of Temporal's issue tracker to help get clarity on design points as a way to influence Jiff's design is something that is unwanted.) |
Using the issue tracker for this is fine! I think you already summarized it really well in the issue thread that you linked:
If I understand the issue correctly, it's about the case when you don't pass a I think it'd be justified if a different library made a different choice for a different environment. You could, for example, make opting into this case more explicit by requiring some sort of marker, like the Rust equivalent of (Having only one Duration type is one of the design decisions I'm most conflicted about in hindsight, FWIW; but given the constraints on Temporal's size, we certainly wouldn't add a second one now even if the proposal was still at a stage where that'd be possible.) |
The "marker" idea is interesting! |
duration.total():
instant.add():
This is inconsistent. Why should instant.add() recognize day length variability, but not duration.total()?
The text was updated successfully, but these errors were encountered: