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

GuardClause cop for loops #1010

Closed
geniou opened this issue Apr 16, 2014 · 3 comments
Closed

GuardClause cop for loops #1010

geniou opened this issue Apr 16, 2014 · 3 comments

Comments

@geniou
Copy link
Contributor

geniou commented Apr 16, 2014

What do you think about a cop that that checks for something similar to a guard clause but in loops. So something like this:

bad

foos.each do |foo|
 if foo['bar']
  # do something
 end
end

good

foos.each do |foo|
 next unless foo['bar']
 # do something
end

What do you think and how should this be called?

@geniou
Copy link
Contributor Author

geniou commented Apr 16, 2014

maybe it could be called PreferNext..

@jdickey
Copy link

jdickey commented Apr 16, 2014

I like it. That's something we keep forgetting to fix all over the place, and the next guard clause is much more readable. I'd also go for the name PreferNext.

Question: How should it deal with more pathological cases, like

foos.each do |foo|
  if foo['bar'] then
    # do *n* lines of something
  else
    if foo['quux'] then
      # do *m* lines of something else
    else
      # more bloviation here
    end
  end
end

I'd argue that the appropriate response would be to send a WTF email to everybody on the team; as a more practical workaround, should this new cop (or another related new cop) read this guy the riot act?

Come to think of it, this would be more appropriate as a different cop, yes? Something like MethodEnvy, since the example above really wants to be broken out into three support methods...

@geniou
Copy link
Contributor Author

geniou commented Apr 16, 2014

I think this cop should only take action if there is a single if at the end without any else. As you pointed out there could be an other cop, that takes care of such a logical mess.

@geniou geniou mentioned this issue Apr 28, 2014
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

No branches or pull requests

2 participants