Open mdedetrich opened 9 years ago
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.bd.toString
.Option
vs. Either
vs. scalaz.Validation
vs. scalaz.\/
vs. cats.Xor
vs. ...It's a fuzzy line, but this seems non-minimal to me.
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.
@seratch @sirthias @eed3si9n @non @propensive @xuwei-k You guys want to put some input into this?
Going to close this for now, I have let the converters in there, if anyone has complaints, re-open the ticket
We currently have
BigDecimal
converters in the reboot version ofjson4s-ast
. There is an argument that this should be separate from thejson4s-ast
, however there are also strong arguments for why it should be included, which areBigDecimal
insideJNumber
. As an example, a common rookie mistake is to do something likeJNumber(BigDecimal(3434f))
, where as, what you should do is something likeJNumber(BigDecimal.decimal(3434f))
. That method doesn't even exist inScala 2.10
, hence why the reason behind https://github.com/mdedetrich/json4s-ast/blob/reboot/jvm/src/main/scala/org/json4s/JValue.scala#L20JNumber
, which can help in serialization/portability between different libraries that usejson4s-ast
. Although not currently implemented, having a converter for something likeBigDecimal
toArray[Byte]
does make some sense in establishing a formatBigDecimal
toInt
when used for things like ids@inline
(see https://github.com/json4s/json4s-ast/issues/2)