How to reduce cyclomatic complexity in your code?

March 14, 20160

– What’s the Object Oriented way to become wealthy?

– Inheritance

Probably every programmer during writing his code didn’t mention how his methods and classes had grown to huge sizes. Striving for locating the whole complex algorithm in one place, we forget about growing cyclomatic complexity. By the way, what is cyclomatic complexity? This is roughly the number of different paths in your source code and there are two ways in java to begin a new path, either by calling a method or by encountering the following keywords: if, while, repeat, for, &&, ||, catch, case, etc
For example let’s take a code with increased number of cyclomatic complexity:

  @Transactional
  public String isWorkstationAllowed(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess) {

    terminalConfig = terminalConfigDao.findById(terminalConfig.getId());
    Workstation workstation = terminalConfig.getWorkstation();

    if (workstation != null) {
      logger.info("Checking if workstation[LID={}] is allowed to do {}", workstation.getLogicalId(), activeProcess);
      MovementControlSubarea subarea = (workstation.getActiveProcessConfig() != null ? workstation.getActiveProcessConfig()
                                        .getDestinationSubarea()
                                        : null);
      if (subarea != null) {
        if (workstation.getActiveProcessConfig().getProcess().getProcess().equals(activeProcess)) {
          ReceivingSession session = receivingSessionHelper.getSession(processDefinitionKey, terminalConfig.getId());
          if (session != null && session.getExecutionId() == null

            ) {
            if (getExecutionId(processDefinitionKey, terminalConfig.getId()) != null) {
              //delete process instance
            }
          }

          //log info and return null
        }
        else // Check if this terminal still has a processInstance, if so continue. If not we can quit.
        {
          if (getProcessInstance(processDefinitionKey, workstation.getId()) != null) {
             //log info and return null
          }
          else {
            //log info and return string
          }
        }
      }
      else {
// If a receiving workstation is still active, allow to continue
        logger.info("No active Process found for Workstation[LID={}], checking if it has still active work.", workstation.getLogicalId());
        if (terminalConfig.getId() != null) {
          ReceivingSession session = receivingSessionHelper.getSession(processDefinitionKey, terminalConfig.getId());
          if (session != null && session.getExecutionId() != null

            ) {
if (receivingServiceBacking.isWorkstationActive(session.getExecutionId())) {
// Workstation has active work, allow to continue.
            }
            else {
// Workstation doesn't have active work, delete process instance.
            }
          }
        }
        //log info and return string
      }
    }
    else {
      //no workstation found
    }
  }

sonarIssueSmall

Cyclomatic complexity for this method is 19 and it’s too complex. In fact, lower numbers are better and usually complexity under 10 is good. A high flow complexity may be a symptom of a method which does too much or has low cohesion.
We can extract methods for increasing readability of code by using Alt+Shift+M combination in most of IDEs, but the problem of cyclomatic complexity will be left unchanged:

@Transactional
  public String isWorkstationAllowed(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess) {

    terminalConfig = terminalConfigDao.findById(terminalConfig.getId());
    Workstation workstation = terminalConfig.getWorkstation();

    if (workstation != null) {
      logger.info("Checking if workstation[LID={}] is allowed to do {}", workstation.getLogicalId(), activeProcess);
      MovementControlSubarea subarea = (workstation.getActiveProcessConfig() != null ? workstation.getActiveProcessConfig()
                                        .getDestinationSubarea()
                                        : null);
      if (subarea != null) {
        return subAreaNotNull(processDefinitionKey, terminalConfig,
                              activeProcess, workstation);
      }
      else {
        return subareaNull(processDefinitionKey, terminalConfig,
                           activeProcess, workstation);
      }
    }
    else {
      logger.info("No workstation found for Terminal[ID={}]", terminalConfig.getId());
      return "No Workstation found for Terminal[" + terminalConfig.getId() + "]";
    }
  }

How to resolve this problem? Generally, you should make methods simpler or break them up. But what should we do if that’s not possible in your situation? My solution for this problem is to use the power of object-oriented paradigm of Java programming language. We could use Strategy pattern for code refactoring.

 

uml-diagram

