-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Description
New Issue Checklist
- Updated SwiftLint to the latest version
- I searched for existing GitHub issues
New rule request
- Why should this rule be added? Share links to existing discussion about what
the community thinks about this.
I find myself regularly disabling the "Cyclomatic complexity" rule for method that only contain a switch, and are really easy to understand.
While discussing this with fellow developers, I was told and given a link about Cognitive Complexity, which aims exactly that : replace cyclomatic complexity to provide a metric best suited to evaluate the difficulty to understand a method. It has the added benefit of evaluating types as well as methods.
I don't have a lot of experience with it by lack of current tooling to mesure it, but for my methods that currently trigger cyclomatic complexity, it makes sense.
White paper: Cognitive Complexity
- Provide several examples of what would and wouldn't trigger violations.
Only methods having a score higher than the given threshold would trigger a violation.
The first exemple of the paper is the following 2 methods. They have the same cyclomatic complexity (4), but different cognitive complexity.
int sumOfPrimes(int max) {
int total = 0;
OUT: for (int i = 1; i <= max; ++i) { // +1
for (int j = 2; j < i; ++j) { // +2 (for: +1, nesting: +1)
if (i % j == 0) { // +3 (if: +1, nesting: +2)
continue OUT; // +1 (break in the flow: +1)
}
}
total += i;
}
return total;
} // Cognitive complexity 7
String getWords(int number) {
switch (number) { // +1
case 1:
return "one";
case 2:
return "a couple";
case 3:
return “a few”;
default:
return "lots";
}
} // Cognitive complexity 1
It tends to favour code structures easier to read. For exemple switch are easier to read than there if {} else if {} counterpart, it has less noise, fewer repetition of the same variables.
- Should the rule be configurable, if so what parameters should be configurable?
The rule would have 2 thresholds: the first for warnings, the second for errors.
They should have a default value, and be configurable.
- Should the rule be opt-in or enabled by default? Why?
In my opinion it should be enabled by default at the end of the road. I use a linter to keep my code understandable, and cognitive complexity precisely try to answer that. And at first look, it does a decent job at that.
But since it is not widely known and use, it is probably wise to have it opt-in for a while, to evaluate its relevance, and find proper default thresholds.