Match 'plus' with the corresponding 'minus' functions #49
Conversation
core/commonMain/src/Instant.kt
Outdated
| * The return value is clamped to the platform-specific boundaries for [Instant] if the result exceeds them. | ||
| */ | ||
| public fun Instant.minus(value: Int, unit: DateTimeUnit.TimeBased): Instant = | ||
| plus(-value.toLong(), unit) |
There was a problem hiding this comment.
Though it's likely the same, but the order of operations would be more explicit.
core/commonMain/src/Instant.kt
Outdated
| * @throws DateTimeArithmeticException if this value or the result is too large to fit in [LocalDateTime]. | ||
| */ | ||
| public fun Instant.minus(value: Int, unit: DateTimeUnit, timeZone: TimeZone): Instant = | ||
| plus(-value, unit, timeZone) |
There was a problem hiding this comment.
Is the negation always overflow-safe? I.e. it certainly can overflow at MIN_VALUE, but would this overflow be compensated later?
There was a problem hiding this comment.
You're right, this would be silently wrong. I pushed a fix for this and a similar problem with LocalDate.minus(Int, ...) on JVM.
There was a problem hiding this comment.
Could you also ensure that instant.minus(Long.MIN_VALUE, DateTimeUnit.NANOSECOND) > instant?
There was a problem hiding this comment.
Done. Since this edge case is a requirement, consistency also required solving a whole new can of worms with DateTimePeriod, which I also did.
Solves #42