Assume a method chooses an action depending on a value from a fairly large enum. Our Sonar now complains about a high cyclomatic complexity (of about the number of case statements, naturally) of this method.
I know, large switch case statements are not really best style in OOP, but sometimes it is quite opportune to use them (in my case a parser evaluating operator tokens) instead of building a complex object tree.
My concern now how to deal with that? Is there any design pattern to split such a switch case meaningfully? Or can I (and should I) exclude the class from measuring CC (as there may be other methods in it where a high CC can easily be avoided)?
It is not a really a critical thing; I just dislike my project having warnings that I can't remove ;o)
Edit: code sample
String process()
String fieldName = this.getField() != null ? this.getField().getSchemaName() : null;
String result = "";
switch (op) {
case PHRASE:
result = "";
if(!value.isEmpty() && value.get(0) != null) {
result = value.get(0).toString();
}
break;
case EQUALS:
case GT:
case GTE:
case LT:
case LTE:
case NOT_EQUALS:
result = prepareSingleParameterStatement(fieldName);
break;
case BETWEEN_EXC:
case BETWEEN_INC:
result = prepareDoubleParameterStatement(fieldName);
break;
case IN:
case NOT_IN:
case ALL_IN:
result = prepareCollectionStatement(fieldName);
break;
case AND:
case OR:
result = prepareLogicalStatement();
break;
case NOT:
result = prepareNotStatement();
break;
default:
break;
}
return result;
}
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.
Consequences: A high cyclomatic complexity for a particular function means that the function will be difficult to understand, and more difficult to test. That in turn means functions with a cyclomatic complexity above 10 or 15 can be reasonably expected to have more undetected defects than simpler functions.
Cyclomatic complexity is a measurement developed by Thomas McCabe to determine the stability and level of confidence in a program. It measures the number of linearly-independent paths through a program module.
Instead of using a large switch statement you can use the enums to build a state machine. What you do is take the code to parse the text in each case block, and put this in a method for each of the enum states.
From example
enum States implements State {
XML {
public boolean process(Context context) {
if (context.buffer().remaining() < 16) return false;
// read header
if(headerComplete)
context.state(States.ROOT);
return true;
}
}, ROOT {
public boolean process(Context context) {
if (context.buffer().remaining() < 8) return false;
// read root tag
if(rootComplete)
context.state(States.IN_ROOT);
return true;
}
}
}
public void process(Context context) {
socket.read(context.buffer());
while(context.state().process(context));
}
From Using an enum as a State Machine
I would suggest replacing a big switch with a hashmap/dictionary, and a command object :
Map<Op, Command> ops = new EnumMap<op>{{
//initialize with different command objects implementing same interface
}};
command = ops.get(result);
result = command.prepare();
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