Lowering Cognitive complexity as per SonarCloud
AlexPatrie opened this issue · 0 comments
Justification
After several attempts of pushing type annotations to biosimulators_copasi with a PR, I have been repeatedly met with an error from SonarCloud which indicates that a given function’s “cognitive complexity” is above the acceptable threshold. The documentation specifies that it will assess a negative mark for a variety of statements and situations including: nesting, recursion, and any statement that includes if/else/while/except/and/or/is/not. Exceptions to this rule are made with try/finally blocks and boolean dictionaries.
Examples:
For example, I was able to devise a solution that satisfied the bot while still returning the desired result for the optional keyword argument “config”, where the default is None:
Originally: (+1 cognitive complexity)
config = config or get_config()
Revised: (+0 cognitive complexity)
config = {True: config, False: get_config()}.get(bool(config), config)
Clearly, this solution is neither scalable nor ‘Pythonic’ in nature. Another source of SonarCloud contention pertains to nested-if statements whose depth of nesting is > 1. For example:
` for change in model.changes:
target_sbml_id = model_change_target_sbml_id_map[change.target]
copasi_model_obj = get_copasi_model_object_by_sbml_id(copasi_model, target_sbml_id, units)
if copasi_model_obj is None:
invalid_changes.append(change.target)
else:
model_obj_parent = copasi_model_obj.getObjectParent()
if isinstance(model_obj_parent, COPASI.CCompartment):
set_func = model_obj_parent.setInitialValue
ref = model_obj_parent.getInitialValueReference()
elif isinstance(model_obj_parent, COPASI.CModelValue):
set_func = model_obj_parent.setInitialValue
ref = model_obj_parent.getInitialValueReference()
elif isinstance(model_obj_parent, COPASI.CMetab):
if units == Units.discrete:
set_func = model_obj_parent.setInitialValue
ref = model_obj_parent.getInitialValueReference()
else:
set_func = model_obj_parent.setInitialConcentration
ref = model_obj_parent.getInitialConcentrationReference()
model_change_obj_map[change.target] = (set_func, ref)`
Possible solutions:
One effective solution to this could include a guard clause. The proposal at hand involves refactoring nested conditional statements whose traversal depth is > 1. The above code is a perfect example of this cognitive complexity. If setup required python >= 3.10, then the implementation of match/case statements in place of ternary-if statements would satisfy the requirements of this issue.