6

I just want to know which way is more preferable in python. Imagine two functions:

1 function:

def foo(key):
    if bar.has_key(key):
        return bar.get(key)
    # do something with bar
    # this will be executed if bar_key(key) is False
    ...
    return something

2 function:

def foo(key):
    if bar.has_key(key):
        return bar.get(key)
    else:
        # do something with bar
        # this will be executed if bar_key(key) is False
        ...
        return something

As you can see the only difference is else statement. So the question is will it affect performance somehow. Or are there any reasons to include else in this type of functions?

Vor
  • 27,623
  • 37
  • 119
  • 184
  • 1
    Assuming bar is a `dict`, how about `return bar.get(key,something)` ? – ixe013 Aug 29 '13 at 17:12
  • 1
    Rather opinion based and I don't think it's specific to Python or being "pythonic". From what I've heard the first option is more readable, and I also choose this version. – BartoszKP Aug 29 '13 at 17:16
  • 5
    Neither is; use `if key in bar: return bar[key]`; no need to use `get()` and `has_key()`. – Martijn Pieters Aug 29 '13 at 17:17
  • Thank you @Martijn Pieters I'll follow you suggestion as well – Vor Aug 29 '13 at 17:18
  • Also `try: return bar[key]` `except:...` is commonly used (helps avoid race conditions) – John La Rooy Aug 29 '13 at 17:19
  • 1
    @Vor: You don't need the `else` either, as you already return in the `if` suite. The function ends there, so `else` is redundant really. – Martijn Pieters Aug 29 '13 at 17:21
  • @gnibbler That doesn't really eliminate race conditions, it just makes the window smaller. There is still a race condition in that code. – cdhowie Aug 29 '13 at 17:25
  • Somewhat related: [Should a function have only one return statement?](http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement). In this case, as Martijn mentioned, the `else` is not needed as you return in the `if` anyway, so the `else` will only unnecessarily increase the indentation level, making the code less readable. So between 1 and 2, 1 wins. But you should really use `key in bar` and `bar[key]` for a *pythonic* solution. – poke Aug 29 '13 at 17:25
  • Hi @poke, can you tell more about this? why not? – Vor Aug 29 '13 at 17:27
  • 1
    @Vor: `dict.has_key()` is deprecated in favour of `key in dict`; once you've established that the key is in the dict, there is no need to use `.get()` anymore, just grab the value *directly* using `dict[key]`. – Martijn Pieters Aug 29 '13 at 17:39
  • I actually prefer the `else` version; that way it's clear just from the outermost code structure that these are two mutually exclusive blocks, without having to read what's in the first block. Certainly if the first block had more complicated code inside it (with more control flow constructs) it's nicer to not have to mentally run termination analysis to find out whether the rest of the code should be read in the context of the condition being false. – Ben Aug 29 '13 at 21:02

3 Answers3

7

If the choice is between those two approaches, I would pick the first one. return is pretty explicit that execution terminates at that point. I find if x { return y } else { ... } an anti-pattern for this reason (not just in Python -- I see this in C/C++ code and it irks me there, too).

If you are returning, an else block is entirely unnecessary and causes pointless indentation of a block of code that might be quite large. The more nested structures you have, the more difficult it is to maintain proper context in your head while reading code. For this reason I tend to prefer less nesting when it does not obfuscate logic, and in this case I don't think it would.

cdhowie
  • 133,716
  • 21
  • 261
  • 264
4

The pythonic way:

def foo(key):
    return bar.get(key, something)
Matt Bryant
  • 4,591
  • 4
  • 27
  • 42
  • 1
    Depending on how long it takes to compute the value of `something`, yes. If it takes a non-negligible amount of time, one would be pre-computing a value that is never used. – cdhowie Aug 29 '13 at 17:14
  • 1
    Well, what if I need to do some something let's say call the DB, to retrieve this key, and than update dict? – Vor Aug 29 '13 at 17:14
  • @Vor: If you want to update your dictionary with `something` anyway, you could also just check if `key not in bar` and compute `something` and then set `bar[key] = something` within the then block. Afterwards, just return `bar[key]`. – poke Aug 29 '13 at 17:23
4

While this question is a little opinion based, I'd say the second is more Pythonic for the reason of "explicit is better than implicit". The second function is clearly saying "if this condition, do this. Otherwise, do this". On the other hand, the first function implies the "Otherwise" part.