Reducing cyclomatic complexity of object oriented python code
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.
python python-3.x cyclomatic-complexity
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
add a comment |
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.
python python-3.x cyclomatic-complexity
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
1
you should definemap_funconce 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: usedict.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
add a comment |
2
2
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.
python python-3.x cyclomatic-complexity
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
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.
python python-3.x cyclomatic-complexity
python python-3.x cyclomatic-complexity
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
edited Nov 14 '18 at 20:33
Jean-François Fabre
104k955112
104k955112
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
asked Nov 14 '18 at 20:05
Chinmaya BChinmaya B
2351516
2351516
1
you should definemap_funconce 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: usedict.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
add a comment |
1
you should definemap_funconce 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: usedict.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
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
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
1
besides that I see no complexity...
– Jean-François Fabre
Nov 14 '18 at 20:09
besides that I see no complexity...
– Jean-François Fabre
Nov 14 '18 at 20:09
1
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
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
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
@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
add a comment |
1 Answer
1
active
oldest
votes
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))
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
Your code throws errorTraceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable
– Chinmaya B
Nov 14 '18 at 20:23
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53307984%2freducing-cyclomatic-complexity-of-object-oriented-python-code%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
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))
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
Your code throws errorTraceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable
– Chinmaya B
Nov 14 '18 at 20:23
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
add a comment |
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))
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
Your code throws errorTraceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable
– Chinmaya B
Nov 14 '18 at 20:23
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
add a comment |
3
3
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))
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
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))
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
answered Nov 14 '18 at 20:13
Jean-François FabreJean-François Fabre
104k955112
104k955112
Your code throws errorTraceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable
– Chinmaya B
Nov 14 '18 at 20:23
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
add a comment |
Your code throws errorTraceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable
– Chinmaya B
Nov 14 '18 at 20:23
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
Your code throws error
Traceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable– Chinmaya B
Nov 14 '18 at 20:23
Your code throws error
Traceback (most recent call last): File "function_call_dict.py", line 20, in <module> print(t.select_one(1)) return map_func[option]() TypeError: 'str' object is not callable– Chinmaya B
Nov 14 '18 at 20:23
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
weird, as I tested it as is. Are you sure you copied exactly?
– Jean-François Fabre
Nov 14 '18 at 20:27
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
it's explained in my comments & my answer already.
– Jean-François Fabre
Nov 14 '18 at 20:28
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
@PatrickArtner Yeah it works
– Chinmaya B
Nov 14 '18 at 20:29
add a comment |
draft saved
draft discarded
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53307984%2freducing-cyclomatic-complexity-of-object-oriented-python-code%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
you should define
map_funconce 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