47degrees / helios

A purely functional JSON library for Kotlin built on Λrrow
https://47degrees.github.io/helios/
Apache License 2.0
169 stars 22 forks source link

Add support for several types #69

Closed AdrianRaFo closed 5 years ago

AdrianRaFo commented 5 years ago

Add more instances to encode and decode the following Arrow types:

Also, support for kotlin types:

and finally, support for Java types:

I have tested all these new types with complex models on the sample module

Note: If you note that an important type is missing and want to see it on the first release please let me know to add it here

47degdev commented 5 years ago

Hood benchmark comparison: :heavy_check_mark: Decoding (Threshold: 257.7455896237748)

Benchmark Value
master_benchmark 10916.990504635269
helios_benchmark 11130.893134319518

:warning: Decodingfromraw (Threshold: 250.0)

Benchmark Value
master_benchmark 7232.359819202301
helios_benchmark 7093.866595814756

:heavy_check_mark: Parsing (Threshold: 250.0)

Benchmark Value
master_benchmark 22898.94171983691
helios_benchmark 22990.565607334767
nomisRev commented 5 years ago

@pakoito I thought I ported this from Circe but I just checked and the Eq instance looks quite different there. The only issue istoBiggerDecimal, which we don't have in Kotlin.

implicit final val eqJsonNumber: Eq[JsonNumber] = Eq.instance {
    case (JsonLong(x), JsonLong(y))             => x == y
    case (JsonDouble(x), JsonDouble(y))         => java.lang.Double.compare(x, y) == 0
    case (JsonFloat(x), JsonFloat(y))           => java.lang.Float.compare(x, y) == 0
    case (JsonBigDecimal(x), JsonBigDecimal(y)) => x.compareTo(y) == 0
    case (a, b)                                 => a.toBiggerDecimal == b.toBiggerDecimal
}

PS: In Arrow it shouldn't be broken since we only have Eq for Kotlin's Number types which we can just redirect to the underlying equals.

AdrianRaFo commented 5 years ago

I'm going to move the float comparison issue to this https://github.com/47deg/helios/issues/70

AdrianRaFo commented 5 years ago

I still need to fix the Map codec before merge

AdrianRaFo commented 5 years ago

This is ready for review now @nomisRev @pakoito @fedefernandez