DEV Community

Dimitri Merejkowsky
Dimitri Merejkowsky

Posted on • Originally published at dmerej.info on

Else after return: yea or nay?

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
Enter fullscreen mode Exit fullscreen mode
$ pylint foo.py
...
Unnecessary "else" after "return" (no-else-return)

Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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()
Enter fullscreen mode Exit fullscreen mode
# 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
Enter fullscreen mode Exit fullscreen mode

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)
Enter fullscreen mode Exit fullscreen mode

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.


  1. I already blogged about this in Some pylint tips and How I lint my Python

Top comments (7)

Collapse
 
sur0g profile image
sur0g

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:

def return_payments(user, connection):

    if not user:
        raise UserNotFoundError()

    if not connection:
        raise ConnectionError()

    ...  # actual _safe_ logic goes here

Collapse
 
strredwolf profile image
STrRedWolf

A better example.

def addticket(subject, detail):
    if already_exists(subject):
        appendticket(subject, detail)
        return

    createticket(subject,detail)

Or... in C#...

void addticket(string subject, string detail) {
    if(already_exists(subject)) {
        appendticket(subject,detail);
        return;
    }

    createticket(subject,detail);
}

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.

Collapse
 
nestedsoftware profile image
Nested Software • Edited

If I’m returning from a condition I never use else. If you prefer that style I’d recommend assigning a value instead, e.g

result = None

if cond:
    result = expression()

return result
Collapse
 
dmerejkowsky profile image
Dimitri Merejkowsky • Edited

Interesting. 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 and result=expression(), then it becomes hard to find out there's a concept of "default value" one the first line.

Collapse
 
qm3ster profile image
Mihail Malo

A ternary would make even more sense than explicit else in the last case:

def check_link(link) -> bool:
    return if is_internal_link(link) {
            check_internal_link(link)
        } else {
            check_external_link(link)
        }
def check_link(link) -> bool:
    return is_internal_link(link)
        ? check_internal_link(link)
        : check_external_link(link)

Unfortunately, python ternary looks like this:

def check_link(link) -> bool:
    return check_internal_link(link) if is_internal_link(link) else check_external_link(link)

how do you even format this?

def check_link(link) -> bool:
    return check_internal_link(link)
        if is_internal_link(link) # this is the condition for the above lol
        else check_external_link(link)
Collapse
 
dmerejkowsky profile image
Dimitri Merejkowsky

Yeah. As a rule I avoid ternary stuff in Python.

But this is a topic for an other discussion ;)

Collapse
 
qm3ster profile image
Mihail Malo

Don't see you in the comments there, whereas I have already laid my 💩posting.