3 ms·

I generally agree. But static analysis tools are still pretty dumb and sometimes cause extra work to no benefit. Flagging a method for cyclomatic complexity w

I generally agree. But static analysis tools are still pretty dumb and sometimes cause extra work to no benefit. Flagging a method for cyclomatic complexity when it uses a case statement that is blatantly obvious and simple to any human seeing it, for example. Or just happening to notice a bunch of issues in file B when you commit file A but didn't even touch file B. Overall it's good, I like it, but with the caveat that it can take some work to tweak and adjust to avoid waste like that. And that usually seems to come right on the deadline or the day before. Worthwhile, but you have to take that cost into account.

jmiserez 6y ago
Cyclomatic complexity IMHO isn't a good metric for code complexity as understood by humans, but sometimes it can be insightful especially when it's unexpected. My favorite static analysis issues are the ones that flip-flop between two issues, I love doing "merge this if statement with the enclosing one" only to get "expressions should not be too complex" after I've merged them. As long as you have the power as the developer to override the tool and mark issues as won't fix (with rationale) I think it's fine. If not, the ruleset needs to be really good and as minimal as possible. Otherwise you'll just get crappy but compliant code which isn't helping anyone. Also, it sometimes makes sense to leave suboptimal but old and correct code as-is to preserve the Git history or because the suggested change doesn't actually make the code any better. And finally, these tools do still generate false positives. While it makes sense to thoroughly review each of these with a coworker, you should still be able to mark them as false positives.
- muldvarp 6y ago
  > And finally, these tools do still generate false positives. And as long as we talk about tools that do some kind of semantic analysis (not just syntactic) on a turing-complete language they will always either have false positives, false negatives or both. Rice's theorem guarantees that. They still can be useful though. I programmed a static analysis tool for PHP a long time ago and it worked surprisingly well.
- ric2b 6y ago
  > ones that flip-flop between two issues, I love doing "merge this if statement with the enclosing one" only to get "expressions should not be too complex" after I've merged them. I'm not sure that's flip-floping, the solution to the overly complex expression is to turn into a function. Unless the tool is just being overly aggressive on what it considers too complex, in that case you can try to tweak the configuration instead.

jmiserez 6y ago

Cyclomatic complexity IMHO isn't a good metric for code complexity as understood by humans, but sometimes it can be insightful especially when it's unexpected. My favorite static analysis issues are the ones that flip-flop between two issues, I love doing "merge this if statement with the enclosing one" only to get "expressions should not be too complex" after I've merged them. As long as you have the power as the developer to override the tool and mark issues as won't fix (with rationale) I think it's fine. If not, the ruleset needs to be really good and as minimal as possible. Otherwise you'll just get crappy but compliant code which isn't helping anyone. Also, it sometimes makes sense to leave suboptimal but old and correct code as-is to preserve the Git history or because the suggested change doesn't actually make the code any better. And finally, these tools do still generate false positives. While it makes sense to thoroughly review each of these with a coworker, you should still be able to mark them as false positives.

muldvarp 6y ago
> And finally, these tools do still generate false positives. And as long as we talk about tools that do some kind of semantic analysis (not just syntactic) on a turing-complete language they will always either have false positives, false negatives or both. Rice's theorem guarantees that. They still can be useful though. I programmed a static analysis tool for PHP a long time ago and it worked surprisingly well.
ric2b 6y ago
> ones that flip-flop between two issues, I love doing "merge this if statement with the enclosing one" only to get "expressions should not be too complex" after I've merged them. I'm not sure that's flip-floping, the solution to the overly complex expression is to turn into a function. Unless the tool is just being overly aggressive on what it considers too complex, in that case you can try to tweak the configuration instead.