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

Implement LocalDate.fromEpochDays #214

Merged
merged 4 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/common/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ public expect class Instant : Comparable<Instant> {
* Returns an [Instant] that is [epochMilliseconds] number of milliseconds from the epoch instant `1970-01-01T00:00:00Z`.
*
* The return value is clamped to the platform-specific boundaries for [Instant] if the result exceeds them.
*
* @see Instant.toEpochMilliseconds
*/
public fun fromEpochMilliseconds(epochMilliseconds: Long): Instant

Expand Down
18 changes: 18 additions & 0 deletions core/common/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ public expect class LocalDate : Comparable<LocalDate> {
*/
public fun parse(isoString: String): LocalDate

/**
* Returns a [LocalDate] that is [epochDays] number of days from the epoch day `1970-01-01`.
*
* @throws IllegalArgumentException if the result exceeds the platform-specific boundaries of [LocalDate].
*
* @see LocalDate.toEpochDays
*/
public fun fromEpochDays(epochDays: Int): LocalDate

internal val MIN: LocalDate
internal val MAX: LocalDate
}
Expand Down Expand Up @@ -79,6 +88,15 @@ public expect class LocalDate : Comparable<LocalDate> {
/** Returns the day-of-year component of the date. */
public val dayOfYear: Int

/**
* Returns the number of days since the epoch day `1970-01-01`.
*
* If the result does not fit in [Int], returns [Int.MAX_VALUE] for a positive result or [Int.MIN_VALUE] for a negative result.
*
* @see LocalDate.fromEpochDays
*/
public fun toEpochDays(): Int

/**
* Compares `this` date with the [other] date.
* Returns zero if this date represent the same day as the other (i.e. equal to other),
Expand Down
15 changes: 14 additions & 1 deletion core/common/src/math.kt
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,17 @@ internal fun multiplyAndAdd(d: Long, n: Long, r: Long): Long {
mr -= n
}
return safeAdd(safeMultiply(md, n), mr)
}
}

// org.threeten.bp.chrono.IsoChronology#isLeapYear
internal fun isLeapYear(year: Int): Boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains common math functions, not specific to any date calculations. I suggest to find another file for these new ones, or introduce a new one, e.g. dateMath.kt/calendarMath.kt/IsoChronology.kt

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does define some constants like internal const val HOURS_PER_DAY = 24 or NANOS_PER_DAY already, so it's not just pure math here. I don't think it's worth it yet to split the file, because

  • it's relatively short,
  • when writing code, I like to look over the available constants, and seeing all of them in one place helps, whereas we would need to move, say, HOURS_PER_DAY into the date-specific-math file, keeping NANOS_PER_ONE in the pure math one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, when we've got two new functions, it's a good chance to group these constants together with them in a separate file.
NANOS_PER_ONE can also be seen as a time-related constant, not general math.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Peculiarly, trying to implement this refactoring slows down the code on JS considerably. The test for fromEpochDays now fails, and checking on my machine, before the refactoring, it takes 1.2 ± 0.2 seconds, while after the refactoring, it takes 1.6 ± 0.3 seconds. I propose that we revert this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't confirm such effect, on the opposite the test runs faster for me on the latest commit than on the previous.
I compared generated JS code and didn't find anything suspicious in the difference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the last resort, the timeout could be increased if it's essential to test all these values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's do that.

val prolepticYear: Long = year.toLong()
return prolepticYear and 3 == 0L && (prolepticYear % 100 != 0L || prolepticYear % 400 == 0L)
}

internal fun Int.monthLength(isLeapYear: Boolean): Int =
when (this) {
2 -> if (isLeapYear) 29 else 28
4, 6, 9, 11 -> 30
else -> 31
}
89 changes: 86 additions & 3 deletions core/common/test/LocalDateTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ class LocalDateTest {

@Test
fun parseIsoString() {
fun checkParsedComponents(value: String, year: Int, month: Int, day: Int, dayOfWeek: Int, dayOfYear: Int) {
fun checkParsedComponents(value: String, year: Int, month: Int, day: Int, dayOfWeek: Int? = null, dayOfYear: Int? = null) {
checkComponents(LocalDate.parse(value), year, month, day, dayOfWeek, dayOfYear)
assertEquals(value, LocalDate(year, month, day).toString())
}
checkParsedComponents("2019-10-01", 2019, 10, 1, 2, 274)
checkParsedComponents("2016-02-29", 2016, 2, 29, 1, 60)
Expand All @@ -49,6 +50,17 @@ class LocalDateTest {
assertInvalidFormat { LocalDate.parse("2017-10--01") }
// this date is currently larger than the largest representable one any of the platforms:
assertInvalidFormat { LocalDate.parse("+1000000000-10-01") }
// threetenbp
checkParsedComponents("2008-07-05", 2008, 7, 5)
checkParsedComponents("2007-12-31", 2007, 12, 31)
checkParsedComponents("0999-12-31", 999, 12, 31)
checkParsedComponents("-0001-01-02", -1, 1, 2)
checkParsedComponents("9999-12-31", 9999, 12, 31)
checkParsedComponents("-9999-12-31", -9999, 12, 31)
checkParsedComponents("+10000-01-01", 10000, 1, 1)
checkParsedComponents("-10000-01-01", -10000, 1, 1)
checkParsedComponents("+123456-01-01", 123456, 1, 1)
checkParsedComponents("-123456-01-01", -123456, 1, 1)
}

@Test
Expand Down Expand Up @@ -221,9 +233,61 @@ class LocalDateTest {
assertEquals(Int.MIN_VALUE, LocalDate.MAX.until(LocalDate.MIN, DateTimeUnit.DAY))
}
}
}

@Test
fun fromEpochDays() {
/** This test uses [LocalDate.next] and [LocalDate.previous] and not [LocalDate.plus] because, on Native,
* [LocalDate.plus] is implemented via [LocalDate.toEpochDays]/[LocalDate.fromEpochDays], and so it's better to
* test those independently. */
if (LocalDate.fromEpochDays(0).daysUntil(LocalDate.MIN) > Int.MIN_VALUE) {
assertEquals(LocalDate.MIN, LocalDate.fromEpochDays(LocalDate.MIN.toEpochDays()))
assertFailsWith<IllegalArgumentException> { LocalDate.fromEpochDays(LocalDate.MIN.toEpochDays() - 1) }
assertFailsWith<IllegalArgumentException> { LocalDate.fromEpochDays(Int.MIN_VALUE) }
}
if (LocalDate.fromEpochDays(0).daysUntil(LocalDate.MAX) < Int.MAX_VALUE) {
assertEquals(LocalDate.MAX, LocalDate.fromEpochDays(LocalDate.MAX.toEpochDays()))
assertFailsWith<IllegalArgumentException> { LocalDate.fromEpochDays(LocalDate.MAX.toEpochDays() + 1) }
assertFailsWith<IllegalArgumentException> { LocalDate.fromEpochDays(Int.MAX_VALUE) }
}
val eraBeginning = -678941 - 40587
assertEquals(LocalDate(1970, 1, 1), LocalDate.fromEpochDays(0))
assertEquals(LocalDate(0, 1, 1), LocalDate.fromEpochDays(eraBeginning))
assertEquals(LocalDate(-1, 12, 31), LocalDate.fromEpochDays(eraBeginning - 1))
var test = LocalDate(0, 1, 1)
for (i in eraBeginning..699999) {
assertEquals(test, LocalDate.fromEpochDays(i))
test = test.next
}
test = LocalDate(0, 1, 1)
for (i in eraBeginning downTo -2000000 + 1) {
assertEquals(test, LocalDate.fromEpochDays(i))
test = test.previous
}
}

// threetenbp
@Test
fun toEpochDays() {
/** This test uses [LocalDate.next] and [LocalDate.previous] and not [LocalDate.plus] because, on Native,
* [LocalDate.plus] is implemented via [LocalDate.toEpochDays]/[LocalDate.fromEpochDays], and so it's better to
* test those independently. */
val startOfEra = -678941 - 40587
var date = LocalDate(0, 1, 1)
for (i in startOfEra..699999) {
assertEquals(i, date.toEpochDays())
date = date.next
}
date = LocalDate(0, 1, 1)
for (i in startOfEra downTo -2000000 + 1) {
assertEquals(i, date.toEpochDays())
date = date.previous
}
assertEquals(-40587, LocalDate(1858, 11, 17).toEpochDays())
assertEquals(-678575 - 40587, LocalDate(1, 1, 1).toEpochDays())
assertEquals(49987 - 40587, LocalDate(1995, 9, 27).toEpochDays())
assertEquals(0, LocalDate(1970, 1, 1).toEpochDays())
assertEquals(-678942 - 40587, LocalDate(-1, 12, 31).toEpochDays())
}
}

