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

Introduce a recommended style for float division #628

Closed
Drenmi opened this issue Feb 7, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@Drenmi
Copy link
Contributor

commented Feb 7, 2017

By default, Ruby will perform integer division when both receiver and argument are integers. The way you perform float division on integers is to coerce one or more of them into a float. I propose there be a guideline for which part--receiver or argument--should take the #to_f coercion.

I am leaning towards both the receiver and the argument being coerced, because:

  1. If you only coerce one, changing the code gets more risky, because you may unwittingly replace the float coerced one with an integer, regressing to integer division.

  2. Choosing one to apply #to_f to is (as far as I can see) completely arbitrary.

Possible styles:

# both
a.to_f / b.to_f

# receiver
a.to_f / b

# argument
a / b.to_f

Another option could be to recommend #fdiv:

a.fdiv(b)

WDYT?

@backus

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2017

Personally I'd prefer the default style to not be a form that requires #to_f on both sides. Since it isn't necessary it is dead code to require both.

Personally it seems like the preference should be to enforce that the left hand receives #to_f. They you always know you are dealing with Float#to_f. Not true if the right hand is the thing being coerced. Since it is ruby I still don't really know what / means for unknown type a 😄

@Drenmi

This comment has been minimized.

Copy link
Contributor Author

commented Feb 7, 2017

@backus: Unless #to_f was overridden for type a (but let's not go there. 😅) I think your argument is sound, and you're well on your way to win me over. 😀

@GBH

This comment has been minimized.

Copy link

commented Jan 21, 2019

Got burned very recently on this bug/feature. Was wondering if there was a cop that would prevent this. It's very easy to make 5/2 == 2 when you really expected 2.5. If you forget this gotcha you're in trouble.

@bbatsov

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2019

@Drenmi Let's recommend left-hand #to_f or fdiv. I think both are fine approaches, provided you stick to them consistently.

tejasbubane added a commit to tejasbubane/ruby-style-guide that referenced this issue Jun 12, 2019

@bbatsov bbatsov closed this in #756 Jun 12, 2019

bbatsov added a commit that referenced this issue Jun 12, 2019

koic added a commit to koic/ruby-style-guide that referenced this issue Jun 22, 2019

Update a style guide for "Float Division"
This PR updates a style guide for "Float Division".

The original issue for which this style was proposed.
rubocop-hq#628

It is an update based on the following argument.
rubocop-hq/rubocop#7153 (comment)

The following is a quote from the argument about the reason for this change.

> I wanted to show the following option to make a bad case only when both have `to_f`.
>
> ```ruby
> # bad
> a.to_f / b.to_f
>
> # good
> a.to_f / b
> a / b.to_f
> a.fdiv(b)
> ```
>
> Whether `to_f` is used on the left or right depends on the nature of `a`
> or `b` parameter. On the other hand, it is redundant that `to_f` is used
> in both. At that time, I think it is better to let users choose whether
> to remove the left side `to_f` or the right side `to_f`. This is the
> reason I recommend this option to default.

bbatsov added a commit that referenced this issue Jun 22, 2019

Update a style guide for "Float Division"
This PR updates a style guide for "Float Division".

The original issue for which this style was proposed.
#628

It is an update based on the following argument.
rubocop-hq/rubocop#7153 (comment)

The following is a quote from the argument about the reason for this change.

> I wanted to show the following option to make a bad case only when both have `to_f`.
>
> ```ruby
> # bad
> a.to_f / b.to_f
>
> # good
> a.to_f / b
> a / b.to_f
> a.fdiv(b)
> ```
>
> Whether `to_f` is used on the left or right depends on the nature of `a`
> or `b` parameter. On the other hand, it is redundant that `to_f` is used
> in both. At that time, I think it is better to let users choose whether
> to remove the left side `to_f` or the right side `to_f`. This is the
> reason I recommend this option to default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.