Skip to content

Commit 5947748

Browse files
dkhalanskyjbdkhalanskyjb
committed
Change DateTimePeriod to be normalized
Now, it only has three components: months, days, and nanoseconds, and all the other properties are just representations of these ones. This way, for each DateTimePeriod there exists a well-defined ISO-8601 representation, and `toString()` behaves correctly. Fixes #79
1 parent 978ae2f commit 5947748

File tree

9 files changed

+134
-85
lines changed

9 files changed

+134
-85
lines changed

‎core/common/src/DateTimePeriod.kt

Lines changed: 75 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,44 @@
55

66
package kotlinx.datetime
77

8+
import kotlin.math.*
89
import kotlin.time.Duration
910
import kotlin.time.ExperimentalTime
1011

1112

1213
// TODO: could be error-prone without explicitly named params
1314
sealed class DateTimePeriod {
14-
abstract val years: Int
15-
abstract val months: Int
15+
internal abstract val totalMonths: Int
1616
abstract val days: Int
17-
abstract val hours: Int
18-
abstract val minutes: Int
19-
abstract val seconds: Long
20-
abstract val nanoseconds: Long
17+
internal abstract val totalNanoseconds: Long
2118

22-
private fun allNotPositive() =
23-
years <= 0 && months <= 0 && days <= 0 && hours <= 0 && minutes <= 0 && seconds <= 0 && nanoseconds <= 0 &&
24-
(years or months or days or hours or minutes != 0 || seconds or nanoseconds != 0L)
19+
val years: Int get() = totalMonths / 12
20+
val months: Int get() = totalMonths % 12
21+
open val hours: Int get() = (totalNanoseconds / 3_600_000_000_000).toInt()
22+
open val minutes: Int get() = ((totalNanoseconds % 3_600_000_000_000) / 60_000_000_000).toInt()
23+
open val seconds: Int get() = ((totalNanoseconds % 60_000_000_000) / NANOS_PER_ONE).toInt()
24+
open val nanoseconds: Int get() = (totalNanoseconds % NANOS_PER_ONE).toInt()
25+
26+
private fun allNonpositive() =
27+
totalMonths <= 0 && days <= 0 && totalNanoseconds <= 0 && (totalMonths or days != 0 || totalNanoseconds != 0L)
2528

2629
override fun toString(): String = buildString {
27-
val sign = if (allNotPositive()) { append('-'); -1 } else 1
30+
val sign = if (allNonpositive()) { append('-'); -1 } else 1
2831
append('P')
2932
if (years != 0) append(years * sign).append('Y')
3033
if (months != 0) append(months * sign).append('M')
3134
if (days != 0) append(days * sign).append('D')
3235
var t = "T"
3336
if (hours != 0) append(t).append(hours * sign).append('H').also { t = "" }
3437
if (minutes != 0) append(t).append(minutes * sign).append('M').also { t = "" }
35-
if (seconds != 0L || nanoseconds != 0L) {
36-
append(t).append(seconds * sign)
37-
if (nanoseconds != 0L) append('.').append((nanoseconds * sign).toString().padStart(9, '0'))
38+
if (seconds or nanoseconds != 0) {
39+
append(t)
40+
append(when {
41+
seconds != 0 -> seconds * sign
42+
nanoseconds * sign < 0 -> "-0"
43+
else -> "0"
44+
})
45+
if (nanoseconds != 0) append('.').append((nanoseconds.absoluteValue).toString().padStart(9, '0'))
3846
append('S')
3947
}
4048

@@ -45,83 +53,95 @@ sealed class DateTimePeriod {
4553
if (this === other) return true
4654
if (other !is DateTimePeriod) return false
4755

48-
if (years != other.years) return false
49-
if (months != other.months) return false
56+
if (totalMonths != other.totalMonths) return false
5057
if (days != other.days) return false
51-
if (hours != other.hours) return false
52-
if (minutes != other.minutes) return false
53-
if (seconds != other.seconds) return false
54-
if (nanoseconds != other.nanoseconds) return false
58+
if (totalNanoseconds != other.totalNanoseconds) return false
5559

5660
return true
5761
}
5862

5963
override fun hashCode(): Int {
60-
var result = years
61-
result = 31 * result + months
64+
var result = totalMonths
6265
result = 31 * result + days
63-
result = 31 * result + hours
64-
result = 31 * result + minutes
65-
result = 31 * result + seconds.hashCode()
66-
result = 31 * result + nanoseconds.hashCode()
66+
result = 31 * result + totalNanoseconds.hashCode()
6767
return result
6868
}
6969

7070
// TODO: parsing from iso string
7171
}
7272

73-
class DatePeriod(
74-
override val years: Int = 0,
75-
override val months: Int = 0,
76-
override val days: Int = 0
73+
class DatePeriod internal constructor(
74+
internal override val totalMonths: Int,
75+
override val days: Int = 0,
7776
) : DateTimePeriod() {
77+
constructor(years: Int = 0, months: Int = 0, days: Int = 0): this(totalMonths(years, months), days)
78+
// avoiding excessive computations
7879
override val hours: Int get() = 0
7980
override val minutes: Int get() = 0
80-
override val seconds: Long get() = 0
81-
override val nanoseconds: Long get() = 0
81+
override val seconds: Int get() = 0
82+
override val nanoseconds: Int get() = 0
83+
override val totalNanoseconds: Long get() = 0
8284
}
8385

8486
private class DateTimePeriodImpl(
85-
override val years: Int = 0,
86-
override val months: Int = 0,
87-
override val days: Int = 0,
88-
override val hours: Int = 0,
89-
override val minutes: Int = 0,
90-
override val seconds: Long = 0,
91-
override val nanoseconds: Long = 0
87+
internal override val totalMonths: Int = 0,
88+
override val days: Int = 0,
89+
override val totalNanoseconds: Long = 0,
9290
) : DateTimePeriod()
9391

92+
// TODO: these calculations fit in a JS Number. Possible to do an expect/actual here.
93+
private fun totalMonths(years: Int, months: Int): Int =
94+
when (val totalMonths = years.toLong() * 12 + months.toLong()) {
95+
in Int.MIN_VALUE..Int.MAX_VALUE -> totalMonths.toInt()
96+
else -> throw IllegalArgumentException("The total number of months in $years years and $months months overflows an Int")
97+
}
98+
99+
private fun totalNanoseconds(hours: Int, minutes: Int, seconds: Int, nanoseconds: Long): Long {
100+
val totalMinutes: Long = hours.toLong() * 60 + minutes
101+
// absolute value at most 61 * Int.MAX_VALUE
102+
val totalMinutesAsSeconds: Long = totalMinutes * 60
103+
// absolute value at most 61 * 60 * Int.MAX_VALUE < 64 * 64 * 2^31 = 2^43
104+
val minutesAndNanosecondsAsSeconds: Long = totalMinutesAsSeconds + nanoseconds / NANOS_PER_ONE
105+
// absolute value at most 2^43 + 2^63 / 10^9 < 2^43 + 2^34 < 2^44
106+
val totalSeconds = minutesAndNanosecondsAsSeconds + seconds
107+
// absolute value at most 2^44 + 2^31 < 2^45
108+
return try {
109+
multiplyAndAdd(totalSeconds, 1_000_000_000, nanoseconds % NANOS_PER_ONE)
110+
} catch (e: ArithmeticException) {
111+
throw IllegalArgumentException("The total number of nanoseconds in $hours hours, $minutes minutes, $seconds seconds, and $nanoseconds nanoseconds overflows a Long")
112+
}
113+
}
114+
115+
internal fun buildDateTimePeriod(totalMonths: Int = 0, days: Int = 0, totalNanoseconds: Long): DateTimePeriod =
116+
if (totalNanoseconds != 0L)
117+
DateTimePeriodImpl(totalMonths, days, totalNanoseconds)
118+
else
119+
DatePeriod(totalMonths, days)
120+
94121
fun DateTimePeriod(
95122
years: Int = 0,
96123
months: Int = 0,
97124
days: Int = 0,
98125
hours: Int = 0,
99126
minutes: Int = 0,
100-
seconds: Long = 0,
127+
seconds: Int = 0,
101128
nanoseconds: Long = 0
102-
): DateTimePeriod = if (hours or minutes != 0 || seconds or nanoseconds != 0L)
103-
DateTimePeriodImpl(years, months, days, hours, minutes, seconds, nanoseconds)
104-
else
105-
DatePeriod(years, months, days)
129+
): DateTimePeriod = buildDateTimePeriod(totalMonths(years, months), days,
130+
totalNanoseconds(hours, minutes, seconds, nanoseconds))
106131

107132
@OptIn(ExperimentalTime::class)
108133
fun Duration.toDateTimePeriod(): DateTimePeriod = toComponents { hours, minutes, seconds, nanoseconds ->
109-
DateTimePeriod(hours = hours, minutes = minutes, seconds = seconds.toLong(), nanoseconds = nanoseconds.toLong())
134+
buildDateTimePeriod(totalNanoseconds = totalNanoseconds(hours, minutes, seconds, nanoseconds.toLong()))
110135
}
111136

112-
operator fun DateTimePeriod.plus(other: DateTimePeriod): DateTimePeriod = DateTimePeriod(
113-
safeAdd(this.years, other.years),
114-
safeAdd(this.months, other.months),
115-
safeAdd(this.days, other.days),
116-
safeAdd(this.hours, other.hours),
117-
safeAdd(this.minutes, other.minutes),
118-
safeAdd(this.seconds, other.seconds),
119-
safeAdd(this.nanoseconds, other.nanoseconds)
137+
operator fun DateTimePeriod.plus(other: DateTimePeriod): DateTimePeriod = buildDateTimePeriod(
138+
safeAdd(totalMonths, other.totalMonths),
139+
safeAdd(days, other.days),
140+
safeAdd(totalNanoseconds, other.totalNanoseconds),
120141
)
121142

122143
operator fun DatePeriod.plus(other: DatePeriod): DatePeriod = DatePeriod(
123-
safeAdd(this.years, other.years),
124-
safeAdd(this.months, other.months),
125-
safeAdd(this.days, other.days)
144+
safeAdd(totalMonths, other.totalMonths),
145+
safeAdd(days, other.days),
126146
)
127147

‎core/common/src/Instant.kt

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -192,19 +192,15 @@ public fun Instant.minus(period: DateTimePeriod, timeZone: TimeZone): Instant =
192192
/* An overflow can happen for any component, but we are only worried about nanoseconds, as having an overflow in
193193
any other component means that `plus` will throw due to the minimum value of the numeric type overflowing the
194194
platform-specific limits. */
195-
if (period.nanoseconds != Long.MIN_VALUE) {
196-
val negatedPeriod = with(period) {
197-
DateTimePeriod(-years, -months, -days, -hours, -minutes, -seconds, -nanoseconds)
198-
}
195+
if (period.totalNanoseconds != Long.MIN_VALUE) {
196+
val negatedPeriod = with(period) { buildDateTimePeriod(-totalMonths, -days, -totalNanoseconds) }
199197
plus(negatedPeriod, timeZone)
200198
} else {
201-
val negatedPeriod = with(period) {
202-
DateTimePeriod(-years, -months, -days, -hours, -minutes, -seconds, -(nanoseconds+1))
203-
}
199+
val negatedPeriod = with(period) { buildDateTimePeriod(-totalMonths, -days, -(totalNanoseconds+1)) }
204200
plus(negatedPeriod, timeZone).plus(DateTimeUnit.NANOSECOND)
205201
}
206202

207-
/**
203+
/**
208204
* Returns a [DateTimePeriod] representing the difference between `this` and [other] instants.
209205
*
210206
* The components of [DateTimePeriod] are calculated so that adding it to `this` instant results in the [other] instant.

‎core/common/src/math.kt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,22 @@ internal fun multiplyAddAndDivide(d: Long, n: Long, r: Long, m: Long): Long {
161161
val (rd, rr) = multiplyAndDivide(md, n, m)
162162
return safeAdd(rd, safeAdd(mr / m, safeAdd(mr % m, rr) / m))
163163
}
164+
165+
/**
166+
* Calculates [d] * [n] + [r], where [n] > 0 and |[r]| <= [n].
167+
*
168+
* @throws ArithmeticException if the result overflows a long
169+
*/
170+
internal fun multiplyAndAdd(d: Long, n: Long, r: Long): Long {
171+
var md = d
172+
var mr = r
173+
// make sure [md] and [mr] have the same sign
174+
if (d > 0 && r < 0) {
175+
md--
176+
mr += n
177+
} else if (d < 0 && r > 0) {
178+
md++
179+
mr -= n
180+
}
181+
return safeAdd(safeMultiply(md, n), mr)
182+
}

‎core/common/test/DateTimePeriodTest.kt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class DateTimePeriodTest {
1717
assertEquals("P1Y", DateTimePeriod(years = 1).toString())
1818
assertEquals("P1Y1M", DatePeriod(years = 1, months = 1).toString())
1919
assertEquals("P11M", DateTimePeriod(months = 11).toString())
20-
assertEquals("P14M", DateTimePeriod(months = 14).toString()) // TODO: normalize or not
20+
assertEquals("P1Y2M", DateTimePeriod(months = 14).toString()) // TODO: normalize or not
2121
assertEquals("P10M5D", DateTimePeriod(months = 10, days = 5).toString())
2222
assertEquals("P1Y40D", DateTimePeriod(years = 1, days = 40).toString())
2323

@@ -29,8 +29,15 @@ class DateTimePeriodTest {
2929
assertEquals("-P1DT1H", DateTimePeriod(days = -1, hours = -1).toString())
3030
assertEquals("-P1M", DateTimePeriod(months = -1).toString())
3131

32-
assertEquals("P-1Y-2M-3DT-4H-5M0.500000000S",
32+
assertEquals("-P1Y2M3DT4H4M59.500000000S",
3333
DateTimePeriod(years = -1, months = -2, days = -3, hours = -4, minutes = -5, seconds = 0, nanoseconds = 500_000_000).toString())
34+
35+
assertEquals("PT277H46M39.999999999S", DateTimePeriod(nanoseconds = 999_999_999_999_999L).toString())
36+
assertEquals("PT0.999999999S", DateTimePeriod(seconds = 1, nanoseconds = -1L).toString())
37+
assertEquals("-PT0.000000001S", DateTimePeriod(nanoseconds = -1L).toString())
38+
assertEquals("P1DT-0.000000001S", DateTimePeriod(days = 1, nanoseconds = -1L).toString())
39+
assertEquals("-PT0.999999999S", DateTimePeriod(seconds = -1, nanoseconds = 1L).toString())
40+
assertEquals("P1DT-0.999999999S", DateTimePeriod(days = 1, seconds = -1, nanoseconds = 1L).toString())
3441
}
3542

3643
@Test

‎core/common/test/InstantTest.kt

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -468,9 +468,11 @@ class InstantRangeTest {
468468
fun periodArithmeticOutOfRange() {
469469
// Instant.plus(DateTimePeriod(), TimeZone)
470470
// Arithmetic overflow
471-
for (instant in smallInstants + largeNegativeInstants + largePositiveInstants) {
472-
assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(seconds = Long.MAX_VALUE), UTC) }
473-
assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(seconds = Long.MIN_VALUE), UTC) }
471+
for (instant in largePositiveInstants) {
472+
assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(nanoseconds = Long.MAX_VALUE), UTC) }
473+
}
474+
for (instant in largeNegativeInstants) {
475+
assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(nanoseconds = Long.MIN_VALUE), UTC) }
474476
}
475477
// Arithmetic overflow in an Int
476478
for (instant in smallInstants + listOf(maxValidInstant)) {
@@ -494,9 +496,8 @@ class InstantRangeTest {
494496
assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(nanoseconds = 1), UTC) }
495497
assertArithmeticFails { minValidInstant.plus(DateTimePeriod(nanoseconds = -1), UTC) }
496498
// Overflowing a LocalDateTime in intermediate computations
497-
assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(seconds = 1, nanoseconds = -1_000_000_001), UTC) }
498-
assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(hours = 1, minutes = -61), UTC) }
499-
assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(days = 1, hours = -48), UTC) }
499+
assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(days = 1, nanoseconds = -1_000_000_001), UTC) }
500+
assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(months = 1, days = -48), UTC) }
500501
}
501502

