reboot - using BigDecimal converters

[mdedetrich]

We currently have BigDecimal converters in the reboot version of json4s-ast. There is an argument that this should be separate from the json4s-ast, however there are also strong arguments for why it should be included, which are

1. Verifies correctness of the construction of the BigDecimal inside JNumber. As an example, a common rookie mistake is to do something like JNumber(BigDecimal(3434f)), where as, what you should do is something like JNumber(BigDecimal.decimal(3434f)). That method doesn't even exist in Scala 2.10, hence why the reason behind https://github.com/mdedetrich/json4s-ast/blob/reboot/jvm/src/main/scala/org/json4s/JValue.scala#L20
2. Mandate a standard format for JNumber, which can help in serialization/portability between different libraries that use json4s-ast. Although not currently implemented, having a converter for something like BigDecimal to Array[Byte] does make some sense in establishing a format
3. Provide convenience methods for really common use cases, i.e. going from BigDecimal to Int when used for things like ids
4. Can provide performance improvements due to our aggressive use of @inline (see reboot - use of @inline #2)

[rossabaker]

1. There is also BigDecimal.binary and BigDecimal.exact for various types. Is one always right for JSON for each numeric type, or is choice good? I honestly don't know.
2. JSON serializes to a character-based format. I'm not sure that it's anymore complicated than bd.toString.
3. But is the goal to make the AST usable as a standalone project? I agree that these are common use cases, but there are so many approaches to conversion:

• exceptions vs. Option vs. Either vs. scalaz.Validation vs. scalaz.\/ vs. cats.Xor vs. ...
• strictness of validation
• cohesion with codecs for more complex types

It's a fuzzy line, but this seems non-minimal to me.

[mdedetrich]

There is also BigDecimal.binary and BigDecimal.exact for various types. Is one always right for JSON for each numeric type, or is choice good? I honestly don't know.

I think this is why having BigDecimal converters makes more sense, it means that libraries that use json4s-ast can interopt with eachother. As to answer your question, not sure, maybe the exact are supposed to be the correct ones?

JSON serializes to a character-based format. I'm not sure that it's anymore complicated than bd.toString.

Locale/CharSet? I haven't delved into this area, so I think you know more than me in this area.

But is the goal to make the AST usable as a standalone project? I agree that these are common use cases, but there are so many approaches to conversion:

Indeed it is fuzzy, the only real compelling case I can come up with for inclusion that isn't convenience is inter-opting between the various JValue types, that and making sure JNumber is always constructed properly.

By interopting, I mean that we have libraries A and B, both of which use json4s-ast, can both do something like JNumber(..).to[Int], and they know that it will work, because both are using the same implementation so they will always provide the same value. If I look at the original converters, they seem to mimic what number types in predef do (i.e. they all provide converters to byte/short/int etc etc)

Like I said before, these were in there when I started, so maybe we can asking @casualjim if there was a reason why they are there. If its because spray/play/typesafe asked for it, then we should probably re-evulate, else it makes sense to just remove them.

[mdedetrich]

@seratch @sirthias @eed3si9n @non @propensive @xuwei-k You guys want to put some input into this?

[mdedetrich]

Going to close this for now, I have let the converters in there, if anyone has complaints, re-open the ticket