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

Avoid floats when creating an Euro from string #6

Merged
merged 5 commits into from Mar 20, 2018

Conversation

@JeroenDeDauw
Copy link
Contributor

commented Mar 19, 2018

See discussion on #4

Copy link
Member

left a comment

I like this code a lot, because it sidesteps the float conversion.

while reading the code, I struggled a bit to understand what it does, esp. the padding and rounding parts. After I understood it, it was easy and clear, but maybe, for speeding up the understanding of future developers you can add some comments or split it into more functions?

Also, the "2" smells of a magic number. I think the code will become more clear, if we call it something like MAX_CENT_DIGITS or, stealing from Sebastian Bergmann FRACTION_DIGITS.

@JeroenDeDauw

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2018

That constant already existed as $DECIMAL_COUNT :)

@JeroenDeDauw JeroenDeDauw force-pushed the avoid-float branch from ab83f9b to a53ba09 Mar 20, 2018
@gbirke gbirke merged commit f4a3a20 into master Mar 20, 2018
3 checks passed
3 checks passed
Scrutinizer 3 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@gbirke gbirke deleted the avoid-float branch Mar 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.