views:

139

answers:

7

I had a function that returned a random member of several groups in order of preference. It went something like this:

def get_random_foo_or_bar():
    "I'd rather have a foo than a bar."

    if there_are_foos():
        return get_random_foo()

    if there_are_bars():
        return get_random_bar()

    raise IndexError, "No foos, no bars"

However, the first thing get_random_foo does is verify there are foos and raise an IndexError if not, so there_are_foos is redundant. Moreover, a database is involved and using separate functions creates a concurrency issue. Accordingly, I rewrote it something like this:

def get_random_foo_or_bar():
    "Still prefer foos."

    try:
        return get_random_foo()
    except IndexError:
        pass

    try:
        return get_random_bar()
    except IndexError:
        pass

    raise IndexError, "No foos, no bars"

But I find this much less readable, and as I've never had reason to use pass before it feels instictively wrong.

Is there a neater efficient pattern, or should I learn to accept pass?

Note: I'd like to avoid any nesting since other types may be added later.


Edit

Thanks everyone who said that pass is fine - that's reassuring!

Also thanks to those who suggested replacing the exception with a return value of None. I can see how this is a useful pattern, but I would argue it's semantically wrong in this situation: the functions have been asked to perform an impossible task so they should raise an exception. I prefer to follow the behaviour of the random module (eg. random.choice([])).

+1  A: 

If it's just those two, could always just...

try:
    return get_random_foo()
except IndexError:
    try:
        return get_random_bar()
    except IndexError:
        raise IndexError, "No foos, no bars"

If it's more than two, what you have written seems perfectly acceptable.

Amber
+10  A: 

That is exactly how I would write it. It's simple and it makes sense. I see no problem with the pass statements.

If you want to reduce the repetition and you anticipate adding future types, you could roll this up into a loop. Then you could change the pass to a functionally-equivalent continue statement, if that's more pleasing to your eyes:

for getter in (get_random_foo, get_random_bar):
    try:
        return getter()
    except IndexError:
        continue  # Ignore the exception and try the next type.

raise IndexError, "No foos, no bars"
John Kugelman
+2  A: 

pass is fine (there's a reason it's in the language!-), but a pass-free alternative just takes a bit more nesting:

try: return get_random_foo()
except IndexError:
    try: return get_random_bar()
    except IndexError:
        raise IndexError "no foos, no bars"

Python's Zen (import this from the interactive interpreter prompt) says "flat is better than nested", but nesting is also in the language, for you to use when you decide (presumably being enlightened) that you can do better than that wise koan!-) (As in, "if you meet the Buddha on the road"...).

Alex Martelli
+2  A: 

It looks a little weird to me that get_random_foo() is raising an IndexError when it doesn't take an index as a param (but it might make more sense in context). Why not have get_random_foo(), or a wrapper, catch the error and return None instead?

def get_random_foo_wrapper():
    try:
        return get_random_foo()
    except IndexError:
        return None

def get_random_foo_or_bar():
    "I'd rather have a foo than a bar."

    return get_random_foo_wrapper() or get_random_bar_wrapper() or None

Edit: I should mention that if foo & bar are objects that may evaluate to False (0 or '' say) then the or comparison will skip over them which is BAD

Peter Gibson
I took my cue from the random module - try running `random.choice([])`. Interesting solution though, thanks!
Ian Mackinnon
A: 

Building on Peter Gibson's suggestion, you could create a generic wrapper function that swallows a given exception. And then you could write a function that returns such a generic wrapper for a provided exception. Or heck, for a provided list of exceptions.

def maketrap(*exceptions):
    def trap(func, *args, **kwargs):
        try:
            return func(*args, **kwargs)
        except exceptions:
            return None
    return trap

def get_random_foo_or_bar():
    mytrap = maketrap(IndexError)
    return mytrap(get_random_foo) or mytrap(get_random_bar) or None
kindall
A: 

If you don't really need the exception message (just the type):

def get_random_foo_or_bar():
    try:
        return get_random_foo()
    except IndexError:
        return get_random_bar()    # if failing at this point,
                                   # the whole function will raise IndexError
Santa
I think I would prefer to raise the error explicitly from `get_random_foo_or_bar` rather than relying on the error happening to be the same and needing a comment. Also that allows me to return an appropriate exception message from `get_random_foo_or_bar`.
Ian Mackinnon
A: 

Is it necessary that get_random_foo/bar() raise an IndexError if it's unable to succeed?

If they returned None, you could do:

def get_random_foo_or_bar():
    return get_random_foo() or get_random_bar()
Russell Borogove