[jstan65536]

A lot about the Rubocop philosophy really grates on me. Many of its preferences are arbitrary and don't, in my opinion, contribute to code readability. Many others are good as a rule of thumb but cause more harm than good when they are blindly enforced by a robot. A recent example from my work went something like this:

  if some_verbose_condition && some_other_verbose_condition
    do_the_thing unless excluded_case || other_excluded_case
  end

Rubocop changed this to

  if (some_verbose_condition && some_other_verbose_condition) && !(excluded_case || other_excluded_case)
    do_the_thing
  end

which is just worse

and then it had the gall to complain that the line containing the `if` was too long.

That said, if you disable half its rules, Rubocop can be a useful tool. We've long had a list of database migration best practices, which we've built up over the years to ensure changes to our application's database schema don't cause downtime or other issues. Lately I've been writing cops to automate checks against these practices.

Useful feedback: "Heads up: changing the type of that column is going to lock the users table and bring the site down; see $BEST_PRACTICES_DOCUMENT"

Not useful feedback: "zomg ur cyclomatic complexity si 2 high!!1"

[d3nj4l]

I don't know, your version in the example looks pretty bad to me? Sure, maybe the excessive line length of the combined if is a readability issue, but something about a postfix unless inside an if makes it very hard to follow what combination of flags would trigger it. In this scenario, I'd try to give meaningful names to the boolean expressions, and write a simpler conditional.

[LanceH]

do_some_thing if ready_to_go(variables_needed_for_conditionals...)

[Spivak]

If this pattern just the norm in Ruby because I loathe these single purpose functions that are basically comments that I have to jump to another function to see. It makes tracing what actually happens in some piece of code impossible without a notes file to make it all on-screen.

[LanceH]

Considering OP was a bit vague with:

if some_verbose_condition && some_other_verbose_condition do_the_thing unless excluded_case || other_excluded_case end Rubocop changed this to if (some_verbose_condition && some_other_verbose_condition) && !(excluded_case || other_excluded_case) do_the_thing end

I don't really know what the proper solution is. I present one possibility here. "verbose condition" may or may not be complex, I don't know. But it's definitely a candidate for putting in it's own function and making tests out of it if there is complexity, or if it's just hard to read.

I'll agree there are too many of the following in Ruby:

def red?(color) return color == "red" end

As we don't know what his conditions are, we don't know if a function makes sense or not. There are times when you want to see the conditions up front, and times when they are a mere afterthought and relegating them to a function in the guard clause is best.

For whoever is downvoting just because they don't like the style of code in a discussion of abstract code, there is always:

do_the_thing if a && b unless c || d

Nothing like double guard clauses for double the lint fun.

[brightstep]

I don't like either of your or rubocop's approach. If you have that many conditionals, bind a local variable that describes the condition being checked. Then you have an easy to read `if` with a single variable.

[jaredsohn]

or a model method or private method in the class containing this code

[Spivak]

But why?!?! There's no way this conditional is a well-defined named concept and just adds indirection for the sake of line length.

[jaredsohn]

>There's no way this conditional is a well-defined named concept

Sure about that? I'm saying if it is then extracting as a method might be clearer. All depends and these are other options.

[brightstep]

maybe two well-named concepts then? If you're unable to reduce it at all, I'd consider that a code smell.

[quesera]

> We've long had a list of database migration best practices > ... > Lately I've been writing cops to automate checks against these practices.

These would be great to share and popularize. Too many Rails shops do this badly!

[jaredsohn]

There is already https://github.com/ankane/strong_migrations