Originally published on my blog.
Introduction
As you may know, I use pylint for most of my Python projects. 1
A few weeks ago, I upgraded pylint and a new warning appeared. This tends to happen when you have a pretty large code base: new checks are added to pylint all the time, so new warnings are bound to show up.
Here’s a minimal example of the kind of code that triggered the new warning:
def foo():
if bar:
return baz
else:
return qux
$ pylint foo.py
...
Unnecessary "else" after "return" (no-else-return)
Indeed, the code after the first return will never execute if bar
is true, so there’s no need for the else
.
In other words, the code should be written like this:
def foo():
if bar:
return baz
return qux
Well, the code is shorter. But is it better?
The problem
If you think about it, the question about whether the code is better in the first form (let’s call it explicit else) or in the second form (let’s call it implicit else) is hard to answer because you have no clue about the meaning of the foo
function, or the bar
, baz
and qux
variables.
So let’s try to come up with better examples.
Guard clauses
Sometimes you’ll find code written this way:
def try_something():
if precondition():
result = compute_something()
return result
else:
display_error()
return None
In other words, you are trying to do something but that’s only possible if a condition is true. If the condition is false, you need to display an error.
The else
is explicit here.
The version with an implicit else
looks like this:
def try_something():
if precondition():
result = compute_something()
return result
display_error()
return None
So far, it’s not very clear what version is better.
Note there’s a third way to write the same code, by using if not precondition()
instead:
# Implicit else, inverted condition
def try_something():
if not precondition():
display_error()
return None
result = compute_something()
return result
Now, watch what happens when we add several preconditions:
# Explicit else
def try_something():
if precondition_one():
if precondition_two():
result = compute_something()
return result
else:
display_error_two()
return
else:
display_error_one()
# Implicit else, inverted condition
def try_something():
if not precondition_one():
display_error_one()
return
if not precondition_two():
display_error_two()
return
result = compute_something()
return result
I hope you’ll agree the second version is better.
There’s one less level of indentation, and the line that displays the error is right after the line that checks for the error.
Clear win for the implicit else here.
Symmetric conditions
Let’s take an other example.
Suppose you are writing a script that will check all the links in documentation written as a set of HTML pages.
You’ve got a list of all the possible pages, and then you need to check both internal links (with a href
looking like../other-page
) and external links like (with a href
like http://example.com
).
Let’s take a look at the two variants:
# Implicit else
def check_link(link) -> bool:
if is_internal_link(link):
return check_internal_link(link)
return check_external_link(link)
# Explicit else
def check_link(link) -> bool:
if is_internal_link(link):
return check_internal_link(link)
else:
return check_external_link(link)
This time, I hope you’ll agree the explicit else is better.
There are two things to be done, and visually they are at them at the same level of indentation.
The symmetry between the type of the link and the check that needs to be done is preserved.
We could say that the algorithm I’ve described as text in the last paragraph is better expressed in the second version.
Conclusion
Pylint is a great tool, but be careful before deciding whether you want to follow its refactoring pieces of advice.
Second, make sure your code is easy to read and reveal your intention. Conciseness is not the only factor here.
Last, be careful with code samples that are too abstract :)
Cheers!
Thanks for reading this far :)
I'd love to hear what you have to say, so please feel free to leave a comment below, or read the feedback page for more ways to get in touch with me.
-
I already blogged about this in Some pylint tips and How I lint my Python. ↩
Top comments (7)
Personally I prefer an implicit else for the most (roughly 90%) cases. For example, we need to return payments info for some user. In that case if we check all unusual states the code looks much more readable. We have all the unusual checks on the topmost and then we implement safe and much cleaner program logic:
A better example.
Or... in C#...
Now expand those methods out, incorporating them into the main routine. Gets complicated real fast, eh?
But what if some stuff needs to be valid to continue? Don't nest if you don't have to, and don't split stuff out into too many routines. Test it and get out if it fails. No need to
}else{}else{}else{}
if you can make it readable there.If I’m returning from a condition I never use
else
. If you prefer that style I’d recommend assigning a value instead, e.gInteresting. Who'd knew there would be so many different ways to do the same thing ;)
Your solution is fine unless there's tons of code between
if conf
andresult=expression()
, then it becomes hard to find out there's a concept of "default value" one the first line.A ternary would make even more sense than explicit else in the last case:
Unfortunately, python ternary looks like this:
how do you even format this?
Yeah. As a rule I avoid ternary stuff in Python.
But this is a topic for an other discussion ;)
Don't see you in the comments there, whereas I have already laid my 💩posting.