if / else led by a negated condition
ID |
objectivec.maintainability.inverted_logic |
Severity |
info |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Control Flow |
Language |
Objective-C |
Description
Reports an if whose condition is a single negation and which has a plain else branch, so the two blocks could be swapped and the ! dropped. A negation combined with && or || is not reported, because removing it there means rewriting the whole condition rather than swapping two blocks. A negated if with no else, a chain ending in else if, and an else with an empty body are not reported either.
Rationale
Reading if (!ready) { … } else { … } costs the reader an extra inversion before they can say which block runs in the ordinary case, and the ordinary case is the one they are usually looking for. The negative branch also comes first, so the exceptional path is what the eye lands on and the main path is the afterthought. In a method with several of these, each one has to be un-negated separately to follow the flow, and the negation is easy to lose when a condition is later extended or extracted.
The following code illustrates the pattern detected by this rule:
@implementation Router
- (void)routeFlag:(BOOL)ready {
// FLAGGED: if / else led by a negated condition
if (!ready) {
[self wait];
} else {
[self go];
}
}
- (void)wait {
}
- (void)go {
}
- (BOOL)isReady {
return YES;
}
@endRemediation
Drop the ! and swap the two blocks, so the positive case is stated first and reads directly. Where the negative case is short and leaves the method, an early return is usually better still: keep the guard as if (!ready) { return; } and let the rest of the body run unindented, which removes the else entirely.