
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@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@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@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@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-statem...
On Sun, Jan 12, 2014 at 8:57 AM, Kay Hayen <kay.hayen@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@python.org https://mail.python.org/mailman/listinfo/code-quality
_______________________________________________ code-quality mailing list code-quality@python.org https://mail.python.org/mailman/listinfo/code-quality
_______________________________________________ code-quality mailing list code-quality@python.org https://mail.python.org/mailman/listinfo/code-quality