4

I have a series of conditions which can, in try "pythonic style" only be tested by exception:

def getGrok(x):
    try:  # is x referring to a snood?
        s = makeSnood(x)
        grokster = s.snoodidle()
    except ValueError:
        try:  # is x referring to a trund?
            t = makeTrund(x)
            grokster = t.trundle()
        except ValueError:
            try:  # maybe too deep?
                d = makeTooDeep(x)
                grokster = d.groan()
            except ValueError:
                grokster = None

     if grokster:
         return grokster.grok()
     else:
         return None

Here there were only three possibilities and already its getting too deep.

Makes me think there has to be a better way. How can you do this logic without this nested tree?

GreenAsJade
  • 14,459
  • 11
  • 63
  • 98
  • The better way is to know what you have in the first place and have more than one function (one for each kind of thing) – Chad S. Jan 01 '16 at 22:39
  • I think the problem is in the premise. You shouldn't need to try one thing after another, you should know ahead of time what to do. I know Python style is to prefer [EAFP](http://stackoverflow.com/questions/11360858/what-is-the-eafp-principle-in-python), but you can see that it breaks down after a couple of possibilities. – Mark Ransom Jan 01 '16 at 22:44
  • The whole point of a function like this is that it can handle a descriptor of different types of things and produce the right output depending on what type of thing is described. Note that it isn't that I don't know what to do, the problem is that there isn't a means to ask "what type of thing does x describe", other than by trying to use it to access each type of thing – GreenAsJade Jan 01 '16 at 23:10

7 Answers7

2

In your example, it doesn't appear that the later tries really need to be nested inside the earlier ones. You're basically just making three separate attempts. They have an order, but they don't have to be nested. So you can just do something like

grokster = None
try:  # is x referring to a snood?
    s = makeSnood(x)
    grokster = s.snoodidle()
except ValueError:
    pass

if grokster is None:
    try:  # is x referring to a trund?
        t = makeTrund(x)
        grokster = t.trundle()
    except ValueError:
        pass

if grokster is None:
    try:  # is x referring to a trund?
        d = makeTooDeep(x)
        grokster = d.groan()
    except ValueError:
        pass

if grokster:
     return grokster.grok()
else:
     return None

If you cannot "recover" from the case where all attempts fail, then instead of return None at the end you would do something like raise GroksterError("Don't know how to make grokster of x").

Of course, if you have a lot of these, you can probably factor out some of this logic, for instance by having a list of the "creators" (makeSnood, makeTrund, etc.) and their corresponding methods (snoodidle, trundle, etc.), so that you can loop over this list and run the same logic for each attempt. However, this may make the code more confusing for a simple case like this (with only three things to try). Also, in practice I find that if you're doing heterogenous attempts like this, often your handling of them is heterogenous too (e.g., you may be doing different things depending on which attempt succeeds); in that case, for a general solution, you have to split things up even more by pulling the "setup" for each attempt out into a separate function so you can call it.

BrenBarn
  • 242,874
  • 37
  • 412
  • 384
  • Thanks, this is the simple insight I was looking for - they don't need to be nested! Sometimes the right answer is simple and plain and right in your face... and you know it's right when the resulting code is easy to read. – GreenAsJade Jan 01 '16 at 22:55
2

Create separate functions …

def f1(x):
    s = makeSnood(x)
    return s.snoodidle()

def f2(x):
    t = makeTrund(x)
    return t.trundle()

def f3(x):
    d = makeTooDeep(x)
    return d.groan()

… and try them one by one until you have no exception.

def getGrok(x):
    grokster = None
    for f in (f1, f2, f3):
        try:
            grokster = f(x)
            break
        except ValueError:
            pass
    return grokster.grok() if grokster else None
dlask
  • 8,776
  • 1
  • 26
  • 30
1

You could store the functions in an iterable, then use a separate function that accepts the functions to call as an argument:

