Cyclomatic complexity and switch-case-statements

[Michael Piefel]

Hello everybody,

I have a function here with which what is basically a mapping performed by a switch statement, like this:

switch (propertyName) {
case Constants.PROPERTY_CONTAINS_BEEF:
case Constants.PROPERTY_RECIPECONTENT_BEEF:
case Constants.PROPERTY_RECIPECONTENT_CALF:
    contains.add(FoodCategory.BEEF.name());
    return true;

…

There’s a number of further cases. Now the number of theoretically possible paths trough this statement is very high; and SonarQube complains about the high cyclomatic complexity of the method. On the other hand, the method is very readable, and I cannot imagine a way to write this down more concisely or simpler. How would you go about it? Really refactor, or just suppress the warning?

[matad...@googlemail.com]

It's good practise to let it trigger the violation in most cases however here it's discretionary and the "admin" sonar users can triage and decide if it should be false positive / won't fix. e.g. we might think the switch statements could be refactored into separate method to handle different cases. However most of the time we would simply consider this acceptable and flag as a false positive. 

I believe Sonar are doing work on a slightly new CC calculation which may take into account switch statements like this more. 

[anand sudhan]

Actually in my team we have customized the language plugins to skip switch cases in the calculation of cyclomatic complexity.
Yes switch cases are nicely readable, even though, technically it increases paths of execution.

HTH,
Anand

[G. Ann Campbell]

@Anand, you may be interested in MMF-548.

Ann

[matad...@googlemail.com]

How did you customise exactly?

[anand sudhan]

Thanks! Will look into it.

[anand sudhan]

Every language plugin is different. For Java there used to be a ComplexityVisitor.java class.

You just have to comment out //JavaKeyword.CASE.

I did this for Java 3.0 plugin. I am sure things might have changed slightly for the newer versions, but that is the underlying idea.

On Tuesday, November 8, 2016 at 3:48:15 PM UTC-5, matad...@googlemail.com wrote:

How did you customise exactly?

[G. Ann Campbell]

Oh yeah, you'll be interested in this one too: MMF-151

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/7yUqzSvaw_g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/c2eb4d68-74df-4cd5-a990-1be79a5c804e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Michael Piefel]

Am Freitag, 4. November 2016 15:10:31 UTC+1 schrieb matad...@googlemail.com:

It's good practise to let it trigger the violation in most cases however here it's discretionary and the "admin" sonar users can triage and decide if it should be false positive / won't fix. e.g. we might think the switch statements could be refactored into separate method to handle different cases. However most of the time we would simply consider this acceptable and flag as a false positive. 

I believe Sonar are doing work on a slightly new CC calculation which may take into account switch statements like this more. 

OK, so perhaps in the future we’ll get different results. Until then, I’ll follow the advice to just suppress the warning after consideration.