5 ms·

And as always, Goodhart's law applies: "When a measure becomes a target, it ceases to be a good measure" While the concept of cyclomatic complexity can be a so

And as always, Goodhart's law applies: "When a measure becomes a target, it ceases to be a good measure" While the concept of cyclomatic complexity can be a somehow useful guideline, I've seen way too many CI/CD pipelines enforcing CC goals, which then usually results in an un-untangleable middleware and module mess where any piece of core logic is scattered over 102 code points in 34 modules — instead of just residing in a single, well readable and commented main function that would neatly self-explain a whole complex task in three screens that might have a CC of 20.

michaelcampbell 3y ago
Indeed, I've used McCabe stuff in the past not as a gate, but a way to say "this should be looked at before that" type of thing. Works pretty well. A low score doesn't mean "good", nor a high one "bad", just an indication that something might do well with some more scrutiny.
- pasc1878 3y ago
  Or even more importantly if the score is increasing as revisions occur you need to do a full review of that part. You are looking for changes and not absolute values of many measures.
  - readthenotes1 3y ago
    I used this advice for decades. It works well...
  - ozim 3y ago
    OK first time I see Cyclomatic complexity as a useful metric. Rate of change is more important than any specific measure. But I never seen anyone mentioning that - but after I saw that here in comment it seems so obvious now.
    - pasc1878 3y ago
      No it is not rate of change. It is change in itself. The value should not be increasing.
Swizec 3y ago
> any piece of core logic is scattered over 102 code points in 34 modules — instead of just residing in a single, well readable and commented main function that would neatly self-explain The tragedy of McCabe metrics is that this is a tooling failure. You can and should apply McCabe to whole software and then all these smol-functions approaches start falling apart. McCabe goes through the roof because the metric doesn’t depend on length. McCabe measures decision points. A 5000 line linear function has a McCabe complexity of 1. It’s fine. An even better measure is architectural complexity. I’ve written about this a lot after reading some papers and a PhD thesis about it [1]. Tried turning it into a book but that didn’t quite happen. It will be a subpoint in a new project :) Architectural Complexity uses visibility metrics to say how intertwined your software has become. A good rule of thumb is that the more imports/exports you have in a file, the more architecturally complex it is. Best seen with a dependency graph visualizer, but you can also feel it viscerally when you have to bounce around a bazillion files to understand what happens with any code flow. [1] https://swizec.com/blog/why-taming-architectural-complexity-is-paramount https://swizec.com/blog/why-taming-architectural-complexity-...
- throwanem 3y ago
  I think it's a shame you didn't write the book. I know some people who desperately need to read it.
  - chrisweekly 3y ago
    Same! Also, Swizec's an awesome writer. (Long-time email subscriber, and fan of his posts and Serverless Handbook book, etc).
- epage 3y ago
  Like cpus with icache and dcache which leverage memory locality, we overlook the effect of locality on understanding code.
- tetha 3y ago
  I mean I'm not working on big code bases anymore, more tooling, analytics and managment. But both the open/closed principle and some advice from Sandy Metz and others really stuck with me: Don't bother abstracting. Just write straight line code. And then introduce indirections and abstractions if good reasons demand it. In fact, if changes to the code demand it. For example, introducing an abstraction for an external database can improve testing prospects. If you need to fix the same bug in several places, it's usually time to extract a common abstraction. If you need to touch some established piece of code to introduce a choice what kinda algorithm/strategy/formatting/.... you need to use here, that's a decent point to introduce some abstraction and to inject some function/object here. If you would have to replicate the exact complex logic from somewhere else, and those places need to move in sync for reasons, moving that to a common place is probably a good idea. Just following what the code needs and the changes and requirements around the code need has resulted in much simpler code and way more effective abstractions for myself.
captainbland 3y ago
I wonder if there are any open source emulators out there which are compliant with a strict limit on cyclomatic complexity. Specifically given a common pattern is to use a large switch/case to map from instructions which usually compiles to a jump table which is performant, fairly clear/maps nicely to architecture docs but falls foul to normal cyclomatic complexity rules. Edit: project concept: Enterprise Gameboy.
- almostnormal 3y ago
  > Specifically given a common pattern is to use a large switch/case to map from instructions which usually compiles to a jump table [...] Work-around: Implement the jump table directly, e.g., array of function pointers or whatever works in the language of choice.
