Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Change default amount serialization to string #231

Closed
chrylis opened this issue Aug 11, 2020 · 2 comments
Closed

Suggestion: Change default amount serialization to string #231

chrylis opened this issue Aug 11, 2020 · 2 comments

Comments

@chrylis
Copy link

chrylis commented Aug 11, 2020

Note: This is likely to be considered a breaking change, but I wanted to raise it due to the progress in #224.

While "amounts" are semantically numeric values, and JSON number values are technically arbitrary-precision decimal, the default handling for JSON numbers in most languages is as a double-precision-similar type (particularly JavaScript Number).

Representing the amount as a decimal number in string format is the safer choice as it eliminates the possibility of silent lossy conversion. With the proposed inclusion of this module into the "official contrib" space (and particularly the tendency to use findAndRegisterModules()), I suggest that the default behavior be changed to use the string representation, with the number representation available in the same optional manner as string currently is.

@whiskeysierra
Copy link
Collaborator

whiskeysierra commented Aug 11, 2020

I would be lying if I were to say that I didn't expect this to be a point of contention.

This is likely to be considered a breaking change

Yes, that would be a breaking change. Not just for current users of this library, but also in regards to the specification/guidelines that inspired us to provide this library in the first place: https://opensource.zalando.com/restful-api-guidelines/#173

While "amounts" are semantically numeric values, and JSON number values are technically arbitrary-precision decimal

I wholeheartedly agree with those two statements. Monetary amounts are by their nature inherently numeric. Representing them as numbers is not just the obvious choice but from a semantic perspective it's the only one. Since JSON treats numbers as arbitrary precision decimals, that's a perfect match, in my opinion.

the default handling for JSON numbers in most languages is as a double-precision-similar type

I agree with the factual statement in there, i.e. I'm not debating that most languages do that. But I don't agree with the conclusion. To me, it feels like being held hostage by limitations of technology of the past. If we don't try to challenge the status quo, how else would we progress and improve? IEEE 754 was good for its time, but all modern languages have arbitrary precision decimals now. It's time to use them. So this argument to me boils to do having semantics and specification in favor of numbers vs. some implementations against it. I'd choose semantics and specification, to be honest.

Moving away from this argument and trying to get the bigger picture. When is a number not a safe choice exactly? JSON is a data interchange format. Typically between two parties. One is defining the contract, the other one implements or adheres to it. If I'm on the client side, then I'm implementing someone else's spec, in which they hopefully clearly outlined how they represent monetary amounts. If that way matches the way of this library, great - just go ahead and use it. But it starts with a spec, not with this library. I sincerely hope that nobody just blindly produces API implementations by stitching libraries together without thinking about the actual contract of said API.

@whiskeysierra
Copy link
Collaborator

I'm closing this, because it is implemented by now:

https://github.com/zalando/jackson-datatype-money#serialization

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants