5

I'm sure this concept has come up before but I can't find a good, simple answer. Is using try/finally a bad way to handle functions with multiple returns? For example I have


try:
    if x:
        return update(1)
    else:
        return update(2)
finally:
    notifyUpdated()

This just seems nicer than storing update() commands in a temporary variable and returning that.

Jonas
  • 121,568
  • 97
  • 310
  • 388
Falmarri
  • 47,727
  • 41
  • 151
  • 191

6 Answers6

11

I wouldn't recommend it. First because notifyUpdated() will be called even if the code in either branch throws an exception. You would need something like this to really get the intended behavior:

try:
    if x:
        return update(1)
    else:
        return update(2)
except:
    raise
else:
    notifyUpdated()

Secondly, because try blocks generally indicate you're doing some kind of exception handling, and you aren't, you're just using them for convenience. So this construct will confuse people.

For example, I don't think either of the first two people (at least one of which deleted their answer) to answer your question realized what you were really trying to do. Confusing code is bad, no matter how convenient and clever it seems.

Omnifarious
  • 54,333
  • 19
  • 131
  • 194
11

I would not use try/finally for flow that doesn't involve exceptions. It's too tricky for its own good.

This is better:

if x:
    ret = update(1)
else:
    ret = update(2)
notifyUpdated()
return ret
Ned Batchelder
  • 364,293
  • 75
  • 561
  • 662
3

I think you mean you want to use try/finally as an alternative to this:

if x:
    result = update(1)
else:
    result = update(2)
notifyUpdated()
return result

I guess this is a matter of style. For me I like to reserve try for handling exceptional conditional. I won't use it as a flow control statement.

Wai Yip Tung
  • 18,106
  • 10
  • 43
  • 47
  • While I guess it's a matter of style, I don't think that means the question does not have a yes or no answer. I would say definitively 'no' for the reason you stated and because it's more confusing. – Omnifarious Aug 16 '10 at 21:25
  • I agree. I should take a stronger stance. – Wai Yip Tung Aug 16 '10 at 21:40
3

I think this is asking for trouble. What happens later, when you change your code to the following?

try:
    if x:
        return update(1)
    elif y:
        return update(2)
    else:
        return noUpdateHere()
finally:
    notifyUpdated() # even if noUpdateHere()!

At best, it's a stumbling point for most readers of your code (probably even you in six months), because it's using try/finally for a purpose that differs from the normal use patterns. And the typing it saves is minimal, anyway.

P Daddy
  • 28,912
  • 9
  • 68
  • 92
  • I missed that stumbling point. That case already exists in hidden form in the original though because if `update(1)` or `update(2)` throw an exception that also probably means `notifyUpdated()` should not be called. – Omnifarious Aug 16 '10 at 21:29
3

I think a decorator is a better idea here

def notifyupdateddecorator(f):
    def inner(*args, **kw):
        retval = f(*args, **kw)
        notifyUpdated()
        return retval
    return inner

@notifyupdateddecorator
def f(x):
    if x:
        return update(1)
    else:
        return update(2)

@notifyupdateddecorator
def g(x):
    return update(1 if x else 2)
John La Rooy
  • 295,403
  • 53
  • 369
  • 502
0

from http://docs.python.org/library/contextlib.html:


from contextlib import closing
import urllib

with closing(urllib.urlopen('http://www.python.org')) as page:
    for line in page:
        print line

so you can create a similar function and use it

akonsu
  • 28,824
  • 33
  • 119
  • 194