[code-quality] pylint: Else clause on a loop without break statement

[Carl Crowder]

I think the point is that Pylint does not only say "this is wrong", but also says "are you sure this is right?". These things are usually warnings but perhaps 'code smell' is a better name. Because, in this case, the 'else' isn't strictly necessary, Pylint (correctly, in my opinion) raises a warning which effectively says "This 'else' clause does not actually need to be there - did you do it on purpose, or have some break statements been refactored away or something?". I consider it more like a code review, in which the reviewer tentatively asks "this looks odd - is it deliberate?" simply to verify in case it was not. 

It's pretty easy to suppress the warning either on this line alone or on the entire project if this is your code style, so I prefer the case where Pylint catches a real error but may be to hasty for some users. I think Pylint should be regarded as producing both actual errors as output but also advisories and questions.

On 12.01.2014, at 18:45, Peter Ludemann <pludemann at google.com> wrote:

> def _areConstants(expressions):
>     return all(_isConstant(expression) for expression in expressions)
> 
> def _isConstant(expression):
>     return expression.isExpressionConstantRef() and not expression.isMutable()
> 
> ?
> 
> When I tried to understand the OP's code, I had to mentally step through the code and assemble the meaning, whereas the above code requires no such work (it probably takes a bit longer per-line, but less time over-all).
> 
> If you're asking about whether pylint should complain about the OP's code, I think that it shouldn't -- in the semantics of for/else, return and break are similar and rewriting OP's code to suppress the warnings is not an improvement:
> 
> def _areConstants(expressions):
>     for expression in expressions:
>         if not expression.isExpressionConstantRef():
>             break
>         if expression.isMutable():
>             break
>     else:
>         return True
>     return False
> 
> (Caveat: none of this code has been tested)
> 
> 
> 
> 
> On 12 January 2014 07:21, Ian Cordasco <graffatcolmingov at gmail.com> wrote:
> Woops. Sorry for the confusion. I never end up using that particular
> construct so I never get it right.
> 
> On Sun, Jan 12, 2014 at 9:18 AM, Carl Crowder <carl.crowder at gmail.com> wrote:
> > Ian, it's the opposite - 'else' is only triggered if *no* break statement is
> > encountered.
> >
> >
> > On 12 January 2014 16:12, Ian Cordasco <graffatcolmingov at gmail.com> wrote:
> >>
> >> I don't see any mesage from pylint in your email, could you post it again?
> >>
> >> Regardless, I think what you're disagreeing with is the language
> >> specification. The documentation specifies that the `else` is only
> >> triggered on breaks:
> >>
> >> http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops
> >>
> >> On Sun, Jan 12, 2014 at 8:57 AM, Kay Hayen <kay.hayen at gmail.com> wrote:
> >> >
> >> > Hello,
> >> >
> >> > often I write code like this:
> >> >
> >> > def _areConstants(expressions):
> >> >     for expression in expressions:
> >> >         if not expression.isExpressionConstantRef():
> >> >             return False
> >> >
> >> >         if expression.isMutable():
> >> >             return False
> >> >     else:
> >> >         return True
> >> >
> >> > That is to search in an iterable, and return based on finding something,
> >> > or
> >> > returning in the alternative, I specifically prefer the "else:" branch
> >> > over
> >> > merely putting it after the "for" loop.
> >> >
> >> > This triggers the above message, which I consider flawed, because as
> >> > soon as
> >> > there is a "raise", or "return", that should be good enough as well.
> >> > Basically any aborting statement, not only break.
> >> >
> >> > I wanted to hear your opinion on this, pylint bug, or only in my mind.
> >> >
> >> > Yours,
> >> > Kay
> >> > _______________________________________________
> >> > code-quality mailing list
> >> > code-quality at python.org
> >> > https://mail.python.org/mailman/listinfo/code-quality
> >> _______________________________________________
> >> code-quality mailing list
> >> code-quality at python.org
> >> https://mail.python.org/mailman/listinfo/code-quality
> >
> >
> _______________________________________________
> code-quality mailing list
> code-quality at python.org
> https://mail.python.org/mailman/listinfo/code-quality
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/code-quality/attachments/20140112/c291c14f/attachment-0001.html>