Open NotJustAnna opened 2 years ago
Hey Adrian, Found this through Hacktoberfest and I think this is a great project. It being compatible with Twitter snowflakes and having Kotlin MPP support are big ups, and I would like to keep it updated with the Kotlin ecosystem.
Let me know what you think about the following:
adding support for additional Kotlin targets (linux and windows). DateTime
may not be as straight-forward. Maybe kotlinx-datetime would be useful here. Or see my second point in the design section.
adding componentN
functions so we can destructure nanoflakes. Would destructure into the 3 fields of a nanoflake.
there are lot of getter like methods in Nanoflake
. Idiomatic Kotlin would rather these simply be val
s imo.
making Nanoflake a data class around value and epoch, this would remove a bit of boilerplate in that class.
The following are more of a larger design proposal which could warrant its own issue or flat out ignored. These come out of a feeling as if I'm using a Java library instead of one made for Kotlin.
Get rid of baseEpoch
being an integral part to the Nanoflake
structure. AFAIK it is not part of the Nanoflake
structure according to the spec and it seems to just be a convenience thing. Maybe something like a NanoflakeWithEpoch
or similarly named, would be appropriate here instead. It would wrap a Nanoflake
, take an additional baseEpoch
, and add the additional functionality involving the baseEpoch
. Though a different alternative could be great as well.
Getting rid of baseEpoch
would allow for Nanoflake
becoming a value class around the value. This should remove the overhead of creating a new instance for every Nanoflake
as it'll eventually be erased into a long. We can still keep destructuring, and utility functions. This one seems like a big improvement.
Would it make sense to make generators suspending? Currently, when generating a nanoflake, if the clock moves backwards, it will throw an exception for x ms. Instead, we could also provide a suspending API which will simply delay for that long until our timestamp is going forward again. I'm not crazy for this since it could be considered out of scope but might be good to think about.
Let me know how you feel about these and what warrant their own issues to further discuss them :smile:.
Hey @lost-illusi0n! Thank you for showing interest regarding the project!
adding support for additional Kotlin targets (linux and windows).
I totally agree! At the time that I implemented the Kotlin library, I didn't try to dip my toes with Kotlin/Native since it was pretty experimental, but having used it for a bit I think that we can add more Kotlin/Native targets.
adding
componentN
functions so we can destructure nanoflakes.
That's legit something that I didn't consider at the time, but I think it might be useful to some people. I approve of it!
there are lot of getter like methods in
Nanoflake
. Idiomatic Kotlin would rather these simply beval
s imo.
Considering that the Kotlin library was a port from Java, I totally agree that I didn't use the most idiomatic style of Kotlin. In other hand, I'd like to consider each case (or class) being transformed in consideration first.
Regarding the three design points:
I agree to the first and second points. Value classes definitively weren't around at that time, and I think that the NanoflakeWithEpoch
solution is also good.
In regard to the last point, although I don't see a issue by itself, wouldn't suspension functions require a dependency in kotlinx-coroutines? If so, I'd like to either avoid or implement a SuspendingNanoflakeGenerator in another library (either as a sub-library to this one or as another repository on the Nanoflakes organization)
Phew, I think that covers it all? I really liked the points that you brought in and some overall improvements! Thank you and please feel free to ask more questions or open a PR!
Awesome, great to hear we're on the same page.
In other hand, I'd like to consider each case (or class) being transformed in consideration first.
I don't think I fully understand what was asked. For the base Nanoflake
I think the following changes would take place:
asLong()
and asString()
NanoflakeWithEpoch
val
s for timestamp
, generator
, and sequence
that are backed by the appropriate Nanoflakes.xValue
methods.wouldn't suspension functions require a dependency in kotlinx-coroutines? ...
Yes, it contains the delay
function.
If avoiding the dependency in the core implementation is the goal, I would go with a Gradle multi-module approach however that may change up artifact names (e.g, we would have nanoflakes-core
and nanoflakes-suspending
?) and I don't know how I feel about renaming the core artifact from nanoflakes-kotlin only for a small addition.
I'm not sure if this would warrant an entire new repository either though. Perhaps we just leave it as is for now?
Regarding everything else, I'm happy to start working on them and start sending PRs once they're ready. Cheers!
Answering @lost-illusi0n
I don't think I fully understand what was asked. For the base
Nanoflake
I think the following changes would take place:* keep `asLong()` and `asString()` * all epoch related stuff goes away into `NanoflakeWithEpoch` * provide `val`s for `timestamp`, `generator`, and `sequence` that are backed by the appropriate `Nanoflakes.xValue` methods.
Oh, I see. I fully approve those changes!
If avoiding the dependency in the core implementation is the goal, I would go with a Gradle multi-module approach however that may change up artifact names (e.g, we would have
nanoflakes-core
andnanoflakes-suspending
?) and I don't know how I feel about renaming the core artifact from nanoflakes-kotlin only for a small addition.
I'd like to avoid the dependency on Kotlin Coroutines, as well as a unnecessary dependency on kotlinx-datetime on Jvm and Js if possible.
I'd go with Gradle multi-modules as well! (Had forgotten the name of it so I called it sub-library in my previous comment)
IMO the coroutine-aware version could be called nanoflakes-kotlin-coroutines
, and in this way we'd keep the core library as nanoflakes-kotlin
.
Regarding everything else, I'm happy to start working on them and start sending PRs once they're ready. Cheers!
You have my greenlight!
Hacktoberfest!
Hey there! If you're reading this, you're probably here for Hacktoberfest. If you're not, you're probably here because you're interested in nanoflakes. Either way, welcome!
Note
This is a child issue to nanoflakes/.github#4, the main Hacktoberfest issue for Nanoflakes. Use this issue to discuss improvements and contributions for the Kotlin implementation of Nanoflakes.
What's a Nanoflake?
Nanoflakes is a specification for unique identifiers which can be generated locally or from a remote server. The specification is designed to be simple and easy to implement, and to be compatible with Twitter's Snowflake algorithm, which Nanoflakes is based on.
The official, full specification can be found here.
I want to contribute!
Awesome! You can contribute to the Kotlin implementation by submitting PRs to this repository!
You can improve on the Kotlin implementation by...
If you have any other ideas for how you can contribute, please open an issue or pull request to suggest them in this issue!
End note
If you have any questions, feel free to ask them in the comments below!
Nanoflakes is my hobby project, and I'm doing this for fun. I'm not expecting anyone to contribute, but if you do, I'll be happy to accept your pull requests!