Question

I am wondering how to reduce the Cyclomatic Complexity of the following code and if this is even something that I should be worried about.

Please refer to the method ValuePojo.getSomething() (Please don't worry about the variable naming, this has been re-written for clarity in this question)

public class ValuePojo
{
    private ValueTypeEnum type;

    private BigDecimal    value1;

    private BigDecimal    value2;

    private BigDecimal    value3;

    public ValuePojo()
    {
        super();
    }

    /**
     * This method reports as "HIGH Cyclomatic Complexity"
     * 
     * @return
     */
    public BigDecimal getSomething()
    {
        if (this.type == null)
        {
            return null;
        }

        switch (this.type)
        {
            case TYPE_A:
            case TYPE_B:
            case TYPE_C:
            case TYPE_D:
                return this.value1;

            case TYPE_E:
            case TYPE_F:
            case TYPE_G:
            case TYPE_H:
                return this.value2;

            case TYPE_I:
            case TYPE_J:
                return this.value3;
        }

        return null;
    }
}
Was it helpful?

Solution

The Cyclomatic Complexity is determined by the number of branches of execution in your code. if - else blocks, switch statements - all increase the Cyclomatic complexity of your code and also increase the number of test cases you would need to ensure appropriate code coverage.

To reduce complexity in your code, I would suggest you remove the case statements that do not have a defined behavior and replace it with a default behavior in your switch statement.

Here is another question on Stack Overflows that addresses this issue.

OTHER TIPS

If you really need to get the cyclomatic complexity down, you can consider using a Map. Obviously, in your implementation, it should only be created and initialized once.

  public BigDecimal getSomething() {
      if (this.type == null) {
          return null;
      }
      Map<Type,BigDecimal> map = new HashMap<Type,BigDecimal>();
      map.put(TYPE_A, value1);
      map.put(TYPE_B, value1);
      map.put(TYPE_C, value1);
      map.put(TYPE_D, value1);
      map.put(TYPE_E, value2);
      map.put(TYPE_F, value2);
      map.put(TYPE_G, value2);
      map.put(TYPE_H, value2);
      map.put(TYPE_I, value3);
      map.put(TYPE_J, value3);

      return map.get(type);
  }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top