Boolean expression with five or more operands
ID |
objectivec.maintainability.expression_too_complex |
Severity |
info |
Remediation Complexity |
medium |
Remediation Risk |
low |
Remediation Effort |
medium |
Resource |
Complexity |
Language |
Objective-C |
Description
Reports a boolean expression that chains five or more operands with the same logical operator - four or more &&, or four or more ||, between the start of the statement and its terminating ; or {. The expression may be written over several lines. Operators inside a string literal do not count, and neither do operators in a comment.
Rationale
A reader has to hold every operand and the whole truth table in mind at once to work out when a long boolean chain is true, and to decide which operand a fix or a new requirement belongs to. The number of combinations a test suite needs in order to exercise the condition grows with each operand, so the long chains are exactly the conditions that end up partly untested. A chain that long is usually testing several independent things at once, each of which deserves a name.
The following code illustrates the pattern detected by this rule:
@implementation Gate
- (BOOL)allowUser:(NSString *)name a:(BOOL)a b:(BOOL)b c:(BOOL)c d:(BOOL)d e:(BOOL)e {
// FLAGGED: Boolean expression with five or more operands
if (a && b && c && d && e) {
return YES;
}
// FLAGGED: Boolean expression with five or more operands
if (a ||
b ||
c ||
d ||
e) {
return YES;
}
// FLAGGED: Boolean expression with five or more operands
BOOL allowed = a && b && c && d && e;
// FLAGGED: Boolean expression with five or more operands
if ([name isEqualToString:@"root"] && a && b && c && d) {
return NO;
}
if (a && b && c && d) {
return YES;
}
if ((a && b) || (c && d)) {
return YES;
}
// Two separate statements, two operators each.
BOOL first = a && b;
BOOL second = c && d;
// if (a && b && c && d && e) {
NSLog(@"if (a && b && c && d && e) {");
/* if (a && b && c && d && e) { */
return allowed && first && second;
}
@endRemediation
Extract a coherent group of the operands into a local BOOL with a name that states what it means, or into a small predicate method on the receiver - BOOL hasValidSession = a && b && c; followed by if (hasValidSession && d && e). Where the operands guard genuinely unrelated concerns, split the condition into separate statements so each can fail on its own terms.