2

I am trying to reduce cylomatic complexity of code, because according to pylama my definition is 'too complex' and suggested solution includes calling functions with dictionary mappings.

So I tried it on my object oriented code but failed miserably.

class trial:
    def __init__(self):
        self.a = 'a'
        self.b = 'b'

    def a(self):
        return self.a

    def b(self):
        return self.b

    def select_one(self, option):
        map_func = {
        1 : self.a,
        2 : self.b
        }
        return map_func[option]()

t = trial()
print(t.select_one(1))

If this is not possible what are the other possible solutions to reduce cyclomatic complexity.

Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219
Chinmaya B
  • 405
  • 1
  • 7
  • 21
  • 1
    you should define `map_func` once and for all in `__init__` so it's not redone each time, else what's the point of a dictionary – Jean-François Fabre Nov 14 '18 at 20:09
  • 1
    besides that I see no complexity... – Jean-François Fabre Nov 14 '18 at 20:09
  • 1
    also you have functions and strings called the same. Please change that. This is probably why it doesn't work for you – Jean-François Fabre Nov 14 '18 at 20:10
  • Why would it reduce cyclomatic complexity? You introduce a factory function that calls one of two other funcs that you could have called by name yourself? Also: use `dict.get()` or die horribly on "alosdfböoads" given to your mapper. This is almost code obfuscation - who knows what 1 or 2 might do? – Patrick Artner Nov 14 '18 at 20:16
  • @PatrickArtner That was just for example I will change my options from 1,2 to something more sensible, my previous (too complex code had) a single method with if else ladder for choosing an option. – Chinmaya B Nov 14 '18 at 20:26

1 Answers1

3

first, the dictionary should be defined in __init__ or you have O(n) complexity each time you enter your select_one function (dictionary is built each time, which makes the example in your link wrong)

second, your methods have the same name as your attributes. Change that:

class trial:
    def __init__(self):
        self.a = 'a'
        self.b = 'b'
        self.map_func = {
        1 : self.f_a,
        2 : self.f_b
        }

    def f_a(self):
        return self.a

    def f_b(self):
        return self.b

    def select_one(self, option):
        return self.map_func[option]()

t = trial()
print(t.select_one(1))
Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219