Skip to content

LocalTime#toSecondOfDay & LocalTime#ofSecondOfDay. #204

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

Merged
merged 3 commits into from
May 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 21 additions & 1 deletion core/common/src/LocalTime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@ public expect class LocalTime : Comparable<LocalTime> {
*/
public fun parse(isoString: String): LocalTime

/**
* Returns a LocalTime with the specified [secondOfDay]. The nanosecond field will be set to zero.
*
* @throws IllegalArgumentException if the boundaries of [secondOfDay] are exceeded.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The reference is broken now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's referencing the parameter. Also works for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, ok, my bad.

*/
public fun fromSecondOfDay(secondOfDay: Int): LocalTime

/**
* Returns a LocalTime with the specified [nanosecondOfDay].
*
* @throws IllegalArgumentException if the boundaries of [nanosecondOfDay] are exceeded.
*/
public fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime

internal val MIN: LocalTime
internal val MAX: LocalTime
}
Expand All @@ -66,6 +80,12 @@ public expect class LocalTime : Comparable<LocalTime> {
/** Returns the nanosecond-of-second time component of this time value. */
public val nanosecond: Int

/** Returns the time as a second of a day, from 0 to 24 * 60 * 60 - 1. */
public val secondOfDay: Int

/** Returns the time as a nanosecond of a day, from 0 to 24 * 60 * 60 * 1000 - 1. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/** Returns the time as a nanosecond of a day, from 0 to 24 * 60 * 60 * 1000 - 1. */
/** Returns the time as a nanosecond of a day, from 0 to 24 * 60 * 60 * 1_000_000_000 - 1. */

public val nanosecondOfDay: Long

/**
* Compares `this` time value with the [other] time value.
* Returns zero if this value is equal to the other, a negative number if this value occurs earlier
Expand Down Expand Up @@ -110,4 +130,4 @@ public fun LocalTime.atDate(year: Int, month: Month, dayOfMonth: Int = 0): Local
/**
* Combines this time's components with the specified [LocalDate] components into a [LocalDateTime] value.
*/
public fun LocalTime.atDate(date: LocalDate): LocalDateTime = LocalDateTime(date, this)
public fun LocalTime.atDate(date: LocalDate): LocalDateTime = LocalDateTime(date, this)
54 changes: 52 additions & 2 deletions core/common/test/LocalTimeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
package kotlinx.datetime.test

import kotlinx.datetime.*
import kotlinx.datetime.Clock
import kotlin.math.min
import kotlin.test.*

class LocalTimeTest {
Expand Down Expand Up @@ -85,6 +83,58 @@ class LocalTimeTest {
assertFailsWith<IllegalArgumentException> { LocalTime(0, 0, 0, 1_000_000_000) }
}

@Test
fun fromNanosecondOfDay() {
val data = mapOf(
0L to LocalTime(0, 0),
5000000001L to LocalTime(0, 0, 5, 1),
44105000000100L to LocalTime(12, 15, 5, 100),
86399000000999L to LocalTime(23, 59, 59, 999),
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of these have the millisecond and microsecond triads zeroed. I'd add a test with 123456789 nanoseconds or something like that.

)

data.forEach { (nanosecondOfDay, localTime) ->
assertEquals(nanosecondOfDay, localTime.nanosecondOfDay)
assertEquals(localTime, LocalTime.fromNanosecondOfDay(nanosecondOfDay))
}
}

@Test
fun fromNanosecondOfDayInvalid() {
LocalTime.fromNanosecondOfDay(0)
assertFailsWith<IllegalArgumentException> { LocalTime.fromNanosecondOfDay(-1) }
assertFailsWith<IllegalArgumentException> { LocalTime.fromNanosecondOfDay(Long.MAX_VALUE) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

A good idea is to check the boundaries. For example, in the test above, in addition to (or instead of) testing 86399000000999L, 86399999999999L, the maximum admissible value, could be tested, and here we could test 86400000000000L, the first non-admissible value.

}

@Test
fun fromSecondOfDay() {
val data = mapOf(
0 to LocalTime(0, 0),
5 to LocalTime(0, 0, 5),
44105 to LocalTime(12, 15, 5),
86399 to LocalTime(23, 59, 59),
)

data.forEach { (secondOfDay, localTime) ->
assertEquals(secondOfDay, localTime.secondOfDay)
assertEquals(localTime, LocalTime.fromSecondOfDay(secondOfDay))
}
}

@Test
fun fromSecondOfDayInvalid() {
LocalTime.fromSecondOfDay(0)
assertFailsWith<IllegalArgumentException> { LocalTime.fromSecondOfDay(-1) }
assertFailsWith<IllegalArgumentException> { LocalTime.fromSecondOfDay(Int.MAX_VALUE) }
}

@Test
fun fromSecondOfDayIgnoresNanosecond() {
assertEquals(
0,
LocalTime(0, 0, 0, 100).secondOfDay,
)
}

@Test
fun atDate() {
val time = LocalTime(12, 1, 59)
Expand Down
18 changes: 17 additions & 1 deletion core/js/src/LocalTime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
public actual val minute: Int get() = value.minute().toInt()
public actual val second: Int get() = value.second().toInt()
public actual val nanosecond: Int get() = value.nano().toInt()
public actual val secondOfDay: Int get() = value.toSecondOfDay().toInt()
public actual val nanosecondOfDay: Long get() = value.toNanoOfDay().toLong()

override fun equals(other: Any?): Boolean =
(this === other) || (other is LocalTime && this.value == other.value)
Expand All @@ -44,7 +46,21 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
throw e
}

public actual fun fromSecondOfDay(secondOfDay: Int): LocalTime = try {
jtLocalTime.ofSecondOfDay(secondOfDay, 0).let(::LocalTime)
} catch (e: Throwable) {
if (e.isJodaDateTimeParseException()) throw DateTimeFormatException(e)
throw e
}

public actual fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime = try {
jtLocalTime.ofNanoOfDay(nanosecondOfDay).let(::LocalTime)
} catch (e: Throwable) {
if (e.isJodaDateTimeParseException()) throw DateTimeFormatException(e)
throw e
}

internal actual val MIN: LocalTime = LocalTime(jtLocalTime.MIN)
internal actual val MAX: LocalTime = LocalTime(jtLocalTime.MAX)
}
}
}
16 changes: 15 additions & 1 deletion core/jvm/src/LocalTime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
public actual val minute: Int get() = value.minute
public actual val second: Int get() = value.second
public actual val nanosecond: Int get() = value.nano
public actual val secondOfDay: Int get() = value.toSecondOfDay()
public actual val nanosecondOfDay: Long get() = value.toNanoOfDay()

