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

Allow decimals for tax calculator #227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stantonxu
Copy link

- Summary

Fixes issue #153 - allow decimals for tax calculator

- Test plan

calculator_test.go

- Description for the changelog

Using float64 instead of uint64 / int64

- A picture of a cute animal (not mandatory but encouraged)

@stantonxu
Copy link
Author

/assign @mraerino

@stantonxu
Copy link
Author

stantonxu commented Apr 1, 2022

@mraerino could you please help review the PR for me?

@biilmann
Copy link
Member

biilmann commented Apr 1, 2022

Hey Jiefeng Xu, thanks for the contribution. As far as I can see this changes all currency calculations to use floats which is an anti pattern when it comes to money:

Here's a few article on why this should be avoided:

https://husobee.github.io/money/float/2016/09/23/never-use-floats-for-currency.html
https://dzone.com/articles/never-use-float-and-double-for-monetary-calculatio#:~:text=All%20floating%20point%20values%20that,store%20it%20as%20it%20is.

@stantonxu
Copy link
Author

Thank you @biilmann. Will read the articles soon.

@tingwei628
Copy link

How about using decimal ?

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

Successfully merging this pull request may close these issues.

3 participants