Scubabear68 3y ago
Old style Java code often exhibits low CC, and as you indicate this is not necessarily a good thing. The actual logic is often spread out in a multitude of helpers, proxies, deep inheritance chains, and unneeded abstractions. The end result is a system that is very difficult to reason about. Also, in general terms over use of DRY can lead you here. In many cases it is better to “risk” higher CC by concentrating logic where it makes sense.
- nicholasjarnold 3y ago
  Exactly, and this is why I always try to steer teams away from "one metric to rule them all" whether this be "always fail if coverage is less than X percent" or "random code metric like CC is beyond limit". Reality is simply more complicated than that, and it takes experienced engineers to actively manage and balance the tradeoffs appropriately over time. Putting arbitrary one-size-fits-all rules in place is almost never the answer. Unfortunately, in some (many?) companies there simply aren't enough experienced engineers who have the time to do the active balancing...leading us back to "just stick this rule in pipeline so the teams are held to _some standard_".
  - Scubabear68 3y ago
    Agreed. I like to do these scans but for informational purposes, not as a gate. Also most tools allow you to annotate code to turn off warnings, which can help when used intelligently. Of course some teams will over use such tools and turn off the metrics left and right. In the end there is no substitute for experienced engineers.
- captainbland 3y ago
  Yeah I agree with this. Or, rather, I think there's an argument to be made that what we're really interested in is logic density Vs logic sparsity and when developing something we have to decide what the right level of that is. So if you're a small team, logic density makes sense because it means one person can have a lot of context in one place and make significant changes without doing a lot of context switching. Don't underestimate the productivity of one experienced guy doing old school PHP. But if you're a large team on a large project logic sparsity (with appropriate decoupling!) makes sense because then multiple people can change the code simultaneously without stepping on each others' toes. People can specialise where appropriate without having to understand the whole thing. The "sparsity" approach is obviously what enterprises see as good news mostly because it means they can have large teams without silos but in real terms it costs a lot of money to operate this way. Although I think as you rightly point out, Java has in recent years realised that it possibly went a bit too far in that direction historically.
lcnPylGDnU4H9OF 3y ago
As soon as I saw the title, I was reminded of these comments that I sometimes see in Ruby code: # rubocop:disable Metrics/CyclomaticComplexity Presumably because nobody on the team is brave enough to open a PR that disables it in the config.
ris 3y ago
This x100. If you take a function and tell someone it's "too complex" and tell them to "fix it", that person will usually end up splitting it into, say, three functions. And now, you've just multiplied that complexity by all the different possible permutations of how/where the different functions can call each other. Corner cases that you could previously rule out because the control flow made them impossible are now fair game.
- kromem 3y ago
  Except complexity as an issue is primarily about readability. Fine, a junior developer has a switch statement that's like 10 things long within a function with several if/else branches and they refactor to meet complexity limits by moving the switch to a function. As long as that function is well named, that's a huge improvement to readability. In part, complexity thresholds relate to human short term memory limits of seven plus or minus two.
MrGilbert 3y ago
That's why it's important to pair this one single aspect (cyclomatic complexity) with other aspects that balance it, and combine it into an index.

michaelcampbell 3y ago

Indeed, I've used McCabe stuff in the past not as a gate, but a way to say "this should be looked at before that" type of thing. Works pretty well. A low score doesn't mean "good", nor a high one "bad", just an indication that something might do well with some more scrutiny.

pasc1878 3y ago
Or even more importantly if the score is increasing as revisions occur you need to do a full review of that part. You are looking for changes and not absolute values of many measures.
- readthenotes1 3y ago
  I used this advice for decades. It works well...
- ozim 3y ago
  OK first time I see Cyclomatic complexity as a useful metric. Rate of change is more important than any specific measure. But I never seen anyone mentioning that - but after I saw that here in comment it seems so obvious now.
  - pasc1878 3y ago
    No it is not rate of change. It is change in itself. The value should not be increasing.

Swizec 3y ago

> any piece of core logic is scattered over 102 code points in 34 modules — instead of just residing in a single, well readable and commented main function that would neatly self-explain The tragedy of McCabe metrics is that this is a tooling failure. You can and should apply McCabe to whole software and then all these smol-functions approaches start falling apart. McCabe goes through the roof because the metric doesn’t depend on length. McCabe measures decision points. A 5000 line linear function has a McCabe complexity of 1. It’s fine. An even better measure is architectural complexity. I’ve written about this a lot after reading some papers and a PhD thesis about it [1]. Tried turning it into a book but that didn’t quite happen. It will be a subpoint in a new project :) Architectural Complexity uses visibility metrics to say how intertwined your software has become. A good rule of thumb is that the more imports/exports you have in a file, the more architecturally complex it is. Best seen with a dependency graph visualizer, but you can also feel it viscerally when you have to bounce around a bazillion files to understand what happens with any code flow. [1] https://swizec.com/blog/why-taming-architectural-complexity-is-paramount https://swizec.com/blog/why-taming-architectural-complexity-...

throwanem 3y ago
I think it's a shame you didn't write the book. I know some people who desperately need to read it.
- chrisweekly 3y ago
  Same! Also, Swizec's an awesome writer. (Long-time email subscriber, and fan of his posts and Serverless Handbook book, etc).
