<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Nov 20, 2014 at 9:05 AM, Antoine Musso <span dir="ltr"><<a href="mailto:hashar+wmf@free.fr" target="_blank">hashar+wmf@free.fr</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Le 20/11/2014 01:38, Dan Duvall a écrit :<br>
<span class="">> Ironically enough, it has failed the Travis CI check because of an<br>
> overzealous `GuardClause` check. (Seriously, who can't read an `if`<br>
> statement ... :)<br>
<br>
</span><a href="http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause" target="_blank">http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause</a><br>
<br>
Well, one could name it early return. That saves a level of indentation<br>
and make it obvious that on that condition nothing else is executed.<br>
Else you have to browse down at the end of the condition to look at<br>
potentially additional code.<br>
<br>
In php I definitely prefers:<br>
<br>
if ( ! $cond ) {<br>
return;<br>
}<br>
// lot of code<br>
<br>
Against:<br>
<br>
if ( $cond ) {<br>
// lot of code<br>
}<br>
<br>
But yeah might be overzealous :]<br></blockquote><div><br></div><div>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.</div><div><br></div><div>In other words, changing this ...</div><div><br></div><div><font face="courier new, monospace"> def foo</font></div><div><font face="courier new, monospace"> something1 if condition1</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> something2 unless condition2</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> something3 if condition3</font></div><div><font face="courier new, monospace"> end</font></div><div><br></div><div>... to this ...</div><div><br></div><div><font face="courier new, monospace"> def foo</font></div><div><font face="courier new, monospace"> return unless condition1</font></div><div><font face="courier new, monospace"> something1</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> return if condition2</font></div><div><font face="courier new, monospace"> something2</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> return unless condition3</font></div><div><font face="courier new, monospace"> something3</font></div><div><font face="courier new, monospace"> end</font></div><div><br></div><div>... would alter the code paths to where a falsey condition1 skips something2 and something3. It's actually equivalent to using nested conditions ...</div><div><br></div><div><font face="courier new, monospace"> def foo</font></div><div><font face="courier new, monospace"> if condition1</font></div><div><font face="courier new, monospace"> something1</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> unless condition2</font></div><div><font face="courier new, monospace"> something2</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace"> if condition3</font></div><div><font face="courier new, monospace"> something3</font></div><div><font face="courier new, monospace"> end</font></div><div><font face="courier new, monospace"> end</font></div><div><font face="courier new, monospace"> end</font></div><div><br></div><div>... which is definitely harder to make sense of. :)</div><div><br></div><div>I guess that's why I'm taking issue with some of these rules: They assert absolute opinions about very subjective patterns.</div><div><br></div><div>Anyway, I have no doubt we can find the right balance.</div><div><br></div><div><br></div></div>-- <br><div class="gmail_signature"><div dir="ltr">Dan Duvall<div>Automation Engineer</div><div><a href="http://wikimediafoundation.org" target="_blank">Wikimedia Foundation</a><br></div></div></div>
</div></div>