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

[wazery]

I have the following code

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

For now I disable it with # rubocop:disable Style/MultilineBlockChain which I need to resolve.

As you can see, the block chaining is necessary for me since I need to handle the errback.

So what is the recommended style for this, I see that the Ruby style guide recommends avoiding using multiline block chains, but with not proposing any other option for it!

[maxjacobson]

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

[alexdowad]

@wazery, does @maxjacobson's answer satisfy you?

[esad]

@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?

[alexdowad]

@esad It's just an aesthetic judgement. To many (but not all) programmers' eyes, code which uses ... end.method ... is slightly less readable than code which doesn't. This is especially so to those who are not very familiar with Ruby.

Nobody is saying it's a "huge improvement". It's a marginal improvement.

[alexdowad]

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

[esad]

@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.

[alexdowad]

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.

[esad]

//cc @jonas054

[jonas054]

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.

[dbodow]

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.

[DuBistKomisch]

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 :tongue:.