Start Return Statements and Cyclical Complexity

Question

I prefer this early return writing style:

public static Type classify(int a, int b, int c) { if (!isTriangle(a, b, c)) { return Type.INVALID; } if (a == b && b == c) { return Type.EQUILATERAL; } if (b == c || a == b || c == a) { return Type.ISOSCELES; } return Type.SCALENE; } 

Unfortunately, each return increases the cyclometric complexity metric calculated by Sonar. Consider this alternative:

 public static Type classify(int a, int b, int c) { final Type result; if (!isTriangle(a, b, c)) { result = Type.INVALID; } else if (a == b && b == c) { result = Type.EQUILATERAL; } else if (b == c || a == b || c == a) { result = Type.ISOSCELES; } else { result = Type.SCALENE; } return result; } 

The cyclomatic complexity of this latter approach, reported by Sonar, is lower than the first by 3. I was told that this could be the result of improper implementation of CC metrics. Or is Sonar right, and is that really better? These related questions do not seem to be consistent with this:

https://softwareengineering.stackexchange.com/questions/118703/where-did-the-notion-of-one-return-only-come-from

https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement

If I add support for a few more types of triangles, the return will add a significant difference in the metric and cause sonar disruption. I do not want to stick to the // NOSONAR method in the method, as this may mask other problems with new functions / errors added to this method in the future. Therefore, I use the second version, although I really do not like it. Is there a better way to handle the situation?

Answer 1

Not quite an answer, but too long for comment.

This SONAR rule seems to be completely violated. You can rewrite

 b == c || a == b || c == a 

a

 b == c | a == b | c == a 

and get two points in this weird game (and maybe even some speed, since branching is expensive, but that’s at the discretion of JITc, anyway).

The old rule states that cyclic complexity is related to the number of tests. the new one is wrong, and the good thing is, obviously, the number of meaningful tests for both of your fragments is exactly the same.

Is there a better way to handle the situation?

Actually, I have an answer: for each early return, use | instead of || once .: D

Now seriously: there is an error requesting annotations that allow you to disable one rule that is marked as fixed. I don't look anymore.

Answer 2

Your question relates to https://jira.codehaus.org/browse/SONAR-4857 . Currently, all SonarQube analyzers mix cyclic complexity and substantial complexity. From a theoretical point of view, the return operator should not increase cc, and this change will occur in the SQ ecosystem.

Answer 3

Since the question also concerns early return statements as a coding style, it would be useful to consider the effect of size on the return style. If the method or function is small, less than 30 lines, early results are not a problem, because anyone who reads the code can immediately see the whole method, including all returns. In larger methods or functions, an early return may be unintentionally set for the reader. If an early return occurs above the code that the reader is looking at, and the reader does not know that the return is higher or forgets that it is higher, the reader misunderstands the code. Production code may be too large to fit on one screen.

Thus, anyone who manages the code base for complexity should consider the size of the method when complexity is the problem. If the code occupies more than one screen, a more pedantic style of return may be justified. If the method or function is small, do not worry about it.

(I am using Sonar and have experienced the same problem.)