override fun equals(other: Any?): Boolean =
(this === other) || (other is LocalTime && this.value == other.value)
Expand All @@ -46,7 +48,19 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
throw DateTimeFormatException(e)
}

public actual fun fromSecondOfDay(secondOfDay: Int): LocalTime = try {
jtLocalTime.ofSecondOfDay(secondOfDay.toLong()).let(::LocalTime)
} catch (e: DateTimeException) {
throw IllegalArgumentException(e)
}

public actual fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime = try {
jtLocalTime.ofNanoOfDay(nanosecondOfDay).let(::LocalTime)
} catch (e: DateTimeException) {
throw IllegalArgumentException(e)
}

internal actual val MIN: LocalTime = LocalTime(jtLocalTime.MIN)
internal actual val MAX: LocalTime = LocalTime(jtLocalTime.MAX)
}
}
}
9 changes: 9 additions & 0 deletions core/native/src/LocalTime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ public actual class LocalTime actual constructor(
public actual fun parse(isoString: String): LocalTime =
localTimeParser.parse(isoString)

public actual fun fromSecondOfDay(secondOfDay: Int): LocalTime =
ofSecondOfDay(secondOfDay, 0)

public actual fun fromNanosecondOfDay(nanosecondOfDay: Long): LocalTime =
ofNanoOfDay(nanosecondOfDay)

// org.threeten.bp.LocalTime#ofSecondOfDay(long, int)
internal fun ofSecondOfDay(secondOfDay: Int, nanoOfSecond: Int): LocalTime {
// Unidiomatic code due to https://github.com/Kotlin/kotlinx-datetime/issues/5
Expand Down Expand Up @@ -117,6 +123,9 @@ public actual class LocalTime actual constructor(
return (nod xor (nod ushr 32)).toInt()
}

public actual val secondOfDay: Int get() = toSecondOfDay()
public actual val nanosecondOfDay: Long get() = toNanoOfDay()

// org.threeten.bp.LocalTime#toNanoOfDay
internal fun toNanoOfDay(): Long {
var total: Long = hour.toLong() * NANOS_PER_ONE * SECONDS_PER_HOUR
Expand Down