Strategy software design pattern manages behavior of an object by changing its internal state. Outside it looks like the object is changing itself. The common way to model state is to create a state variable inside object, which keeps different states, and to handle them in conditional statements.
For transforming state diagram into programming code, you should perform a few steps:

  1. 1) Gather all states;
  2. 2) Create the variable for keeping the current state and define values for each possible state;
  3. 3) Gather all actions which can be executed in the system.
  @Transactional
  public String isWorkstationAllowed(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess) {

    terminalConfig = terminalConfigDao.findById(terminalConfig.getId());
    Workstation workstation = terminalConfig.getWorkstation();

    if (workstation != null) {
      logger.info("Checking if workstation[LID={}] is allowed to do {}", workstation.getLogicalId(), activeProcess);
      MovementControlSubarea subarea = (workstation.getActiveProcessConfig() != null ? workstation.getActiveProcessConfig()
                                        .getDestinationSubarea()
                                        : null);
      Context context = new Context(processDefinitionKey, terminalConfig, activeProcess, subarea);
      return context.action();
    }
    else {
      logger.info("No workstation found for Terminal[ID={}]", terminalConfig.getId());
      return "No Workstation found for Terminal[" + terminalConfig.getId() + "]";
    }
  }

  interface State {

    String action();
  }

  abstract class AbstractState implements State {

    protected String processDefinitionKey;
    protected TerminalConfig terminalConfig;
    protected WMSProcessEnum activeProcess;
    private Workstation workstation;

    private AbstractState(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess) {
      this.processDefinitionKey = processDefinitionKey;
      this.terminalConfig = terminalConfig;
      this.activeProcess = activeProcess;
    }

    public String getProcessDefinitionKey() {
      return processDefinitionKey;
    }
    public void setProcessDefinitionKey(String processDefinitionKey) {
      this.processDefinitionKey = processDefinitionKey;
    }
    public TerminalConfig getTerminalConfig() {
      return terminalConfig;
    }
    public void setTerminalConfig(TerminalConfig terminalConfig) {
      this.terminalConfig = terminalConfig;
    }
    public WMSProcessEnum getActiveProcess() {
      return activeProcess;
    }
    public void setActiveProcess(WMSProcessEnum activeProcess) {
      this.activeProcess = activeProcess;
    }
    public Workstation getWorkstation() {
      return workstation;
    }
    public void setWorkstation(Workstation workstation) {
      this.workstation = workstation;
    }
  }

  class SubareaIsNotNull extends AbstractState {

    private Context context;
    private SubareaIsNotNull(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess) {
      super(processDefinitionKey, terminalConfig, activeProcess);
    }

    @Override
    public String action() {
      if (getWorkstation().getActiveProcessConfig().getProcess().getProcess().equals(activeProcess)) {
        ReceivingSession session = receivingSessionHelper.getSession(processDefinitionKey, terminalConfig.getId());
        if (session != null && session.getExecutionId() == null) {
          if (getExecutionId(processDefinitionKey, terminalConfig.getId()) != null) {
            deleteProcessInstance(getExecutionId(processDefinitionKey, terminalConfig.getId()));
          }
        }

        logger.info("Active process for Workstation[LID={}] is {}, continue dialog.", getWorkstation().getLogicalId(), activeProcess);
        return null;
      }
      else // Check if this terminal still has a processInstance, if so continue. If not we can quit.
       if (getProcessInstance(processDefinitionKey, getWorkstation().getId()) != null) {
          logger.info("Active process for Workstation[LID={}] is not {} anymore, but continue dialog since it is not finished yet.", getWorkstation()
                      .getLogicalId(), activeProcess);
          return null;
        }
        else {
          logger.info("Active process for Workstation[LID={}] is not {}", getWorkstation().getLogicalId(), activeProcess);
          return "Active process for Workstation[" + getWorkstation().getLogicalId() + "] is not " + activeProcess;
        }
    }
  }

  class SubareaIsNull extends AbstractState {

    private Context context;
    private SubareaIsNull(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess) {
      super(processDefinitionKey, terminalConfig, activeProcess);
    }

    @Override
    public String action() {

      // If a receiving workstation is still active, allow to continue
      logger.info("No active Process found for Workstation[LID={}], checking if it has still active work.", getWorkstation().getLogicalId());
      if (terminalConfig.getId() != null) {
        ReceivingSession session = receivingSessionHelper.getSession(processDefinitionKey, terminalConfig.getId());
        if (session != null && session.getExecutionId() != null) {
          if (receivingServiceBacking.isWorkstationActive(session.getExecutionId())) {
            // Workstation has active work, allow to continue.
            logger.info("Active process for Workstation[LID={}] is {}, continue dialog.", getWorkstation().getLogicalId(), activeProcess);
            return null;
          }
          else {
            // Workstation doesn't have active work, delete process instance.
            logger.info("Workstation [LID={}] has no active work, delete ProcessInstance.", getWorkstation().getLogicalId());
            deleteProcessInstance(session.getExecutionId());
          }
        }
      }
      logger.info("No active work and no active Process found for Workstation[LID={}]", getWorkstation().getLogicalId());
      return "No active Process found for Workstation[" + getWorkstation().getLogicalId() + "]";

    }

  }

  class Context extends AbstractState {

    private final AbstractState subareaIsNotNull, subareaIsNull;

    private AbstractState state;

    private MovementControlSubarea subarea;

    public Context(String processDefinitionKey, TerminalConfig terminalConfig, WMSProcessEnum activeProcess, MovementControlSubarea subarea) {
      super(processDefinitionKey, terminalConfig, activeProcess);
      this.subarea = subarea;
      subareaIsNotNull = new SubareaIsNotNull(processDefinitionKey, terminalConfig, activeProcess);
      subareaIsNull = new SubareaIsNull(processDefinitionKey, terminalConfig, activeProcess);
    }

    @Override
    public String action() {
      terminalConfig = terminalConfigDao.findById(terminalConfig.getId());
      state.setWorkstation(terminalConfig.getWorkstation());

      if (subarea != null) {
        state = subareaIsNotNull;
      }
      else {
        state = subareaIsNull;
      }
      return state.action();
    }
  }

By using object-oriented way we’ll have behavior, which will be opened for extension but closed for modification. Splitting up a big method into different classes will definitely reduce cyclomatic complexity. This might be useful for continuous inspection of code quality by sonarqube or other tools for static code analysis. Moreover, it also will create code base and class hierarchy structure close to finite state machine and therefore it will create an easier way to read and understand the code.

Leave a Reply

Your email address will not be published.

https://isd-soft.com/wp-content/uploads/2022/08/whitelogo150.png
Connect with us
Bulgara Street 33/1, Chisinau MD-2001, Moldova
+ 373 22 996 170
info@isd-soft.com
De Amfoor 15, 5807 GW Venray-Oostrum, The Netherlands
+ 31 478 502944

Subscribe to our newsletter today to receive updates on the latest news, releases and special offers.

Copyright ©2022, ISD. All rights reserved | Cookies Policy | Privacy Policy