[QA] Ruby Coding Conventions and RuboCop (was: Rubocop style checker)

[Dan Duvall]

On Thu, Nov 20, 2014 at 9:05 AM, Antoine Musso <hashar+wmf at free.fr> wrote:

> Le 20/11/2014 01:38, Dan Duvall a écrit :
> > Ironically enough, it has failed the Travis CI check because of an
> > overzealous `GuardClause` check. (Seriously, who can't read an `if`
> > statement ... :)
>
>
> http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause
>
> Well, one could name it early return.  That saves a level of indentation
> and make it obvious that on that condition nothing else is executed.
> Else you have to browse down at the end of the condition to look at
> potentially additional code.
>
> In php I definitely prefers:
>
>  if ( ! $cond ) {
>    return;
>  }
>  // lot of code
>
> Against:
>
>  if ( $cond ) {
>    // lot of code
>  }
>
> But yeah might be overzealous :]
>

That makes sense in some cases, especially where there are a lot of
operations following the condition, but I would try to avoid that many
operations in a single function anyhow. In this case there are conditionals
there already, and using guards for all of them would change the entire
behavior of method.

In other words, changing this ...

  def foo
    something1 if condition1

    something2 unless condition2

    something3 if condition3
  end

... to this ...

  def foo
    return unless condition1
    something1

    return if condition2
    something2

    return unless condition3
    something3
  end

... would alter the code paths to where a falsey condition1 skips
something2 and something3. It's actually equivalent to using nested
conditions ...

  def foo
    if condition1
      something1

      unless condition2
        something2

        if condition3
          something3
        end
      end
  end

... which is definitely harder to make sense of. :)

I guess that's why I'm taking issue with some of these rules: They assert
absolute opinions about very subjective patterns.

Anyway, I have no doubt we can find the right balance.


-- 
Dan Duvall
Automation Engineer
Wikimedia Foundation <http://wikimediafoundation.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.wikimedia.org/pipermail/qa/attachments/20141120/e094222f/attachment.html>