epage 3y ago
Like cpus with icache and dcache which leverage memory locality, we overlook the effect of locality on understanding code.
tetha 3y ago
I mean I'm not working on big code bases anymore, more tooling, analytics and managment. But both the open/closed principle and some advice from Sandy Metz and others really stuck with me: Don't bother abstracting. Just write straight line code. And then introduce indirections and abstractions if good reasons demand it. In fact, if changes to the code demand it. For example, introducing an abstraction for an external database can improve testing prospects. If you need to fix the same bug in several places, it's usually time to extract a common abstraction. If you need to touch some established piece of code to introduce a choice what kinda algorithm/strategy/formatting/.... you need to use here, that's a decent point to introduce some abstraction and to inject some function/object here. If you would have to replicate the exact complex logic from somewhere else, and those places need to move in sync for reasons, moving that to a common place is probably a good idea. Just following what the code needs and the changes and requirements around the code need has resulted in much simpler code and way more effective abstractions for myself.

captainbland 3y ago

I wonder if there are any open source emulators out there which are compliant with a strict limit on cyclomatic complexity. Specifically given a common pattern is to use a large switch/case to map from instructions which usually compiles to a jump table which is performant, fairly clear/maps nicely to architecture docs but falls foul to normal cyclomatic complexity rules. Edit: project concept: Enterprise Gameboy.

almostnormal 3y ago
> Specifically given a common pattern is to use a large switch/case to map from instructions which usually compiles to a jump table [...] Work-around: Implement the jump table directly, e.g., array of function pointers or whatever works in the language of choice.

Scubabear68 3y ago

Old style Java code often exhibits low CC, and as you indicate this is not necessarily a good thing. The actual logic is often spread out in a multitude of helpers, proxies, deep inheritance chains, and unneeded abstractions. The end result is a system that is very difficult to reason about. Also, in general terms over use of DRY can lead you here. In many cases it is better to “risk” higher CC by concentrating logic where it makes sense.

nicholasjarnold 3y ago
Exactly, and this is why I always try to steer teams away from "one metric to rule them all" whether this be "always fail if coverage is less than X percent" or "random code metric like CC is beyond limit". Reality is simply more complicated than that, and it takes experienced engineers to actively manage and balance the tradeoffs appropriately over time. Putting arbitrary one-size-fits-all rules in place is almost never the answer. Unfortunately, in some (many?) companies there simply aren't enough experienced engineers who have the time to do the active balancing...leading us back to "just stick this rule in pipeline so the teams are held to _some standard_".
- Scubabear68 3y ago
  Agreed. I like to do these scans but for informational purposes, not as a gate. Also most tools allow you to annotate code to turn off warnings, which can help when used intelligently. Of course some teams will over use such tools and turn off the metrics left and right. In the end there is no substitute for experienced engineers.
captainbland 3y ago
Yeah I agree with this. Or, rather, I think there's an argument to be made that what we're really interested in is logic density Vs logic sparsity and when developing something we have to decide what the right level of that is. So if you're a small team, logic density makes sense because it means one person can have a lot of context in one place and make significant changes without doing a lot of context switching. Don't underestimate the productivity of one experienced guy doing old school PHP. But if you're a large team on a large project logic sparsity (with appropriate decoupling!) makes sense because then multiple people can change the code simultaneously without stepping on each others' toes. People can specialise where appropriate without having to understand the whole thing. The "sparsity" approach is obviously what enterprises see as good news mostly because it means they can have large teams without silos but in real terms it costs a lot of money to operate this way. Although I think as you rightly point out, Java has in recent years realised that it possibly went a bit too far in that direction historically.

lcnPylGDnU4H9OF 3y ago

As soon as I saw the title, I was reminded of these comments that I sometimes see in Ruby code: # rubocop:disable Metrics/CyclomaticComplexity Presumably because nobody on the team is brave enough to open a PR that disables it in the config.

ris 3y ago

This x100. If you take a function and tell someone it's "too complex" and tell them to "fix it", that person will usually end up splitting it into, say, three functions. And now, you've just multiplied that complexity by all the different possible permutations of how/where the different functions can call each other. Corner cases that you could previously rule out because the control flow made them impossible are now fair game.

kromem 3y ago
Except complexity as an issue is primarily about readability. Fine, a junior developer has a switch statement that's like 10 things long within a function with several if/else branches and they refactor to meet complexity limits by moving the switch to a function. As long as that function is well named, that's a huge improvement to readability. In part, complexity thresholds relate to human short term memory limits of seven plus or minus two.

MrGilbert 3y ago

That's why it's important to pair this one single aspect (cyclomatic complexity) with other aspects that balance it, and combine it into an index.