High Cognitive Complexity
ID |
java.cognitive_complexity_methods_high |
Severity |
high |
Remediation Complexity |
medium |
Remediation Risk |
low |
Remediation Effort |
medium |
Resource |
Complexity |
Language |
Java |
Tags |
complexity, maintainability |
Description
Reports a function whose control flow is harder to read than the configured budget — maxCognitiveComplexity, 15 by default. Unlike a raw branch count, this measure weighs where
a branch sits: every construct that breaks the normal top-to-bottom reading flow (if, for,
while, do, switch, catch, a ternary, a mixed &&/|| condition) costs one point, and
each extra level of nesting a construct sits at costs one more. A well-named helper call costs
nothing, however much logic it hides.
Rationale
Two functions with the same number of branches are not equally hard to read if one nests them
five deep and the other lays them out flat with early returns. Nesting is what actually taxes a
reader — at each level they must keep every enclosing condition in mind to know whether the
current line even runs. A single if at the top of a method is trivial; the same if buried
inside three other conditionals is not, even though a plain branch count treats them the same.
Mixing && and || in one condition pays an extra point on top of the branch itself, because a
reader has to stop and work out how the two operators group — precedence makes a || b && c
read differently than it groups. A run of the same operator (a && b && c) reads as a single
idea and costs only one point.
A well-picked function name is free: calling out to a clearly-named helper costs nothing, no matter how much branching that helper hides. This is what makes "extract a method" the standard fix — it does not remove complexity, it relocates it to a place where the reader can choose not to look at it right now.
equals(Object) and hashCode() overrides are exempt: comparing or hashing every field with a
chain of conditions is the idiom for those two methods, not a smell.
class Quote {
double price(User user, Order order) { // FLAW -- cognitive complexity 17
if (user.isActive()) { // +1 (depth 0)
if (order.isRush() || order.isFragile()) { // +2 (depth 1), +1 (one "||" run)
if (user.hasDiscount() && user.hasCredit()) { // +3 (depth 2), +1 (one "&&" run)
for (Item item : order.items()) { // +4 (depth 3)
if (item.isTaxed()) { // +5 (depth 4)
return order.total() * 0.9;
}
}
}
}
}
return order.total();
}
double simplerPrice(User user, Order order) { // OK -- flattened, well under the budget
if (!user.isActive()) {
return order.total();
}
if (!(order.isRush() || order.isFragile())) {
return order.total();
}
return isEligibleForRushDiscount(user) ? order.total() * 0.9 : order.total();
}
private boolean isEligibleForRushDiscount(User user) {
return user.hasDiscount() && user.hasCredit();
}
}Remediation
-
Return early. Handle the exceptional or trivial case first and return, instead of wrapping the rest of the method in an
if. Each early return removes one level of nesting for everything that follows it. -
Extract a named predicate. A compound boolean condition promoted to a small, well-named method reads as a sentence at the call site and is scored on its own, separately, wherever it is used.
-
Split the function. A function that legitimately does several distinct things is easier to read — and to test — as several small functions than as one large one with many branches.