views:

130

answers:

5

I have this code here:

import re
def get_attr(str, attr):
    m = re.search(attr + r'=(\w+)', str)
    return None if not m else m.group(1)

str = 'type=greeting hello=world'

print get_attr(str, 'type')   # greeting    
print get_attr(str, 'hello')  # world
print get_attr(str, 'attr')   # None

Which works, but I am not particularly fond of this line:

return None if not m else m.group(1)

In my opinion this would look cleaner if we could use a ternary operator:

return (m ? m.group(1) : None)

But that of course isn't there. What do you suggest?

A: 
return m and m.group(1)

would be one Pythonic way to do it.

If m is None (or something else that evaluates "falsely"), it returns m, but if m is "true-ish", then it returns m.group(1).

Mark Rushakoff
I feel like this is trying to be too hard to be clever.
Daenyth
I am [astonished](http://en.wikipedia.org/wiki/Principle_of_least_astonishment).
NullUserException
Actually, if expressions have superseded the clever use of the fact that boolean operators return their operands.
delnan
Agreed. It works, but it kind of feels like an abuse of the `and` operator. Oddly, I don't feel the same way about the `or` operator (e.g. `return x or 'NO VALUE'`)
Chris B.
A: 

From other discussion at SO, this seems the correct way out.

pyfunc
+8  A: 

Python has a ternary operator. You're using it. It's just in the X if Y else Z form.

That said, I'm prone to writing these things out. Fitting things on one line isn't so great if you sacrifice clarity.

def get_attr(str, attr):
    m = re.search(attr + r'=(\w+)', str)
    if m:
        return m.group(1)

    return None
Chris B.
+2  A: 

What you have there is python's conditional operator. IMO it's perfectly pythonic as-is and needs no change. Remember, explicit is better than implicit. What you have now is readable and instantly understandable.

Daenyth
+5  A: 

Another option is to use:

return m.group(1) if m else m

It's explicit, and you don't have to do any logic puzzles to understand it :)

unutbu
How is that more explicit than return None? Who knows what `m` is there? What if m is actually '' or 0 or some custom class that evaluates to False
Falmarri