1

In a class method I have a set of possible options for a single keyword argument, each with a different algorithm to calculate something. To check which option has been added to the keyword I made a chain of if, elif, else too find the keyword option provided.

class MyClass:
    def my_method(self, my_parameter, my_keyword='spacial'):
        if my_keyword == 'spacial':
            print('Cool stuf')
        elif my_keyword == 'discoidal':
            print('OTHER cool stuff')
        elif my_keyword == 'temporal':
            print('You get the gist')
        else:
            print('not in options list')

In my opinion this is not a very elegant way to code this. Especially if the options list keeps growing. Is there a way to omit the list of if, elif, elif, else statements?

Martin Tournoij
  • 26,737
  • 24
  • 105
  • 146
Eljee
  • 267
  • 1
  • 3
  • 10
  • 1
    `=`? Are you sure? And where did the `def` hide? And what are those string literals doing there? ... When posting example code, you should always make it as correct as you can make it. I have edited the code for you now, to what you *probably* intended it to be. – Martin Tournoij Dec 02 '14 at 16:44
  • 1
    Please always post working code... – wenzul Dec 02 '14 at 16:45
  • The canonical replacement for lots of `elif`s is a dictionary `lookup = {'spacial': 'Cool stuf', 'discoidal': 'OTHER cool stuff'}` and then `print(lookup[my_keyword])`, but you may have to refactor into separate functions to implement this with a non-trivial example. – jonrsharpe Dec 02 '14 at 16:47

4 Answers4

2

Use a dictionary:

def cool_stuff(param):
   ...

def other_cool_stuff(param):
   ...

def you_get_the_gist(param):
   ....


dispatch_mapping = {
    'spacial': cool_stuff,
    'discoidal': other_cool_stuff,
    'temporal': you_get_the_gist
}

Somewhere else:

def my_method(self, param, keyword='spacial'):
    handler = dispatch_mapping.get(keyword)
    if handler is None:
        raise Exception("No handler for %s" % keyword)
    return handler(param)
Thomas Orozco
  • 53,284
  • 11
  • 113
  • 116
  • @wenzul It should, I was updating it : ) – Thomas Orozco Dec 02 '14 at 16:49
  • To expand/explain slightly, this takes advantage of the fact that in Python functions are first-class objects; for an explanation, also see: http://stackoverflow.com/questions/245192/what-are-first-class-objects – Martin Tournoij Dec 02 '14 at 16:50
  • For several functions it will also be faster than if-else due to dictionary lookup time complexity O(1). – wenzul Dec 02 '14 at 16:52
  • @wenzul: In practice that is unlikely to be a major concern. Firstly, dictionary lookup with a string key is O(n) in the length of the string (unless Python caches the hash values, which I believe it does). Secondly and more importantly, dictionary lookup has significantly more O(1) overhead than if/else chaining, so you'd probably need at least 10+ functions to see a benefit. – Kevin Dec 02 '14 at 18:51
  • @Kevin Yes I meant at least 10+ functions. :) Otherwise I would also prefer the switch-case. But a dictionary lookup is in an avarage case O(1). It doesn't depend on a string because the string will be hashed. Worst case if we will get collisions it will be O(n) times string comparison. https://wiki.python.org/moin/TimeComplexity – wenzul Dec 02 '14 at 18:55
  • @wenzul: Hashing a string is always linear in the length of the string, but I believe Python caches hash values once computed so you only have to hash any given string once. In the general case, hashing can be an arbitrarily expensive operation, and it is not necessarily valid to assume O(1) dictionary operations. – Kevin Dec 02 '14 at 19:07
  • 1
    @Kevin O(1) means the lookup complexity in the dictionary. Not the complexity of preparing a hash... I agree it's an abstract measurement... – wenzul Dec 02 '14 at 19:19
0

There exists always at least one place where you have to divide the cases.

In your case you set a string and then compare it again.

A "way" around that would be replace setting your string by different function/method calls directly instead of routing it inside a function. Or use a dictionary to map strings to a function call.

wenzul
  • 3,948
  • 2
  • 21
  • 33
0

Using dictionary is a good idea. However, another option is reflection which function be called by string.

class MyClass:
    def handle_spacial(self):
        print('Cool stuf')

    def handle_discoidal(self):
        print('OTHER cool stuff')

    def handle_temporal(self):
        print('You get the gist')

    def default(self):
        print('not in options list')

    def my_method(self, my_parameter, my_keyword='spacial'):
        function_name = "handle_"+my_keyword
        if hasattr(self, function_name):
            getattr(self, function_name)()
        else:
            self.default()
owenwater
  • 811
  • 1
  • 6
  • 18
0

The best way to to create a dictionary of keywords and options to be displayed and use in the following way:

>>> class MyClass:
...    global mykey_option
...    mykey_option={'spacial':'Cool stuf','discoidal':'OTHER cool stuff','temporal':'You get the gist'}
...    def my_method(self, my_parameter, my_keyword='spacial'):
...        try:
...            print(mykey_option[my_keyword])
...        except:
...            print('not in options list') 
... 
>>> x = MyClass()
>>> x.my_method(1,'discoidal')
OTHER cool stuff
>>> x.my_method(1,'spacial')
Cool stuf
>>> x.my_method(1,'temporal')
You get the gist
>>> x.my_method(1,'newstring')
not in options list
Irshad Bhat
  • 8,479
  • 1
  • 26
  • 36
  • Don't use bare `except`. Do `except KeyError`. Otherwise, you'll catch `SystemExit` and other things, which is worse than it sounds if e.g. stdout is a pipe and `print()` blocks. – Kevin Dec 02 '14 at 18:53