fun checkInvalidDate(constructor: (year: Int, month: Int, day: Int) -> LocalDate) {
assertFailsWith<IllegalArgumentException> { constructor(2007, 2, 29) }
Expand All @@ -236,3 +300,22 @@ fun checkInvalidDate(constructor: (year: Int, month: Int, day: Int) -> LocalDate
assertFailsWith<IllegalArgumentException> { constructor(2007, 0, 1) }
assertFailsWith<IllegalArgumentException> { constructor(2007, 13, 1) }
}

private val LocalDate.next: LocalDate get() =
if (dayOfMonth != monthNumber.monthLength(isLeapYear(year))) {
LocalDate(year, monthNumber, dayOfMonth + 1)
} else if (monthNumber != 12) {
LocalDate(year, monthNumber + 1, 1)
} else {
LocalDate(year + 1, 1, 1)
}

private val LocalDate.previous: LocalDate get() =
if (dayOfMonth != 1) {
LocalDate(year, monthNumber, dayOfMonth - 1)
} else if (monthNumber != 1) {
val newMonthNumber = monthNumber - 1
LocalDate(year, newMonthNumber, newMonthNumber.monthLength(isLeapYear(year)))
} else {
LocalDate(year - 1, 12, 31)
}
9 changes: 9 additions & 0 deletions core/js/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa

internal actual val MIN: LocalDate = LocalDate(jtLocalDate.MIN)
internal actual val MAX: LocalDate = LocalDate(jtLocalDate.MAX)

public actual fun fromEpochDays(epochDays: Int): LocalDate = try {
LocalDate(jtLocalDate.ofEpochDay(epochDays))
} catch (e: Throwable) {
if (e.isJodaDateTimeException()) throw IllegalArgumentException(e)
throw e
}
}

