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;
}
}
Reduce the Number of Decision Structures If the decision structures—especially if-else and switch case—are what cause more branches in the code, it stands to reason that you should reduce them if you want to keep cyclomatic complexity at bay.
Apply formulas in order to compute Cyclomatic complexity. 3) Cyclomatic complexity V(G) = P +1 V (G) = 2 + 1 = 3 Where P is predicate nodes (node 1 and node 2) are predicate nodes because from these nodes only the decision of which path is to be followed is taken. Thus Cyclomatic complexity is 3 for given code.
Avoid use of switch/case statements in your code. Use Factory or Strategy design patterns instead. Complexity of 8 (1 for each CASE and 1 for method itself). Refactoring using Factory design pattern and complexity changed to 1.
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);
}
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With