502503
@Test
@@ -540,9 +541,9 @@ class InstantRangeTest {
540541
@Test
541542
fun periodUntilOutOfRange() {
542543
// Instant.periodUntil
543-
maxValidInstant.periodUntil(minValidInstant, UTC)
544-
assertArithmeticFails { (maxValidInstant + 1.nanoseconds).periodUntil(minValidInstant, UTC) }
545-
assertArithmeticFails { maxValidInstant.periodUntil(minValidInstant - 1.nanoseconds, UTC) }
544+
maxValidInstant.periodUntil(maxValidInstant, UTC)
545+
assertArithmeticFails { (maxValidInstant + 1.nanoseconds).periodUntil(maxValidInstant, UTC) }
546+
assertArithmeticFails { minValidInstant.periodUntil(minValidInstant - 1.nanoseconds, UTC) }
546547
}
547548

548549
@Test

‎core/js/src/Instant.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst
116116
.run { if (days != 0) plusDays(days) as ZonedDateTime else this }
117117
.run { if (hours != 0) plusHours(hours) else this }
118118
.run { if (minutes != 0) plusMinutes(minutes) else this }
119-
.run { if (seconds != 0L) plusSeconds(seconds.toDouble()) else this }
120-
.run { if (nanoseconds != 0L) plusNanos(nanoseconds.toDouble()) else this }
119+
.run { if (seconds != 0) plusSeconds(seconds) else this }
120+
.run { if (nanoseconds != 0) plusNanos(nanoseconds.toDouble()) else this }
121121
}.toInstant().let(::Instant)
122122
} catch (e: Throwable) {
123123
if (e.isJodaDateTimeException()) throw DateTimeArithmeticException(e)
@@ -191,7 +191,7 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT
191191
val time = thisZdt.until(otherZdt, ChronoUnit.NANOS).toDouble().nanoseconds
192192

193193
time.toComponents { hours, minutes, seconds, nanoseconds ->
194-
return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds.toLong(), nanoseconds.toLong())
194+
return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds, nanoseconds.toLong())
195195
}
196196
} catch (e: Throwable) {
197197
if (e.isJodaDateTimeException()) throw DateTimeArithmeticException(e) else throw e

‎core/jvm/src/Instant.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst
9999
.run { if (days != 0) plusDays(days.toLong()) else this }
100100
.run { if (hours != 0) plusHours(hours.toLong()) else this }
101101
.run { if (minutes != 0) plusMinutes(minutes.toLong()) else this }
102-
.run { if (seconds != 0L) plusSeconds(seconds) else this }
103-
.run { if (nanoseconds != 0L) plusNanos(nanoseconds) else this }
102+
.run { if (seconds != 0) plusSeconds(seconds.toLong()) else this }
103+
.run { if (nanoseconds != 0) plusNanos(nanoseconds.toLong()) else this }
104104
}.toInstant().let(::Instant)
105105
} catch (e: DateTimeException) {
106106
throw DateTimeArithmeticException(e)
@@ -152,7 +152,7 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT
152152
val time = thisZdt.until(otherZdt, ChronoUnit.NANOS).nanoseconds
153153

154154
time.toComponents { hours, minutes, seconds, nanoseconds ->
155-
return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds.toLong(), nanoseconds.toLong())
155+
return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds, nanoseconds.toLong())
156156
}
157157
}
158158

