Doing something on the value only if key exists in dictionary

Question

I have the following snippets, among which I cannot find the clear winner. Which style would you recommend? Is there anything better than these?

(assuming self.task.flags is a dictionary)

1. in and []

   if 'target_names' in self.task.flags:
       foo(self.task.flags['target_names'])
   

   This mentions the names of the dictionary and the key twice (violating DRY).

   I can also imagine that this does the identical lookup twice internally, throwing away the value in the first case (in).

2. get and temporary variable

   value = self.task.flags.get('target_names')
   if value:
       foo(value)
   

   This removes the repetition, but a temporary variable is not very elegant.

   The meaning is also subtly different from the others, when {'target_names': None} is present in self.task.flags.

3. exceptions ("EAFP")

   try:
       foo(self.task.flags['target_names'])
   except KeyError:
       pass
   

   I hear that EAFP is generally encouraged in Python, but I still fear that exceptions can be expensive, especially when the chance of the key being present is not very high.

   This may also mask any exceptions raised inside foo.

   And the try…except structure can be visually distracting; even more when other exception handling is involved.

Answer 1

ugly way:

# with impossibleValue being anything that can't possibly be one of your 
# values, like self.task.flags or an Object created here and now, like i'll do now.
impossibleValue = Object()

# dict.get(key, defaultValue) returns dict[key] if key 
# is in dict, otherwise returns defaultValue.
value = self.task.flags.get('target_names', impossibleValue)
if value != impossibleValue:
    foo(value)

Answer 2

Really, it's up to you. No substantial performance differences, the main worry is just be careful with the second form. If your values are null, and so they don't trigger the "if/then" clause, you will falsely assume there is no key.

a = {k:k**2 for k in range(1000000)}
def nullfunc(*args, **kwargs):
    del args, kwargs

def infunc(a):
    if 5000 in a:
        nullfunc(a[5000])

def getvalue(a):
    value = a.get(5000)
    if value:
        nullfunc(value)

def tryexcept(a):
    try:
        nullfunc(a[5000])
    except KeyError:
        pass


In [12]: %timeit infunc(a)
1000000 loops, best of 3: 238 ns per loop

In [13]: %timeit getvalue(a)
1000000 loops, best of 3: 256 ns per loop

In [14]: %timeit tryexcept(a)
1000000 loops, best of 3: 197 ns per loop

In [15]: 

Answer 3

Both the first and the third options make sense to me. I prefer to reserve try/except for cases that are actually exceptional, and not expected results (user error for example). If it makes sense for the flag to not be there a lot of the time, I would personally prefer case 1. It may also make sense to do the inverse of it if there is no code to be executed if the flag is not there, i.e.:

def foo(self, some_other_args):
    ... do something ...
    if 'target_names' not in self.task.flags:
        # Nothing further to do here, so return the result
        return result

    ... do something with self.task.flags['target_names'] ...
    return result

However I know some people like to avoid multiple returns like that. If you truly expect 'target_names' to be present in the dictionary, I think your case 3 is probably the better solution, because it makes the intent clear (that the missing key is truly exceptional, not the standard case).

Answer 4

The third way, since as you said it conforms to EAFP.

Note that the try/except clause can be written more succintly in this case (Python 3.4+):

import contextlib
with contextlib.suppress(KeyError):
    foo(self.task.flags['target_names'])