biosimulators/Biosimulators_COPASI

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.