‎core/jvm/test/ConvertersTest.kt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,21 @@ class ConvertersTest {
8585

8686
@Test
8787
fun datePeriod() {
88+
89+
fun assertJtPeriodEqual(a: JTPeriod, b: JTPeriod) {
90+
assertEquals(a.days, b.days)
91+
assertEquals(a.months + a.years * 12, b.months + b.years * 12)
92+
}
93+
8894
fun test(years: Int, months: Int, days: Int) {
8995
val ktPeriod = DatePeriod(years, months, days)
9096
val jtPeriod = JTPeriod.of(years, months, days)
9197

9298
assertEquals(ktPeriod, jtPeriod.toKotlinDatePeriod())
93-
assertEquals(jtPeriod, ktPeriod.toJavaPeriod())
99+
assertJtPeriodEqual(jtPeriod, ktPeriod.toJavaPeriod())
94100

95101
// TODO: assertEquals(ktPeriod, jtPeriod.toString().let(DatePeriod::parse))
96-
assertEquals(jtPeriod, ktPeriod.toString().let(JTPeriod::parse))
102+
assertJtPeriodEqual(jtPeriod, ktPeriod.toString().let(JTPeriod::parse))
97103
}
98104

99105
repeat(1000) {

‎core/native/src/Instant.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Instant = t
274274
plus(hours.toLong() * SECONDS_PER_HOUR, 0).check(timeZone) else this }
275275
.run { if (minutes != 0)
276276
plus(minutes.toLong() * SECONDS_PER_MINUTE, 0).check(timeZone) else this }
277-
.run { if (seconds != 0L) plus(seconds, 0).check(timeZone) else this }
278-
.run { if (nanoseconds != 0L) plus(0, nanoseconds).check(timeZone) else this }
277+
.run { if (seconds != 0) plus(seconds.toLong(), 0).check(timeZone) else this }
278+
.run { if (nanoseconds != 0) plus(0, nanoseconds.toLong()).check(timeZone) else this }
279279
}.check(timeZone)
280280
} catch (e: ArithmeticException) {
281281
throw DateTimeArithmeticException("Arithmetic overflow when adding CalendarPeriod to an Instant", e)
@@ -328,7 +328,7 @@ actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeri
328328
val time = thisLdt.until(otherLdt, DateTimeUnit.NANOSECOND).nanoseconds // |otherLdt - thisLdt| < 24h
329329

330330
time.toComponents { hours, minutes, seconds, nanoseconds ->
331-
return DateTimePeriod((months / 12), (months % 12), days, hours, minutes, seconds.toLong(), nanoseconds.toLong())
331+
return DateTimePeriod((months / 12), (months % 12), days, hours, minutes, seconds, nanoseconds.toLong())
332332
}
333333
}
334334

0 commit comments

Comments
 (0)