-
Notifications
You must be signed in to change notification settings - Fork 107
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
Consider deprecating DateTimePeriod.plus
and DatePeriod.plus
#381
Comments
I don't think the issue is with @Test
fun associativeAndCommutative() {
val p1 = DateTimePeriod(years = 1, months = 1, days = 28, hours = 1, minutes = 1, seconds = 1, nanoseconds = 86399999999997)
val p2 = DateTimePeriod(years = 2, months = 2, days = 30, hours = 2, minutes = 2, seconds = 2, nanoseconds = 86399999999998)
val p3 = DateTimePeriod(years = 3, months = 3, days = 31, hours = 3, minutes = 3, seconds = 3, nanoseconds = 86399999999999)
val dp1 = DatePeriod(years = 1, months = 1, days = 28)
val dp2 = DatePeriod(years = 2, months = 2, days = 30)
val dp3 = DatePeriod(years = 3, months = 3, days = 31)
assertEquals(p1 + p2, p2 + p1)
assertEquals(dp1 + dp2, dp2 + dp1)
assertEquals((p1 + p2) + p3, p1 + (p2 + p3))
assertEquals((dp1 + dp2) + dp3, dp1 + (dp2 + dp3))
} The issues arises when adding It is somewhat reminiscent of when the limitations of numeric types yield different behavior than pure math, such as the following failing tests: assertEquals(1 / 2 * 2, 1) // integer division means 1/2 rounds down to 0 instead of 0.5
assertEquals<Int>(1 + 1.0 - 1.0, 1) // will not even compile because int/double arithmetic returns a double
assertEquals(0.1 + 0.2, 0.3) // floating point arithmetic simply isn't precise Even without @Test
fun addAndSubtract() {
val ld1 = LocalDate(2000, 1, 31)
val dp1 = DatePeriod(months = 1)
assertEquals(ld1 + dp1 - dp1, ld1)
} This failure occurs entirely within the regime of adding/subtracting val ld1 = LocalDate(2000, 1, 31)
val dp1 = DatePeriod(months = 1)
val twoMonthsFromNow = (ld1 + dp1) + dp1
val twoMonthsFromNowPreserveDayOfMonth = ld1 + (dp1 + dp1) This is what we were discussing in terms of DateRange; which of the two is more intuitive and should be the default for iterating through dates. It may even be desirable to provide an optional parameter that allows advanced users to choose which behavior the progression will utilize. |
Yes, but this wasn't my point. The point was that the sum of "4 months and 23 days of waiting", and then "3 months and 23 days of waiting" isn't "7 months and 46 days of waiting", so the obvious semantics of
All of these examples use operations that make sense, they have a well-defined meaning.
Agreed here. Whether adding together two periods has a purpose is up for discussion (hence the issue).
How about this instead? val ld1 = LocalDate(2000, 1, 31)
val dp1 = DateTimeUnit.MONTH
val twoMonthsFromNow = dp1.plus(1, dp1).plus(1, dp1)
val twoMonthsFromNowPreserveDayOfMonth = ld1.plus(2, dp1) I find this much clearer: usually, parens don't matter in expressions like |
The more I go down this rabbit hole, the deeper it gets. Returning to the definition of a If we return to greenfield, restricting ourselves solely to periods of the unit I'm spitballing a little bit here rather than making a hard proposal, but what if we split The advantage of this approach is that it would allow for a sane default (days-only) in all cases, while still allowing for more complex uses for users who deliberately choose it. If you really wanted to you could even enforce this by using the |
Yes, you're right: it's not the difference, it's a difference.
The number of representations becomes finite if we limit ourselves to periods where all components have the same sign. Then, for dates, there are In practical terms, this shouldn't be a problem: usually, either you want just the number of days (so you call
That's a good way to think about
This is a valid standardized generalization of
So, yes, it's possible to define composite durations as lists of duration components, and this would make addition (and therefore, multiplication by a scalar) well defined. Here's a reason not to do that: we don't want to miss the forest for the trees. In the end, our library is supposed to help the programmers, not to force them to research how dates and times work. Technically, the We can still introduce
If I'm looking at the difference between
These building blocks are all already present: we have |
The purpose of
DatePeriod
andDateTimePeriod
is to define the duration between two dates or instants, respectively. Intuitively, it makes sense to define addition of two periods: if there's timep1
between momentsA
andB
and timep2
between momentsB
andC
, thenp1 + p2
could be the amount of time betweenA
andC
. Unfortunately, it doesn't work this way.If we don't add periods together, we get what we expected:
x + (y - x)
becomesy
, so in the end, we get back todate3
.The reason the result is different when we add periods together first lies in the order in which addition of periods happens. In the normal case, we get
In the case of adding periods together first, instead, we get
But waiting for three months and then for 23 days is not the same as waiting for 23 days and then waiting for three months, because the lengths of months are different.
With
DateTimePeriod
, the situation is even worse, because the time component is also added, giving us additional problems like the difference between waiting for 24 hours and one whole day (which isn't always the same, because clocks could shift due to DST transitions).With all of this in mind, it's unclear what use cases adding periods together correctly achieves and even what precise semantics it has. If we learn of use cases where
plus
always does what's expected, we could try to limitplus
to just these cases.The text was updated successfully, but these errors were encountered: