Add local time #187
Add local time #187
Conversation
| * See [LocalTime.parse] for examples of time string representations. | ||
| * | ||
| * @throws IllegalArgumentException if the text cannot be parsed or the boundaries of [LocalTime] are exceeded. | ||
| */ |
There was a problem hiding this comment.
The API looks nice.
The documentation, I think, should be carefully reviewed and reworded. It's clearly copy-pasted with few changes from LocalDateTime, but some points just don't make as much sense here.
For example, is it a real concern that someone would think that LocalTime somehow includes the time zone? (For the record, Java does think so: https://docs.oracle.com/javase/8/docs/api/java/time/LocalTime.html, maybe they have a point?) In any case, it should be enough to state it at most once that LocalTime doesn't know the date or the time zone. The less useless clutter there is in the docs, the easier it is to find the relevant parts.
|
@dkhalanskyjb I also added some converter methods between LocalDate/LocalTime |
| check(minute, 0, 59, "minute") | ||
| check(second, 0, 59, "second") | ||
| check(nanosecond, 0, NANOS_PER_ONE - 1, "nanosecond") | ||
| } |
There was a problem hiding this comment.
Sure, it's safe, just a bit more extra work for the computer to do. Maybe we'll merge this as is and rework it later if benchmarks show that the impact is non-negligible.
|
When will this be available , I really need this... |
|
Thank you for the great work! |
|
What's the reason that LocalTime.Companion.MIN and MAX are internal? They seem to be unused, can they be made public? |
|
We probably just forgot to remove them at some point. We didn't plan to make them public, at least with those names. Do you need them for something? |
|
Previously we had our own LocalTime implementation that looks 95% like this implementation. For now we have our own extension: import kotlinx.datetime.LocalTime
private val min = LocalTime(0, 0, 0, 0)
public val LocalTime.Companion.Min: LocalTime get() = min
private val max = LocalTime(23, 59, 59, 999_999_999)
public val LocalTime.Companion.Max: LocalTime get() = maxBut I think it's justified to add these upstream in pairity with the javas LocalTime. |
|
Could you please show some code that uses these constants? I didn't quite grasp how they help with calculating the fasting streaks. Also, if it's something non-trivial, please open a separate issue so that this rationale becomes easier to find. |
Fixes #57