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

Consider disabling Style/EmptyCaseCondition #229

Closed
aliismayilov opened this issue Dec 15, 2020 · 4 comments
Closed

Consider disabling Style/EmptyCaseCondition #229

aliismayilov opened this issue Dec 15, 2020 · 4 comments
Labels
rule change 👩‍⚖️ Suggest a style guide rule change

Comments

@aliismayilov
Copy link
Contributor

I don't find either of following code snippets to be better than the other:

case
when done?       then :done
when exception?  then :exception
else :processing
end
if done?
  :done
elsif exception?
  :exception
else
  :processing
end

in fact, me and colleagues use the first example more often.

original discussion: rubocop/rubocop#3019

@searls searls added the rule change 👩‍⚖️ Suggest a style guide rule change label Dec 16, 2020
@searls
Copy link
Contributor

searls commented Dec 16, 2020

I appreciate the spirit of this and I'm always open to cases where keywords like case can improve expressiveness, but I think this rule is �a good one and it's going to stay. Reason being: many developers don't know what an empty case statement will do (I didn't until reading this issue, and I've been using Ruby for a while now), and reducing ambiguity in what a program will do ��at� r���untime has some value when we decide which rules are net-beneficial. Closing for now but will gladly welcome others' opinions in the comments

@searls searls closed this as completed Dec 16, 2020
@aliismayilov
Copy link
Contributor Author

I appreciate the spirit of this and I'm always open to cases where keywords like case can improve expressiveness, but I think this rule is �a good one and it's going to stay. Reason being: many developers don't know what an empty case statement will do (I didn't until reading this issue, and I've been using Ruby for a while now), and reducing ambiguity in what a program will do ��at� r���untime has some value when we decide which rules are net-beneficial. Closing for now but will gladly welcome others' opinions in the comments

Thanks for clarification! I see your point, but I'd like to then ask, wouldn't the same argument (many developers don't know how case statement works) apply in the following example too?

case string
when "foo" then puts "hi!"
when /^foo.*/ then puts "ho!"
end

This might not be the best example, but my point is that it's possible that multiple when conditions can match the variable, but only the first matching one will be executed. This was never confusing [to me], as I've never seen break statements inside case statements in Ruby (and I love this cleanliness). With Ruby's new pattern matching inside case statements might introduce more of these situations, where multiple when conditions might match and developers should know how this works.

I hope, I could explain myself, as with the above example, the standard gem will be happy, but the developers who don't know how case statements work in Ruby will still be confused.

@twe4ked
Copy link
Contributor

twe4ked commented Dec 17, 2020

I often use empty case statements over if/elsif when there are more than 2-3 options. I find it makes it clearer when the branches aren't different importances.

@nilcolor
Copy link

nilcolor commented Nov 7, 2024

Also, ifs kind of imply hierarchy (imo). While cases is kind of "decision point" with all options to be on the same level of importance... (imo again). Hence

case # rubocop:disable Style/EmptyCaseCondition
when search_term then by_term
...

is kind of common in our code base :) (yeah, this can be disabled but still doesn't)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule change 👩‍⚖️ Suggest a style guide rule change
Projects
None yet
Development

No branches or pull requests

4 participants