An if whose only statement is another if
ID |
objectivec.maintainability.collapsible_if |
Severity |
info |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Control Flow |
Language |
Objective-C |
Description
Reports a braced if whose body holds one statement, itself a braced if, where neither if carries an else and both conditions fit on their own line. A body with any further statement before or after the inner if, an else on either if, an inner if written without braces, and a copy of the shape inside a comment are not reported.
Rationale
The two conditions have to hold together for anything to happen, so the nesting encodes an && in the shape of the code rather than in the expression, and the reader pays for it twice: once tracking an extra level of indentation, and once checking whether the outer block really does end where the inner one does before concluding no other case is handled. The nesting also invites a later edit to add a statement to the outer block, at which point the two branches quietly stop being equivalent and the intent is lost.
The following code illustrates the pattern detected by this rule:
@implementation Uploader
- (void)uploadIfPossible {
// FLAGGED: An if whose only statement is another if
if (self.connected) {
if (self.queue.count > 0) {
[self flushQueue];
}
}
}
- (void)send:(id)item { }
- (void)flushQueue { }
- (void)recordAttempt { }
- (void)recordEmptyQueue { }
- (void)recordOffline { }
@endRemediation
Join the two conditions with && and keep a single if. Where the combined condition grows too long to read, give one half a name - a local BOOL or a small predicate method on the receiver - and combine the named halves. Where the outer condition is a precondition for the whole method, an early return on its negation removes the nesting instead.