Rubocop: Question re. 'Use a guard clause instead of wrapping the code inside a conditional expression.'

ruby-talk
1

Hello,

Rubocop complains with the following message with one of my methods:

  C: Use a guard clause instead of wrapping the code inside a conditional expression.
      if value >= 0
      ^^

···

#—-
  def get_credit_debit(value)
    if value >= 0
      return 'C'
    else
      return 'D'
    end
  end
#—-

This approach, although maybe being not very elegant, is from my point of view at least expressive / tells the reader exactly what’s going on.

I found the following advice [1] re. Rubocops warning and made this out of it:

*—-
  def get_credit_debit(value)
    return 'C' if value >= 0
    return 'D'
  end
#—-

But this looks strange / not right to me and Rubocop now gives me:

  C: Redundant return detected.
      return 'D'
      ^^^^^^

How do I have to implement the ‘guard clause’ mentioned by Rubocop correctly?

It’s probably more of a style-related question and there’s for sure no right way to do it, but it would be great to get some ideas on this.

Many thanks!

Cheers,
Michael

_____

[1]: http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause

2

Or even simpler:

def get_credit_debit(value)
  value >= 0 ? 'C' : 'D'
end

···

On Wed, Mar 2, 2016 at 5:04 AM, Doug Lake-Hammond <d.lakehammond@gmail.com> wrote:

Similarly, your original implementation could be simplified to:

def get_credit_debit(value)
  if value >= 0
    'C'
  else
    'D'
  end
end

--
Dave Aronson, consulting software developer of Codosaur.us,
PullRequestRoulette.com, Blog.Codosaur.us, and Dare2XL.com.

3

Hi Michael,

  C: Redundant return detected.

      return 'D'
      ^^^^^^

What rubocop is referring to here is that all ruby blocks evaluate to the
value of the last expression in the block. Rubocop shouldn't complain if
you use:

def get_credit_debit(value)
  return 'C' if value >= 0

  'D'
end

Similarly, your original implementation could be simplified to:

def get_credit_debit(value)
  if value >= 0
    'C'
  else
    'D'
  end
end

If you feel that this is more expressive, you might like to know that the
GuardClause cop has a MinBodyLength setting that causes it to trigger only
for more complex methods:

Carlo wrote:
Having a *machine* question about my coding style?
And this, based on someone else's personal opinions of what's

better/worse?

I can certainly see where you're coming from, after all the joy of ruby is
its expressiveness. All I would say is that rubocop is highly configurable,
and is perhaps better suited to group/open source projects, where it helps
to avoid time-wasting arguments over minor style details. It generally
favours more compact code, and personally it has taught me some great
tricks for keeping my code concise. It also identifies some coding idioms
that can hurt application performance.

Cheers,
Doug

4

Quoting Michael Schwarze (michael@schwarze-web.de):

How do I have to implement the ‘guard clause’ mentioned by Rubocop
correctly?

I believe you should be able to defuse the complaint by removing the
'return' keyword. Methods as default return the result of the last
instruction executed, so it is sufficient to write

'D'

to obtain what you need.

Anyway, it is the first time I hear about this 'rubocop' thing, and it
is nothing I'd like to have to meddle with. Having a *machine*
question about my coding style? And this, based on someone else's
personal opinions of what's better/worse?

Much of Ruby's beauty is due to the variety of possible ways to
express oneself. I happen to use both your original approach and the
suggested one. They both have space, and using both contributes to the
expressivity of your code, as well as to maintain you as a human being
that one small notch above machines.

Carlo

···

Subject: Rubocop: Question re. 'Use a guard clause instead of wrapping the code inside a conditional expression.'
  Date: mer 02 mar 16 10:24:38 +0100

--
  * Se la Strada e la sua Virtu' non fossero state messe da parte,
* K * Carlo E. Prelz - fluido@fluido.as che bisogno ci sarebbe
  * di parlare tanto di amore e di rettitudine? (Chuang-Tzu)

5

Hey!

Rubocop explains about explicit return at the end of the method. In order
to silence the warning you need to replace `return 'D'` with `'D'`. However

if value >= 0
  'C'
else
  'D'
end

definitely reads better than the guard clause. If you remove `return`
statements (they are not required) from your initial version the rubocop
doesn't complain. I'd say it's a bug in rubocop. I filed
https://github.com/bbatsov/rubocop/issues/2903.

Best regards

···

--
Greg Navis
I help small software companies to scale Heroku-hosted Rails apps.
http://www.gregnavis.com/

6

Hi Doug,

def get_credit_debit(value)
  return 'C' if value >= 0

  'D'
end

Although I can see the logic behind this I personally do not really feel comfortable with this approach…

Similarly, your original implementation could be simplified to:

def get_credit_debit(value)
  if value >= 0
    'C'
  else
    'D'
  end
end

…and therefore I will stay with this one!

If you feel that this is more expressive, you might like to know that the GuardClause cop has a MinBodyLength setting that causes it to trigger only for more complex methods:
https://github.com/bbatsov/rubocop/blob/master/config/default.yml#L500

Many thanks again to all of you for your comments; I’ll try to sum this up:

According to Rubocop’s GuardClause Style Cop [1] it’s better to not wrap all of a method’s code into one conditional; so instead of writing

  def test
    if something
      work
      work
      work
    end
  end

it’s better style to write

  def test
    return unless something
    work
    work
    work
  end

As stated by Doug (see above) you can configure the number of lines when to trigger for this Cop [2]; the default is one.

In my case (conditionally returning different values) there seems to be a bug in Rubocop’s implementation of the GuardClause Style Cop: the GuardClause Style Cop actually shouldn’t be applied here but Rubocop should complain about the unnecessary return statement. Greg was so kind to file an issue [3].

Ruby allows for many different ways to implement what I’m trying to achieve here, esp. less verbose ways. Which one to pick depends on your personal style.

Cheers,
Michael

···

_____

[1] http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause
[2] https://github.com/bbatsov/rubocop/blob/master/config/default.yml#L500
[3] https://github.com/bbatsov/rubocop/issues/2903

7

Hi Carlo,

Many thanks for the quick reply!

[…] so it is sufficient to write

'D'

to obtain what you need.

Yepp, that works!

[…] Having a *machine* question about my coding style? And this, based on someone else’s personal opinions of what’s better/worse?

It depends: it gives me an indication based on the opinions of 231 contributors [1] who are probably more senior Ruby developers than me. So I see it as a chance to learn and improve – and I’ve got no problem ignoring warnings or switching them off where I don’t agree :wink: But this one really bothered me as I couldn’t get my head around the warning.

Much of Ruby’s beauty is due to the variety of possible ways to express oneself. […]

100% agreed!

Cheers,
Michael

···

_____

[1] https://github.com/bbatsov/rubocop

8

Hi Greg,

if value >= 0
'C'
else
'D'
end

definitely reads better than the guard clause.

Yes, that works and reads better!

If you remove `return` statements (they are not required) from your initial version the rubocop doesn't complain. I’d say it’s a bug in rubocop.

Then Rubocop’s warning is definitely misleading here; I’m aware of return’s redundancy in Ruby but still like its expressiveness and I did not conclude from the warning re. a guard clause to my redundant use of return!

I filed https://github.com/bbatsov/rubocop/issues/2903.

Many thanks!

Cheers,
Michael