def make_thing(x, func, method_name):
    obj = func(x)
    return getattr(obj, method_name)()


funcs = [(makeSnood, 'snoodle'), (makeTrund, 'trundle'), (makeTooDeep, 'groan')]
for func, method_name in funcs:
    try:
        make_thing(x, func, method_name)
    except ValueError:
        pass
    else:
        break
Daniel Roseman
  • 588,541
  • 66
  • 880
  • 895
1

The if check for None is redundant, just try returning and if None or successful you will return None:

def getGrok(x):
    try: 
        return  makeSnood(x).snoodidle().grok()
    except ValueError:
        pass
    try:  
        return makeTrund(x).trundle().grok()
    except ValueError:
        pass    
    try: 
        return makeTooDeep(x).groan().grok()
    except ValueError:
        pass
    return None

You end up calling grok if any of the calls succeed so you may as well just return and call in the try.

Or like Daniel's answer use getattr and move the logic into your function trying to return each time:

def getGrok(x):
    for f, m in [(makeSnood, 'snoodle'), (makeTrund, 'trundle'), (makeTooDeep, 'groan')]:
        try:
            return getattr(f(x), m)()
        except ValueError:
            pass
    return None
Padraic Cunningham
  • 176,452
  • 29
  • 245
  • 321
  • 1
    The "return from within the try" did occur to me, and is elegant to read. My artificial example doesn't have significant common logic to be done after the decision about what kind of thing x is pointing to - but my real life example does, amd so I can't just return. But I can see that I could bundle it up into doCommonStuff() and then use the above. It forces an extra level of method call that feels a bit unfortunate, but certainly could be the right thing. There's a certain "amount of logic after the decision", where it just seems right to have it all inline, a-la-accepted soltiuon – GreenAsJade Jan 01 '16 at 23:12
  • @GreenAsJade, ah ok, that makes sense. – Padraic Cunningham Jan 01 '16 at 23:14
  • This looks good, but the logic has change a bit: A ValueError in grok() was handled different in the original code. In the original an ValueError in grok() was not catched. – guettli Jul 29 '16 at 10:01
0

If I wanted to make this compact, but to confuse my future self and co-workers, I would make some higher-level functions and do it like this:

def isThing(x, function, get_val):
    t = function(x)
    return get_val(t)

def getGrok(x):
    funcs = [(makeSnood, lambda s: s.snoodidle()), (makeTrund, lambda t: t.trundle()), (makeTooDeep, lambda d = d.groan())]
    grokster = None
    for func, get_val in funcs:
        try:  # is x referring to a snood?
            grokster = isThing(x, func, get_val)
            break
        except ValueError:
            continue

     if grokster:
         return grokster.grok()
     else:
         return None

Or something like that.

Will Richardson
  • 7,780
  • 7
  • 42
  • 56
0

Here is my simplified version.

def getGrok(x):
    grokster=get_grokster(x):
    if not grokster:
        return
    return grokster.grok()

def get_grokster(x):
    try:  # is x referring to a snood?
        return makeSnood(x).snoodidle()
    except ValueError:
        pass

    try:  # is x referring to a trund?
        return makeTrund(x).trundle()
    except ValueError:
        pass

    try:  # maybe too deep?
        return makeTooDeep(x).groan()
    except ValueError:
        pass

This reduces the whitespace area a lot.

I like Guard Clauses to reduce indentation. Unfortunately it's an unknown pattern. Maybe it's too easy - geeks prefer complicated stuff :-)

guettli
  • 25,042
  • 81
  • 346
  • 663
-1
funcs = [(makeSnood, 'snoodidle'), (makeTrund, 'snoodidle'), (makeTooDeep, 'groan')]
grokster = None

while grokster is None and len(funcs) >0:
    f = funcs.pop()
    try:
        y = f[0](x)
        grokster = getattr(y, f[1])()
    except ValueError:
        pass
if grokster:
    return grokster.grok()
return None
kia
  • 828
  • 5
  • 12