Circe: Should decodeDouble really give NaN for null?

[oscar-broman]

NaN and null are two different things, and the potential problems surely outweigh the small benefit - especially when we have options!

For example, we dealt with an API that claimed a certain field would always have a double value. Then we got a NaN. The decoder didn't fail and the NaN propagated causing a crash in a place far from the API code.

Could an option be added to simply make the float/double decoders fail, rather than give NaN?

[travisbrown]

There are a couple of arguments for the current behavior:

• It's just what people tend to expect from a JSON library.
• We want to be able to round-trip floating point values, which means we have to map NaN to something (encoding can't fail), and JSON nulls are the most obvious candidate.

It's definitely possible to modify the default floating point type decoders so that they fail on JSON nulls, and we could potentially bundle these instances in an object under io.circe.extras so that you could easily import them. I think it's pretty unlikely that we'll change the default representation, though.

[oscar-broman]

I understand that changing the default representation seems reckless. However, an object under extras that would require import would make it hard to know where in the code the default double decoder is used, and where the more restrictive one is.
Could this perhaps be configurable by a global flag or something similar? In our use case, we want to make sure that no place where circe is used should it interpret null as NaN.

[oscar-broman]

Another way to make floating-point values truly "round-trippable" would be to encode NaN, Infinity and -Infinity as strings. Some libs in C#, for example, do this. Again, not default behavior but some way to toggle this globally would be great.

[travisbrown]

@oscar-broman We don't have any configuration like that, and I think that's a good thing. There's a standard way in Scala to override the behavior of type class instances, and that's by providing local instances (actually I guess there are two ways: local instances or new types, but the new type approach doesn't really fit well here in my view).

[oscar-broman]

Ok, got it. Could another way perhaps be to add an annotation?

Providing local instances is a bit bug prone as the code would compile either way.

case class Foo(@NotNull bar: Double)

[dcastro]

A bit late to the party, but here's an alternative using refined & circe-refined:

import io.circe.parser._
import io.circe.refined._
import eu.timepit.refined.api._
import eu.timepit.refined.numeric._

decode[Double Refined NonNaN]("null")

Left(DecodingFailure(Predicate failed: (NaN != NaN)., List()))

[eggsby]

As a user of this library I disagree that 'this is what the users expect'.
This was very unexpected behavior.

The json does not round trip:

Given:

case class Foo(x: Double)

If I decode the json string:
{"x": null} as Foo, the resulting round trip encoding becomes {}.
Now this string fails to decode and I lose my round-trip.

null should not be conflated with NaN or undefined.

As far as scala semantics go, if a user wanted to allow null, the expected behavior would be to define the type as Option[Double].

I think the default behavior should be an exception, but +1 for configuration option.