-
Notifications
You must be signed in to change notification settings - Fork 110
FR: Use inline class for Instant #121
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
Comments
"Arbitrary" as in chosen by us, the library writers, or by you? If it's the latter, then Also, |
Thanks, those are valid points regarding the more hypothetical "relative time" use case, I did mean arbitrary reference point chosen by my own code. On that note, if I did decide to go down this route at some point, I would want to use Int as the underlying base type for millisecond-precision durations up to about 49 days, which is sufficient for my use case. I cannot do this with the Duration type. My primary use case at this time remains a Long/Double-wrapping inline Instant class; the alternative relative time option is a somewhat radical optimization to my present codebase, so I'm not planning to pursue it any time soon, if ever. As a stopgap I've created my own |
Could you explain why not?
But how is it different from a |
Long vs. Int: the motivation for this feature request is memory footprint. Int has half the memory footprint of Duration's Long (on many platforms). My use case is limited to durations of no more than one or two days, and I do not need nanosecond precision, so Int would be a good choice. Using a |
We'd rather avoid introducing another type for representing instants into |
Could it make sense to provide some of the general-purpose date/time APIs for usage with epoch-based milliseconds without the extra step of a throwaway CompactInstant conversion? And In my KMP project I have to pull in kotlinx-datetime just to log some human-readable debug timestamps from a ms-since-epoch value; alternatively I could introduce platform-specific dependencies, but that seems even worse. |
@bubenheimer https://github.com/korlibs/klock has what you are looking for. |
@OrhanTozan, are you talking about the |
@bubenheimer, it looks like your needs can be easily satisfied with just a tiny wrapper around @JvmInline
public value class CompactInstant(public val sinceEpoch: Duration): Comparable<CompactInstant> {
public companion object {
public fun fromEpochMilliseconds(epochMilliseconds: Long): CompactInstant =
CompactInstant(epochMilliseconds.milliseconds)
}
private val kotlinxInstant: Instant
get() = Instant.fromEpochMilliseconds(0) + sinceEpoch
override fun toString(): String = kotlinxInstant.toString()
public operator fun plus(other: Duration): CompactInstant = CompactInstant(sinceEpoch.plus(other))
public operator fun minus(other: Duration): CompactInstant = CompactInstant(sinceEpoch.minus(other))
public operator fun minus(other: CompactInstant): Duration = sinceEpoch.minus(other.sinceEpoch)
override fun compareTo(other: CompactInstant): Int = sinceEpoch.compareTo(other.sinceEpoch)
} Tested like this: val instant1 = CompactInstant.fromEpochMilliseconds(1637843185123L)
val instant2 = CompactInstant.fromEpochMilliseconds(1444443185123L)
assertEquals("2021-11-25T12:26:25.123Z", instant1.toString())
assertEquals("2015-10-10T02:13:05.123Z", instant2.toString())
assertTrue(instant2 < instant1)
assertFalse(instant2 > instant1)
assertNotEquals(instant2, instant1)
assertTrue(instant2 != instant1)
assertEquals(instant2, instant2) In my opinion, if something in its entirety takes about 20 straightforward lines of code, it's better to just copy-paste the definition into the project and not include any library: this way, adding new operations, removing old ones, and in general iterating on the thing is more straightforward. EDIT: for example, for eventually replacing the What do you think? |
Missed another portion of your request.
Do you mean, just a separate library artifact that only contained a function to convert the number of epoch milliseconds into an ISO-8601 string? All the other things you've requested are more or less just For formatting, |
@OrhanTozan thank you for the pointer.
Yes, I was thinking of it as foundational functionality (a core artifact or utility artifact), that other datetime artifacts would use. But from what you're saying that's not how the code is structured, unless it would be appropriate to move LocalDateTime & LocalDate into this core artifact. All I want here is write a human-readable date/time string to Android logcat and/or a bug tracker for troubleshooting; it's hard to justify pulling in the entire datetime library only for this, but I can't easily and automatically post-process those types of logs, either. Also, thank you for coming up with that code gist, it is interesting to see the Duration-based wrapper, I may switch to it at some point. |
My needs for Java-agnostic datetime handling may expand over time, that would provide reasonable justification for incorporating the full datetime artificat. |
There's not much demand for this, and given how easy #121 (comment) is to introduce in the special scenarios where memory footprint is crucial, there doesn't seem to be much point in adding this to our library. The API being compact and learnable is a property that we don't want to give up easily. |
When initially looking at this library I was hoping to find an implementation of Instant that just wraps a Long or Double with an inline class, similar to the implementation of Duration. I would like to replace plain Long millisecond values in web server code where I cannot compromise on the memory footprint needed for these values. My API needs are basic, essentially "ms since epoch" (or possibly: ms since an arbitrary more recent reference point), adding/subtracting Durations, getting time deltas as Duration, plus basic logging for debug & monitoring purposes.
The text was updated successfully, but these errors were encountered: