10

I am using SonarLint for Eclipse since recently, and it helped me a lot. However, it raised to me a question about cyclomatic complexity.

SonarLint considers as acceptable a C.C of 10, and there are some cases where I am beyond it, about 5 or 6 units. Those parts are related to mappers where the values relies on different variables, for example:

  • Field A relies on String sA;
  • Field B relies on String sB;
  • Field C relies on String sC;
  • etc ...

I have no other choice that putting an if for each field. This is not my choice (fortunately) but an already existing and complex system that I cannot change by myself.

The core of my question is: why is it so important to not have a too high C.C in a single method ? If you move some of your conditions in one or more sub-methods to reduce the complexity, it does not reduce the cost of your overall function, it is just moving the problem elsewhere, I guess ?

(Sorry for small mistakes, if any).

EDIT

My question does not refer to global cyclomatic complexity, but only to single method complexity and method splitting (I have a rough time explaining what exactly I mean, sorry). I am asking why does it is allowable to split your conditions into smaller methods if it still belongs to a 'super method', which will just execute every sub-method, thus adding complexity to the algorithm.

The second link however (about the anti-pattern) is of great help.

Improve this question
6
  • 5
    Possible duplicate of What does the 'cyclomatic complexity' of my code mean?
    – gnat
    Dec 2, 2016 at 13:57
  • see also: How to tackle a 'branched' arrow head anti-pattern?
    – gnat
    Dec 2, 2016 at 13:58
  • ^^^ "arrow head" question is probably a better duplicate in the sense that it explains how to improve your code but I picked first one because of detailed explanation answering the part of your question about cyclomatic complexity
    – gnat
    Dec 2, 2016 at 14:12
  • Splitting a method up into smaller parts doesn't reduce the total amount of code executed but makes plain the individual tasks that are happening; they can each be understood much more easily individually than when they're all tangled up in a larger whole. At the very least it removes many one-time-use, intermediate variables from the larger scope.
    – Doval
    Dec 2, 2016 at 14:21
  • 1
    For your particular described case, I'd do something in your primary method like "A = extractAFrom(sA);" for each field. You can probably come up with better names since you know the actual fields and their uses.
    – Tin Wizard
    Dec 2, 2016 at 22:15

7 Answers 7

Reset to default
33

The core thing here: "brain capacity".

You see, one of the main functions of code is ... to be read. And code can be easy to read and understand; or hard.

And having a high CC simply implies a lot of "levels" within one method. And that implies: you, as a human reader will have a hard time understanding that method.

When you read source code, your brain automatically tries to put things into perspective: in other words - it tries to create some form of "context".

And when you have a small method (with a good name) that only consists of a few lines, and very low CC; then your brain can easily accept this "block". You read it, you understand it; DONE.

On the other hand, if your code has high CC, your brain will spend many many "cycles" more to deduct what is going on.

Another way of saying that: you should always lean towards preferring a complex network of simple things over a simple network of complex things. Because your brain is better at understanding small things.

Improve this answer
9
  • 9
    So basically it is not about technical issues, but about human ? This sounds, indeed, kind of clever, I don't know how I did not think of it before. Thanks !
    – Yassine Badache
    Dec 2, 2016 at 14:05
  • 4
    If so, I wholeheartedly recommend you to acquire "Clean Code" by Robert Martin (you might find the pdf for free on the nets). You see, creating readable code is one of the most important but very often ignored virtue of a good programmer.
    – GhostCat
    Dec 2, 2016 at 14:07
  • @YassineBadache CC also makes it hard to test every nook and cranny (full coverage).
    – Tulains Córdova
    Dec 2, 2016 at 19:44
  • This was the crux of Dijkstra's goto paper, too.
    – Seth Battin
    Dec 2, 2016 at 20:16
  • In my experience bugs are almost always in the methods with a high CC and when bugs are in a low-CC method they're usually totally obvious, no way for them to hide past the first run of the code. Also, I find I almost never have to modify a low-CC method--more load taken off the brain.
    – Loren Pechtel
    Dec 2, 2016 at 21:42
6

C.C, like all other rules of thumb for code smells, is a heuristic. It's not a fail-safe criterion that tells you an absolute truth. If it were, the reasonable thing to do would be simply to make such methods illegal in the language and force people to achieve their ends in another way.

But that isn't the way indicators work. Most of the time their function is to alert people of things that they weren't aware of. In your case, you're aware that the logic is convoluted and the alternative solutions would make it even more convoluted. Therefore there is no point in trying to satisfy the primitive rule of thumb, when its main purpose is to issue warnings to people who aren't aware that there is a problem.

Improve this answer
1
  • 2
    If "FieldA relies on String sA" as OP states I am not convinced that moving this into a CalculateFieldA(String sA) makes for more convoluted code.
    – Taemyr
    Dec 2, 2016 at 15:15
2

In short: it's all about readability and therefore maintainability of your code.

If you have a long, complex method with a lot of (nested) if, it becomes hard to tell what it actually does. If you extract some private methods and name them in a meaningful way, it is much easier.

Improve this answer
1
  • While this is a good answer, it's a little bit short. It would be nice if you could provide an example of what you're saying, and be more specific regarding the question asked by OP: "why is it so important to not have a too high C.C in a single method ?".
    – Machado
    Dec 7, 2016 at 20:15
2

Cyclomatic complexity of a method is related to the number of test cases required for a method. Specifically, a cyclomatic complexity of 10 means that 10 is the upper bound for test cases to have total branch coverage for your method. It's also related to the number of paths that must be tested, minus any impossible paths.

Beyond that, I agree with the other answers for other considerations - mental capacity of a developer or an indicator of potential problems or refactoring or a measure of readability and maintainability of the code.

Improve this answer
0

CC is just a heuristic, and how 'bad' a particular score is depends on many things.

That said, you should always view a high CC as something that highlights code that could/should be refactored. You say that moving the if statement to another method is hiding the problem - but is there a pattern there that you can abstract instead of copy pasting n times? If it's a long if-else chain, can you turn it into a switch statement, or perhaps use polymorphism, or something else? If there's deep nesting, can some of your conditional clauses be joined, or does it indicate separate responsibilities that should be split into different classes?

Improve this answer
0

I see cyclomatic complexity as a warning. If you can read the code and it is not too complex to understand then I wouldn't worry to much about it, there are probably more important things wrong to worry about, there always are.

One way to reduce the kind of CC you mention is to use polymorphism, as you have tagged your question with the Java tag. So instead of code paths being stringly typed you can use well named classes. This can help, but is sometimes overkill and can make your code even harder to understand.

However, it can be a sign of code that will be hard to maintain. When reading the method, is it easy to see which code path you will go down for each case? Would you be able to skip past this method if you don't know the code base well and were searching for something related but further on in the code? I know some people advocate splitting methods up in to lots of 1/2 line methods with descriptive names but sometimes I think that is even harder to read than the code it was meant to replace.

Ultimately maintainability is a hard problem and it is up to you to decide which you think will be easier to read. The fact that you are thinking about this at all means you are on the right track. Just remember, the maintainer who has to try to decipher this code in a years time might be you. So make it as easy for them as possible.

Improve this answer
0

It comes down to how much time is spent looking (brain cycles) at code and comprehending what the code is doing.

  • Consider a 10 line method - Probably take a few minutes to comprehend what it is doing.
  • Consider a 100 line method - Probably take an hour or more so to comprehend what is doing.
  • Consider a 1000 line method - Probably take a day or more so to comprehend what is doing.

Also larger methods are harder to test and harder to predict what kind of behavior could occur.

Cyclomatic complexity is a measure. In this case, higher values are indicators of potential problems. Complex code takes longer to comprehend understand and probably is not tested as thoroughly as less complex methods. So, it's important to note what areas of the code are complex with this measure for refactoring and maintenance purposes.

Another thing to consider is updating complex code. When doing analysis, a developer could report that changing the code is more or less risky by looking at it's complexity.

So, there a lot of value to measuring complexity that can be leveraged and used for decision making purposes.

Improve this answer
1
  • 1
    Sorry Ghost Cat answer very similar, should have refreshed page.
    – Jon Raynor
    Dec 2, 2016 at 14:36

Not the answer you're looking for? Browse other questions tagged or ask your own question.