ionspin / kotlin-multiplatform-bignum

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

Implement Number #170

Closed tadfisher closed 3 years ago

tadfisher commented 3 years ago

Is your feature request related to a problem? Please describe.

I'm trying to serialize a BigDecimal using kotlinx.serialization into a JSON-formatted string, but the library gives me two choices:

  1. Convert to a Double before serializing, resulting in IEEE-754-induced precision loss
  2. Serialize as a quoted string, breaking compatibility with the receiving deserializer

Describe the solution you'd like

kotlinx.serialization does support the kotlin.Number type, which does exactly what I want: call toString() on the value and encode the resulting string as an unquoted JSON number.

Describe alternatives you've considered

The other option for me is to add expect class BigDecimal : Number and essentially reimplement this library.

Additional context

Bug report about serializing bignums: https://github.com/Kotlin/kotlinx.serialization/issues/1051

tadfisher commented 3 years ago

Here's a hack I found to make this work:

// HACK: Wrap BigDecimal to extend Number, allowing us to serialize as an unquoted string.
private class BigDecimalNumber(val value: BigDecimal) : Number() {
    override fun toByte(): Byte = throw UnsupportedOperationException()
    override fun toChar(): Char = throw UnsupportedOperationException()
    override fun toDouble(): Double = throw UnsupportedOperationException()
    override fun toFloat(): Float = throw UnsupportedOperationException()
    override fun toInt(): Int = throw UnsupportedOperationException()
    override fun toLong(): Long = throw UnsupportedOperationException()
    override fun toShort(): Short = throw UnsupportedOperationException()
    override fun toString(): String = value.toStringExpanded()
}

@Serializer(BigDecimal::class)
object BigDecimalSerializer : KSerializer<BigDecimal> {
    override val descriptor: SerialDescriptor =
        PrimitiveSerialDescriptor("BigDecimal", PrimitiveKind.DOUBLE)

    override fun deserialize(decoder: Decoder): BigDecimal {
        val element = (decoder as JsonDecoder).decodeJsonElement() as JsonPrimitive
        return element.content.toBigDecimal()
    }

    override fun serialize(encoder: Encoder, value: BigDecimal) {
        val element = JsonPrimitive(BigDecimalNumber(value))
        (encoder as JsonEncoder).encodeJsonElement(element)
    }
}

Note: This hack does not avoid precision loss from conversion to Double, as JsonSerializer converts the string representation to a Double internally.

ionspin commented 3 years ago

Hi @tadfisher ,

I actually wanted to implement serialization long time ago, it just slipped my mind. Thanks for the feature request and the workaround, I'll see if I can write a proper solution over the weekend!

ionspin commented 3 years ago

After thinking about this for a bit I decided not to do it, because I don't want to have any additional dependencies except Kotlin standard library, at least for now.

volkert-fastned commented 6 months ago

@tadfisher Thanks for the workaround! I believe the precision loss on serialization can be avoided when we make use of the (currently still experimental) JsonUnquotedLiteral instead of JsonPrimitive, which would also eliminate the need for the BigDecimalNumber wrapper. This is done by rewriting the override fun serialize part as follows:

    override fun serialize(encoder: Encoder, value: BigDecimal) {
        val element = JsonUnquotedLiteral(value.toStringExpanded())
        (encoder as JsonEncoder).encodeJsonElement(element)
    }

@ionspin I'm not sure what "additional dependencies" you are referring to. With the above modification, @tadfisher's workaround only needs the following mult-platform imports:

import com.ionspin.kotlin.bignum.decimal.BigDecimal
import com.ionspin.kotlin.bignum.decimal.toBigDecimal
import kotlinx.serialization.KSerializer
import kotlinx.serialization.Serializer
import kotlinx.serialization.descriptors.PrimitiveKind
import kotlinx.serialization.descriptors.PrimitiveSerialDescriptor
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder
import kotlinx.serialization.json.JsonDecoder
import kotlinx.serialization.json.JsonEncoder
import kotlinx.serialization.json.JsonPrimitive
import kotlinx.serialization.json.JsonUnquotedLiteral
ionspin commented 6 months ago

I am referring to kotlinx.serialization library.

volkert-fastned commented 6 months ago

@ionspin OK, but then it woul be useful to include this serializer in the bignum-serialization-kotlinx library, right? that library/dependency is already dependent on kotlinx.serialization.

ionspin commented 6 months ago

Yes, that would be great, pull request would be highly appreciated!

volkert-fastned commented 6 months ago

Would be happy to. But bignum-serialization-kotlinx doesn't appear to have its own repository. Is that correct?

ionspin commented 6 months ago

Yeah, that is correct.