Skip to content

Space Age: Don't compare floats using ==#103

Merged
kytrinyx merged 1 commit intoexercism:masterfrom
monkbroc:space-age-rounding
Mar 28, 2015
Merged

Space Age: Don't compare floats using ==#103
kytrinyx merged 1 commit intoexercism:masterfrom
monkbroc:space-age-rounding

Conversation

@monkbroc
Copy link
Copy Markdown
Contributor

It's bad form to compare floats with each other using == and Exercism shouldn't be showing this to aspiring programmers.

As the standard lib docs says asset_in_delta should be used to compare floats.

The modified test pass with code designed for the old test (i.e. with round(2) after the division) and allows new code not to have to round the division which feels unnatural (it should not a responsibility of
SpaceAge to round its output).

It's bad form to compare floats with each other using == and Exercism shouldn't be showing this to aspiring programmers.

[The standard lib docs says](http://ruby-doc.org/stdlib-1.9.2/libdoc/minitest/unit/rdoc/MiniTest/Assertions.html#method-i-assert_in_delta) that `asset_in_delta` should be used to compare floats.

The modified test pass with code designed for the old test (i.e. with `round(2)` after the division) and allows new code not to have to round the division which feels unnatural (it should not a responsibility of `SpaceAge` to round its output).
@monkbroc monkbroc changed the title Space Age: Don't compare floats with == Space Age: Don't compare floats using == Mar 27, 2015
@kytrinyx
Copy link
Copy Markdown
Member

Yes, this is a definite improvement. Thanks!

kytrinyx added a commit that referenced this pull request Mar 28, 2015
Space Age: Don't compare floats using ==
@kytrinyx kytrinyx merged commit 2f2fba6 into exercism:master Mar 28, 2015
kytrinyx added a commit that referenced this pull request Mar 31, 2015
Space Age: Don't compare floats using ==
gchan pushed a commit to gchan/xruby that referenced this pull request Oct 18, 2016
Added core set of test cases for word-count
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.

2 participants