The way to solve this is to split decisions from execution and that’s a notion I got from our old pal Bertrand Meyer.

    if (weShouldDoThis()) {
        doThis();
    }

It complements or is part of functional core imperative shell. All those checks being separate makes them easy to test, and if you care about complexity you can break out a function per clause in the check.

Functions should decide or act, not both.

But if that’s all you have, then how does your system do anything ? You ultimately need to be able to decide and then act based in that decision somewhere..

One possibility is a file.py that is called by your framework. The interface could be something like

  def doth_match(*args):
    return True  # the predicate

  def doeth_thou(*args):
    # processing things
    return {}  # status object for example

The framework loops and checks the first function; if true, then execute the second function. And then break or continue for other rule files (or objects).

There could be multiple files rule1.py, rule2.py, etc that check and do different things.

I think the parent's argument is that wherever in your framework you're calling `doth_match` and then `doeth_thou`, you have a single function that's both deciding and acting. There has to be a function in your program that's responsible for doing both.

A function that asks a question with one function call and calls another based on the answer isn’t doing any work or asking any questions. It’s just glue code. And as long as it stays just glue code, that’s barely any of your code.

Asking for absolutes is something journeymen developers need to grow out of.

The principle of the excluded middle applies to Boolean logic and bits of set theory and belongs basically nowhere else in software development. But it’s a one trick pony that many like to ride into the ground.

This just moves decisions from inside of functions to the call site. At which point, there's more that can go wrong, since you're calling a function much more than it's single definition.

This is an excellent rule.

To add to this, a pattern that's really helpful here is: findThingWeShouldDoThisTo can both satisfy a condition and greatly simplify doThis if you can pass it the thing in question. It's read-only, testable, and readable. Highly recommend.

This is not obvious to me. The whole point was to separate the conditionals from the actions.

In your example, it’s not clear if/how much “should we do this” logic is in your function. If none, then great; you’ve implemented a find or lookup function and I agree those can be helpful.

If there’s some logic, eg you have to iterate through a set or query a database to find all the things that meet the criteria for “should do this”, then that’s different than what the original commenter was saying.

maybe: doThis( findAllMatchingThings( determineCriteriaForThingsToDoThisTo()))

would be a good separation of concerns

let list = findHumansToKill();

//list = [];

killHumans(list);

Code scanners reports should be treated with suspicion, not accepted as gospel. Sonar in particular will report “code smells” which aren’t actually bugs. Addressing these “not a bug” issues actually increases the risk of introducing a new error from zero to greater than zero, and can waste developer time addressing actual production issues.

I agree with you. Cyclomatic complexity check may be my least favorite of these rules. I think any senior developer almost always “knows better” than the tool does what is a function of perfectly fine complexity vs too much. But I have to grudgingly grant that they have some use since if the devs in question routinely churn out 100-line functions that do 1,000 things, the CCC will basically coincidentally trigger and force a refactor which may help to fix that problem.

Cyclomatic complexity may be a helpful warning to detect really big functions, but the people who worry about cyclomatic complexity also seem to be the sort of people who want to set the limit really low and get fiesty if a function has much more than a for loop with a single if clause in it. These settings produce those code bases where no function anywhere actually does anything, it just dispatches to three other functions that also don't hardly do anything, making it very hard to figure out what is going on, and that is not a good design.

I call this "poltergeist code". Dozens of tiny functions that together clearly does something complex correctly, but it's very hard to find where and how it's actually done.

One incomplete but easy to state counter "rule" to fling back in such cases is just: If the function isn't generic and re-used for other unrelated things, then it probably shouldn't be a seperate function.

Yeah only probably, there can sure be large distinct sub-tasks that aren't used by any other function yet would improve understanding to encapsulate and replace with a single function call. You decide which by asking which way makes the overall ultimate intent clearer.

Which way is a closer match to the email where the boss or customer described the business logic they wanted? Did they say "make it look at the 3rd word to see if it has trailing spaces..."?

Or to find out which side of the fuzzy line a given situation is falling, just make them answer a question like, what is the purpose of this function? Is it to do the thing it's named after? Or is it to do some meaningless microscopic string manipulation or single math op? Why in the world do you want to give a name and identity to a single if() or memcpy() etc?

I love that name, and will definitely steal it!

I get that, and I think it’s a fine check for junior devs especially. In my experience, mostly with PHP mess detector, Cyclomatic Complexity smells typically manifest as nested conditionals. I’m at a point now where I try to keep that minimal myself, but I have peers that often have these diving sets of if statements that irk me. I’d rather invert a conditional and return early if needed, or at least separate the logic so that there’s maybe two levels at most.

I do agree that, in general, a senior engineer should be able to suss out what’s too complex. But if the bar is somewhat high and it keeps me from sending a PR back just for complexity, that’s fine.

The tools are usually required for compliance of some sort.

Fiddling with the default rules is a baby & bathwater opportunity similar to code formatters, best to advocate for a change to the shipping defaults but "ain't nobody got time for that"™.

I like the approach of “if there is no standard, give everyone a week to agree and if they don’t just pick something.” I’ve built too many sheds with “tabs or spaces?”

a/k/a if it works don't fuck with it.

IME this is frequently a local optimum though. "Local" meaning, until some requirement changes or edge case is discovered, where some branching needs to happen outside of the loop. Then, if you've got branching both inside and outside the loop, it gets harder to reason about.

It can be case-dependent. Are you reasonably sure that the condition will only ever effect stuff inside the loop? Then sure, go ahead and put it there. If it's not hard to imagine requirements that would also branch outside of the loop, then it may be better to preemptively design it that way. The code may be more verbose, but frequently easier to follow, and hopefully less likely to turn into spaghetti later on.

(This is why I quit writing Haskell; it tends to make you feel like you want to write the most concise, "locally optimum" logic. But that doesn't express the intent behind the logic so much as the logic itself, and can lead to horrible unrolling of stuff when minor requirements change. At least, that was my experience.)

I've always hated code complexity scanners ever since I noticed them complaining about perfectly readable large functions. It's a lot more readable when you have the logic in one place, and you should only be trying to break it up when the details cause you to lose track of the big picture.

There was a thread yesterday about LLMs where somebody asked "what other unreliable tool people accept for coding?"

Well, now I have an answer...

tangent: how did you get that superscript 0 to render in an HN comment?

HN allows many Unicode characters, including U+2070 superscript zero which I copy+pasted after a web search. I'd actually be interested to learn the full allowlist.

https://en.wikipedia.org/wiki/Unicode_subscripts_and_supersc...