Why does Cyclomatic Complexity matter?

If you’re using some code analysis tool like Checkstyle or Sonar, you must be familiar with the following warning: Cyclomatic Complexity is XX (max allowed is 10).

Here is the explanation that Checkstyle (also embedded in Sonar) gives about this coding rule:

Checks cyclomatic complexity of methods against a specified limit. The complexity is measured by the number of if, while, do, for, ?:, catch, switch, case statements, and operators && and || (plus one) in the body of a constructor, method, static initializer, or instance initializer. It is a measure of the minimum number of possible paths through the source and therefore the number of required tests. Generally 1-4 is considered good, 5-7 ok, 8-10 consider re-factoring, and 11+ re-factor now !

In case you want to learn more about the concept, Wikipedia gives more details about this metric, developed by Thomas McCabe in 1976. In this article I will rather focus on why keeping the cyclomatic complexity per method as low as possible matters.

Here is what you don’t want your application’s viscera to look like:


Creative Commons License photo credit: Adrien Van Hamme

This thing might work, but imagine the poor guy, newly hired, who needs to debug that because something in the middle of this mess doesn’t work anymore. He can spend days trying to figure out what the freaks who designed that 10 years ago  (or 10 months ago) had in mind. In the IT world, it happens everyday and everywhere (don’t tell that to the idealistic students, we need them to join the party).

Here are two interesting quotes about programming:

Programmers over the lifetime of a system spend far more time reading code than writing code. (Steve McConnell)

Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. (Brian Kernighan)

In other words,  you shouldn’t try to make your code as clever (i.e. complex) as possible, but as simple as possible because someone, maybe yourself, will have to read it and understand it one day.

Let’s take a simple example to make it visual.

public int getMax(int a, int b, int c) {
	int max = Integer.MIN_VALUE;
	if (a > b) {
		max = a;
	} else {
		max = b;
	}
	if (b > c) {
		max = b;
	} else {
		max = c;
	}
	return max;
}

Listing 1 – One silly method containing a stupid bug  (cyclomatic complexity 3)

Now let’s write a JUnit test case to see if our code is correct.

@Test
public void testGetMax1() {
	int max = new SmartClass().getMax(3, 4, 2);
	assertEquals(4, max);
}

This test case doesn’t cover all the paths, the output is 4 as expected and we don’t find the bug. We need to write more test cases to cover all the paths:

@Test
public void testGetMax2() {
	int max = new SmartClass().getMax(3, 4, 5);
	assertEquals(5, max);
}
@Test
public void testGetMax3() {
	int max = new SmartClass().getMax(5, 4, 3);
	assertEquals(5, max);
}
@Test
public void testGetMax4() {
	int max = new SmartClass().getMax(5, 4, 6);
	assertEquals(6, max);
}

Now we’re covering everything and we can see that testGetMax3 fails. Hold on! That makes 4 necessary test cases so why is the cyclomatic complexity  of this method only 3? Good catch! In fact, the Checkstyle description simplified the metric a little bit. Let’s not bother too much about the theory but the truth is that (with the number for our example between parentheses) :

branch coverage (2)  ≤ cyclomatic complexity (3) ≤ number of paths (4).

Ok, we already said that a complexity of 3 is acceptable, but let’s try to improve that a little bit:

public int getMax(int a, int b, int c) {
	int max = Integer.MIN_VALUE;
	max = getMax(a, b);
	max = getMax(b, c);
	return max;
}

private int getMax(int a, int b) {
	int max = Integer.MIN_VALUE;
	if (a > b) {
		max = a;
	} else {
		max = b;
	}
	return max;
}

Listing 2 – Two silly methods containing a stupid bug … but better (cyclomatic complexity 1 and 2)

Now we only need testGetMax1 to cover all the paths, even in the private method which will be called with (a=3, b=4) then (a=4, b=2). Isn’t it wonderful?

Wait a minute Bobby! We still have a bug and we’re saying that we don’t need the test case to find it anymore? Yes, that’s exactly what we’re saying here. In fact, we didn’t need it in the first place.

Let’s face it honestly, nobody will ever require 100% of paths coverage with unit testing because nobody can afford that. Even in very sensitive areas, like the navigation system of the airplanes or space ships, I would be very surprised to hear such a thing. In our daily jobs, 100% of code coverage is already more than good. It’s excellent and already very hard to achieve.

With the first listing, a full code coverage is provided with 2 test cases, testGetMax1 and testGetMax4. With the second listing, as already stated, it’s achieved with only one test case, testGetMax1, and that’s a nice improvement. In both cases, with this 100% code coverage, we will miss the bug. Yes, it happens, that’s life … just too bad.

So, assuming we’re targetting only 100% of code coverage, reducing the cyclomatic complexity of the code can help reducing the number of test cases. That’s a good benefit, but does it help dealing with this (not so) sneaky bug?

Recent cognitive science studies have shown that the human brain needs an average of 6.4 seconds to find the bug in listing 1, whereas it only needs 3.1 seconds with listing 2 … Ok, that’s bullsh*t, I just made up those numbers. But if there’s any bored researcher reading this article (who knows …), please try this kind of experience. I’m ready to bet on the outcome (not the exact times, you won’t make money with me).


Creative Commons License photo credit: Rishi Menon

Doesn’t it look better?

So, the bottom line is that limiting cyclomatic complexity in your code is important for one very simple and pragmatic reason that can even be expressed with one word: READABILITY. That was a big article just to state the obvious, which sometimes doesn’t seem obvious enough.

public int getMax(int a, int b, int c) {

int max = Integer.MIN_VALUE;

if (a < b) {

max = a;

} else {

max = b;

}

if (b < c) {

max = b;

} else {

max = c;

}

return max;

}

public int getMax(int a, int b, int c) {

int max = Integer.MIN_VALUE;

max = getMax(a, b);

max = getMax(b, c);

return max;

}

private int getMax(int a, int b) {

int max = Integer.MIN_VALUE;

if (a < b) {

max = a;

} else {

max = b;

}

return max;

}

5 thoughts on “Why does Cyclomatic Complexity matter?

  1. Interesting article, thanks.

    Some minor stuff: shouldn’t the comparisons be “a > b” instead of “a < b" in your examples? Also, there is a typo in "the output is 5 as expected", it should say 4.

  2. Good article, but I disagree with your conclusion (I also disagree with your use of Comic Sans to emphasize said conclusion, but I won’t get into that here :)).

    I don’t think your conclusion that low cyclomatic complexity implies readability is true (in fact, I think the converse might be more true). Transforming branch statements into function calls doesn’t guarantee that your code is good, correct or readable.

    1. I will stand behind my choice of Comic Sans. This font has been suffering a bad reputation for too long, it’s time to rehabilitate it ;)

      Your opinion is interesting. It’s certainly possible to write bad code with low complexity, using meaningless method names or modifying the same object across multiple methods. However I believe it’s harder to screw up with a protection against cyclomatic complexity.

    2. I agree with the article’s writer though. Many function calls is better than one huge method because function calls have names and comments on their own and often you can guess what a function does just because of the name (that’s not to mention things like IntelliSense on Visual Studio).

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>