public actual constructor(year: Int, monthNumber: Int, dayOfMonth: Int) :
Expand Down Expand Up @@ -49,6 +56,8 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa
actual override fun toString(): String = value.toString()

actual override fun compareTo(other: LocalDate): Int = this.value.compareTo(other.value).toInt()

public actual fun toEpochDays(): Int = value.toEpochDay().toInt()
}

public actual fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate = plusNumber(1, unit)
Expand Down
5 changes: 5 additions & 0 deletions core/jvm/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa

internal actual val MIN: LocalDate = LocalDate(jtLocalDate.MIN)
internal actual val MAX: LocalDate = LocalDate(jtLocalDate.MAX)

public actual fun fromEpochDays(epochDays: Int): LocalDate =
LocalDate(jtLocalDate.ofEpochDay(epochDays.toLong()))
}

public actual constructor(year: Int, monthNumber: Int, dayOfMonth: Int) :
Expand All @@ -49,6 +52,8 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa
actual override fun toString(): String = value.toString()

actual override fun compareTo(other: LocalDate): Int = this.value.compareTo(other.value)

public actual fun toEpochDays(): Int = value.toEpochDay().clampToInt()
}

public actual fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate =
Expand Down
2 changes: 1 addition & 1 deletion core/native/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private val instantParser: Parser<Instant>
} catch (e: ArithmeticException) {
throw DateTimeFormatException(e)
}
val epochDay = localDate.toEpochDay().toLong()
val epochDay = localDate.toEpochDays().toLong()
val instantSecs = epochDay * 86400 - offset.totalSeconds + localTime.toSecondOfDay() + secDelta
try {
Instant(instantSecs, nano)
Expand Down
39 changes: 9 additions & 30 deletions core/native/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
init {
// org.threeten.bp.LocalDate#create
require(isValidYear(year)) { "Invalid date: the year is out of range" }
require(monthNumber >= 1 && monthNumber <= 12) { "Invalid date: month must be a number between 1 and 12, got $monthNumber" }
require(dayOfMonth >= 1 && dayOfMonth <= 31) { "Invalid date: day of month must be a number between 1 and 31, got $dayOfMonth" }
require(monthNumber in 1..12) { "Invalid date: month must be a number between 1 and 12, got $monthNumber" }
require(dayOfMonth in 1..31) { "Invalid date: day of month must be a number between 1 and 31, got $dayOfMonth" }
if (dayOfMonth > 28 && dayOfMonth > monthNumber.monthLength(isLeapYear(year))) {
if (dayOfMonth == 29) {
throw IllegalArgumentException("Invalid date 'February 29' as '$year' is not a leap year")
Expand All @@ -60,16 +60,12 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
localDateParser.parse(isoString)

// org.threeten.bp.LocalDate#toEpochDay
/**
* @throws IllegalArgumentException if the result exceeds the boundaries
*/
internal fun ofEpochDay(epochDay: Int): LocalDate {
public actual fun fromEpochDays(epochDays: Int): LocalDate {
// LocalDate(-999999, 1, 1).toEpochDay(), LocalDate(999999, 12, 31).toEpochDay()
// Unidiomatic code due to https://github.com/Kotlin/kotlinx-datetime/issues/5
require(epochDay >= MIN_EPOCH_DAY && epochDay <= MAX_EPOCH_DAY) {
require(epochDays in MIN_EPOCH_DAY..MAX_EPOCH_DAY) {
"Invalid date: boundaries of LocalDate exceeded"
}
var zeroDay = epochDay + DAYS_0000_TO_1970
var zeroDay = epochDays + DAYS_0000_TO_1970
// find the march-based year
zeroDay -= 60 // adjust to 0000-03-01 so leap day is at end of four year cycle

Expand Down Expand Up @@ -106,7 +102,7 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
}

// org.threeten.bp.LocalDate#toEpochDay
internal fun toEpochDay(): Int {
public actual fun toEpochDays(): Int {
val y = year
val m = monthNumber
var total = 0
Expand Down Expand Up @@ -140,7 +136,7 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
// org.threeten.bp.LocalDate#getDayOfWeek
public actual val dayOfWeek: DayOfWeek
get() {
val dow0 = (toEpochDay() + 3).mod(7)
val dow0 = (toEpochDays() + 3).mod(7)
return DayOfWeek(dow0 + 1)
}

Expand Down Expand Up @@ -170,15 +166,6 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
return LocalDate(year, month, newDay)
}

// org.threeten.bp.LocalDate#plusYears
/**
* @throws IllegalArgumentException if the result exceeds the boundaries
* @throws ArithmeticException if arithmetic overflow occurs
*/
internal fun plusYears(yearsToAdd: Int): LocalDate =
if (yearsToAdd == 0) this
else resolvePreviousValid(safeAdd(year, yearsToAdd), monthNumber, dayOfMonth)

// org.threeten.bp.LocalDate#plusMonths
/**
* @throws IllegalArgumentException if the result exceeds the boundaries
Expand All @@ -195,22 +182,14 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
return resolvePreviousValid(newYear, newMonth, dayOfMonth)
}

// org.threeten.bp.LocalDate#plusWeeks
/**
* @throws IllegalArgumentException if the result exceeds the boundaries
* @throws ArithmeticException if arithmetic overflow occurs
*/
internal fun plusWeeks(value: Int): LocalDate =
plusDays(safeMultiply(value, 7))

// org.threeten.bp.LocalDate#plusDays
/**
* @throws IllegalArgumentException if the result exceeds the boundaries
* @throws ArithmeticException if arithmetic overflow occurs
*/
internal fun plusDays(daysToAdd: Int): LocalDate =
if (daysToAdd == 0) this
else ofEpochDay(safeAdd(toEpochDay(), daysToAdd))
else fromEpochDays(safeAdd(toEpochDays(), daysToAdd))

override fun equals(other: Any?): Boolean =
this === other || (other is LocalDate && compareTo(other) == 0)
Expand Down Expand Up @@ -292,7 +271,7 @@ public actual fun LocalDate.until(other: LocalDate, unit: DateTimeUnit.DateBased

// org.threeten.bp.LocalDate#daysUntil
public actual fun LocalDate.daysUntil(other: LocalDate): Int =
other.toEpochDay() - this.toEpochDay()
other.toEpochDays() - this.toEpochDays()

// org.threeten.bp.LocalDate#getProlepticMonth
internal val LocalDate.prolepticMonth get() = (year * 12) + (monthNumber - 1)
Expand Down
2 changes: 1 addition & 1 deletion core/native/src/LocalDateTime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public actual constructor(public actual val date: LocalDate, public actual val t

// org.threeten.bp.chrono.ChronoLocalDateTime#toEpochSecond
internal fun toEpochSecond(offset: UtcOffset): Long {
val epochDay = date.toEpochDay().toLong()
val epochDay = date.toEpochDays().toLong()
var secs: Long = epochDay * 86400 + time.toSecondOfDay()
secs -= offset.totalSeconds
return secs
Expand Down
9 changes: 0 additions & 9 deletions core/native/src/Month.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,3 @@ internal fun Month.firstDayOfYear(leapYear: Boolean): Int {
Month.DECEMBER -> 335 + leap
}
}

// From threetenbp
internal fun Int.monthLength(leapYear: Boolean): Int {
return when (this) {
2 -> if (leapYear) 29 else 28
4, 6, 9, 11 -> 30
else -> 31
}
}
2 changes: 1 addition & 1 deletion core/native/src/TimeZone.kt
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ internal fun Instant.toLocalDateTimeImpl(offset: UtcOffset): LocalDateTime {
val localSecond: Long = epochSeconds + offset.totalSeconds // overflow caught later
val localEpochDay = localSecond.floorDiv(SECONDS_PER_DAY.toLong()).toInt()
val secsOfDay = localSecond.mod(SECONDS_PER_DAY.toLong()).toInt()
val date: LocalDate = LocalDate.ofEpochDay(localEpochDay) // may throw
val date: LocalDate = LocalDate.fromEpochDays(localEpochDay) // may throw
val time: LocalTime = LocalTime.ofSecondOfDay(secsOfDay, nanosecondsOfSecond)
return LocalDateTime(date, time)
}
Expand Down
8 changes: 1 addition & 7 deletions core/native/src/mathNative.kt
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,4 @@ internal fun zoneIdByOffset(totalSeconds: Int): String {
}
buf.toString()
}
}

// org.threeten.bp.chrono.IsoChronology#isLeapYear
internal fun isLeapYear(year: Int): Boolean {
val prolepticYear: Long = year.toLong()
return prolepticYear and 3 == 0L && (prolepticYear % 100 != 0L || prolepticYear % 400 == 0L)
}
}
Loading