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

Use github.com/shopspring/decimal #1

Closed
joefitzgerald opened this issue Jul 16, 2017 · 3 comments
Closed

Use github.com/shopspring/decimal #1

joefitzgerald opened this issue Jul 16, 2017 · 3 comments

Comments

@joefitzgerald
Copy link

Rationale: https://github.com/shopspring/decimal#why-dont-you-just-use-float64

Using float64 to handle decimal values can lead to unexpected outcomes. If you're open to this, I would be happy to open a PR for it.

@adlio
Copy link
Owner

adlio commented Jul 21, 2017

What's the use case where a float64 issue can occur now? Users of this package are free to take the float64 and wrap it in a decimal type if they need to do math on it. The package itself doesn't do any mathematical operations (at least none that I can think of).

I had wanted to use a Decimal type instead of float64, but didn't want to introduce dependencies just for their types. Golang #12127 suggests we may eventually get stdlib support for decimals, and lists a few contenders for the model (including shopspring/decimal). Without a resolution there, I don't want to change types in a backwards incompatible ways.

I'd be open to a pull request that adds a second accessor for a shopspring/decimal for the places you'd want one added.

@ajgrf
Copy link

ajgrf commented Mar 26, 2018

Why not just use json.Number? Then users can convert it to whichever number type they like.

This is what I did in my own ad-hoc Harvest API package.

@ajgrf
Copy link

ajgrf commented Mar 26, 2018

Users of this package are free to take the float64 and wrap it in a decimal type if they need to do math on it.

This is really not a good strategy for dealing with monetary values because the conversion itself may introduce errors before any calculation even occurs.

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

No branches or pull requests

3 participants