rubocop: How to avoid Multiline block chains? What is the recommended style?

@alexdowad A tool such as Rubocop should be more than an arbitrary set of style choice and should aim to codify good pratices. In this sense, I think that every check should have a documented rationale why it exists and why the style it warns is considered bad.

@bbatsov @alexdowad Can someone elaborate why this “style fix” is enabled per default? I really don’t see why having an intermediate variable is a huge improvement over tacit-style of chaining blocks together?

So all the style guide says on the topic is:

(multi-line chaining is always ugly)

Still sounds pretty arbitrary to me. Anyway added it to the 30 other disabled cops in my config 👅.

I’ve found splitting the block taking method onto the next line to be helpful for this cop, eg:

    EM.run do
      Pusher.
        get_async('/channels/' + pusher_channel).
        callback { |response| handle_response(response) }.
        errback { |error| handle_error(error) }
    end

It’s particularly helpful for me when trying to comprehend what a long chain of enumerators is doing.

If you run rubocop -S, you’ll get a link to the relevant style guide section, so in this case the rule isn’t arbitrary at all.

Speaking more generally, I think it’s better if RuboCop is too strict by default than too lenient. There’s the issue of discoverability. Cops that don’t report will go unnoticed.

So don’t let RuboCop be your master. If you disagree with a style choice, configure the cop if that’s possible, or disable it if it’s not.

maybe

EM.run do
  something = Pusher.get_async('/channels/' + pusher_channel).callback do |response|
    handle_response(response)
  end

  something.errback do |error|
    handle_error(error)
  end
end

I think that every check should have a documented rationale why it exists

Sure. Nobody will argue with that.

There’s a lot of things which RC should do but doesn’t. It’s a work in progress.

…And we do recommend that you disable the check on your own projects if you disagree.

The primary maintainer of RC has made it clear that he wants RC’s defaults to be strict rather than lenient, so completely disabling this “cop” by default would be less likely to be accepted.

In recently years I’m actually learning towards more lenient defaults (in particular fewer cops enabled by default), but:

• any change of defaults is a breaking change, so it has to be considered carefully
• the metrics cops in particular don’t make much sense if they are too lenient

I wrote some thoughts about the future of RuboCop here, in case someone’s curious about it.

@KelseyDH, that’s an unfortunate position to be in. Thanks for providing this information.

I’m sure you can appreciate that it makes it difficult for the RC developers to help users if the users cannot or will not follow advice. More could be said, but I won’t belabor that point.

As a suggestion, perhaps the most pragmatic course of action which you could undertake (if you want to make things better for yourself and others like you) would be to submit a PR raising the default threshold for Metrics/AbcSize. The primary maintainer of RC has made it clear that he wants RC’s defaults to be strict rather than lenient, so completely disabling this “cop” by default would be less likely to be accepted.

Before anyone asks, I am not available to prepare a PR, review a PR, advocate for a PR, etc. It would be interesting to hear if you do get one accepted, though.