ionspin / kotlin-multiplatform-bignum

A Kotlin multiplatform library for arbitrary precision arithmetics
Apache License 2.0
350 stars 41 forks source link

BigInteger & BigDecimal kotlinx serialization support #181

Closed ValdZX closed 3 years ago

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

ionspin commented 3 years ago

Hi @ValdZX, thanks for the contribution, unfortunately I can't accept it in it's current form, as I don't want to add a dependency to the base bignum library except for the stdlib, and potentialy coroutines at some point. This was discussed a bit in #170 . If I am not mistaken it would be possible to publish a separate bignum-serialization library that could solve the issue the way you did, do you think that would work? If it could I'd prefer to go that way.

ValdZX commented 3 years ago

This solution is inspired by kotlinx-datetime In the common from new dependencies only plugin serialization and serializationJson for tests. I have no idea how it realize in separate module witout annotation @Serializable on class BigDecimal

ionspin commented 3 years ago

Ok, let me give this a bit more thought over weekend, and decide on way forward.

ionspin commented 3 years ago

I'll work on setting up a separate library that will just contain the KSerializer for BigInteger/Decimal and provide a serialization module, then when the users will be able to use it by importing the library and adding the serialization module when they are creating their instance of kotlinx serialization json. It's wont be as easy for users as having @Serializable on the classes themselves but it doesn't force a dependency on the people not using it, and also allows us to target the platforms we want and not only the ones kotlinx serialization supports (this is not a problem at the moment, but I've been burned by this before)

If you want me to include you commits, could you fix that problem that CLA_bot is complaining about, you might need to set up the email in you git client to the one you are using on github and then amend the commits and force push.

ValdZX commented 3 years ago

Gradle has function compileOnly for add dependecy without adding library to archive. It make support kotlinx serialization optional

ionspin commented 3 years ago

Gradle has function compileOnly for add dependecy without adding library to archive. It make support kotlinx serialization optional

I'm not a gradle expert but I am not sure that is the proper way to go, look at this forum post for more details https://discuss.gradle.org/t/is-it-recommended-to-use-compileonly-over-implementation-if-another-module-use-implementation-already/26699/2. Additionally I'm not sure how compileOnly would work with somebody including different version of kotlinx in their project. And it still keeps the problem of being able to support only the targets that serialization supports, currently the set is same, but that might change (i.e. riscv32/64 gets support, then we would need to wait on serialization to support that before we could support it)

ionspin commented 1 year ago

Hi @jQrgen,

here are some instructions https://github.com/ionspin/kotlin-multiplatform-bignum#serialization