1
\$\begingroup\$

Which of these two alternatives is more readable?

token = map.get(token_str)
if not token:
   token = Course(token_str)
return token

or

token = map.get(token_str)
token if token else Course(token_str)
\$\endgroup\$

closed as off-topic by Billal Begueradj, Stephen Rauch, Incomputable, Vogel612 Jul 1 '18 at 10:21

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Billal Begueradj, Stephen Rauch, Incomputable, Vogel612
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ How about token = map.get(token_str) or Course(token_str)? Or token = map.get(token_str, Course(token_str)) if map is a dict? \$\endgroup\$ – abuzittin gillifirca Dec 6 '13 at 12:07
5
\$\begingroup\$

get() takes an optional second parameter, for a value to use if the key isn't present:

token = map.get(token_str, Course(token_str))

I think that one is clearly the best solution, unless you don't want to create and immediately throw away a Course object. In that case, I would use 'or':

token = map.get(token_str) or Course(token_str)

Both of these are only one line, that one line is quite readable, and there is no unnecessary repetition.

There is one third possible situation, in which you don't want to unnecessarily construct a Course instance, and you also can't rely on the values that are present to evaluate as True. Then 'or' wouldn't work correctly.

Then I would use an 'if' statement:

token = map.get(token_str)
if token is None:
    token = Course(token_str)

but that's personal preference. Something using 'in' is also fine:

token = map[token_str] if token_str in map else Course(token_str)

although of course that looks up token_str in map twice, so if you need to be really time efficient then don't do that.

\$\endgroup\$
  • \$\begingroup\$ The first suggestion with the default value isn't quite the behaviour asked for. Eg: {0: False}.get(0, Course(0)) returns False because the truth value of the dict value is never determined. The second suggestion does work though. \$\endgroup\$ – ejrb Dec 6 '13 at 13:53
  • \$\begingroup\$ It's not exactly the same as his if statement, but I assumed that his values would be truthy Course instances. But good point. \$\endgroup\$ – RemcoGerlich Dec 6 '13 at 14:09
2
\$\begingroup\$

If this behavior is implemented at a specific consuming location of your map, I favor token = map.get(token_str) or Course(token_str) over both of your alternatives. This, like your original, assumes that no expected entries will have a False value.

If this is a general need, such that for all token_str the map should contain a Course object, you could make this part of the dict instance itself:

class Courses(dict):
    def __missing__(self, key):
        return Course(key)

map = Courses()

# ...
token = map[token_str]

Note that this handles False values differently from your original code; it only instantiates new Course objects when there wasn't an entry in map.

Side note: try to avoid using builtins such as map as local or global variables. It works fine, but throws readers off. In this case I would suggest a name such as courses.

\$\endgroup\$
1
\$\begingroup\$

In python, a common style is "easier to ask for forgiveness than permission" (http://docs.python.org/2/glossary.html#term-eafp). I.e.

try:
    return map[token_str]
except KeyError:
    return Course(token_str)
\$\endgroup\$
  • 1
    \$\begingroup\$ While this approach is certainly good to know, remember that exceptions aren't cheap. If token_str is almost always in map, this is fine; if it's middle or low odds of being present, this is likely a worse choice. \$\endgroup\$ – Michael Urman Dec 6 '13 at 16:31

Not the answer you're looking for? Browse other questions tagged or ask your own question.