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

The suggested alternative for FlipFlop is wrong #5587

Closed
kke opened this issue Feb 21, 2018 · 4 comments
Closed

The suggested alternative for FlipFlop is wrong #5587

kke opened this issue Feb 21, 2018 · 4 comments

Comments

@kke
Copy link

kke commented Feb 21, 2018

The FlipFlop cop documentation from 684d835 states:

      # @example
      #   # bad
      #   (1..20).each do |x|
      #     puts x if (x == 5) .. (x == 10)
      #   end
      #
      #   # good
      #   (1..20).each do |x|
      #     puts x if (x >= 5) && (x <= 10)
      #   end

The suggested better alternative however does something completely different than the bad example for a different kind of dataset, for example:

irb(main):084:0> [5,20,15,10,5].each { |x| puts x if (x >= 5) && (x <= 10) }
5
10
5
irb(main):085:0> [5,20,15,10,5].each { |x| puts x if (x == 5)..(x == 10) }
5
20
15
10
5

Maybe the examples should be something like:

# bad
[4, 3, 2, 1, 2, 3, 4].each do |x|
  puts x if (x==3)..(x==2)
end

And:

# good
output_enabled = false
[4, 3, 2, 1, 2, 3, 4].each do |x|
  output_enabled = true if x == 3
  puts x if output_enabled
  output_enabled = false if x == 2
end

The good one does not look that hot either, but maybe someone can come up with a prettier solution.

@andriimosin
Copy link
Contributor

The 'good' example is definitely wrong here, but FlipFlop is really weird and rare operator and I'm not even sure what alternative can be put in a 'good' example.

Your suggested alternative is correct but in fact, it's just an algorithm rather than a good practice example. I'm not sure that it's the best option to suggest here.

Maybe it will sound too much, but maybe it's not be a bad idea to just suggest not to use it at all? I mean without 'good' example.

Check out this article about how rare this operator is:

about 1 in every 23 million lines of Ruby code, or 1-in-10 million to round to an order of magnitude

@koic
Copy link
Member

koic commented Jan 8, 2019

flip-flop is deprecated since Ruby 2.6.0.
https://github.com/ruby/ruby/blob/v2_6_0/NEWS#language-changes

I didn't improve the good example code, but I updated by #6635 why flip-flop isn't good.

@kke
Copy link
Author

kke commented Jan 8, 2019

Good riddance

@koic
Copy link
Member

koic commented Jan 8, 2019

Thank you. I will close this issue, but If you have good example code please open a PR.

@koic koic closed this as completed Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants