Unexplained numeric literal
ID |
objectivec.maintainability.magic_number |
Severity |
info |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Numeric Literal |
Language |
Objective-C |
Description
Reports a decimal integer literal of three or more digits compared against inside an if or while condition, or used as the initialiser of a mutable variable. Declarations marked const are the fix rather than the defect and are not reported, and neither are #define directives, hexadecimal literals - which are masks and flags rather than quantities - or the round scale factors 100, 1000, 10000, 100000, 1000000 and 1024.
Rationale
A bare number carries no unit and no reason. A reader meeting 86400 has to work out that it is a day in seconds, and cannot tell whether the 86400 in another file is the same quantity or a coincidence - so when the value has to change, some occurrences get updated and others do not. In a condition the problem is sharper still, because the literal is the threshold that decides the behaviour and there is nothing to grep for when someone asks where that threshold is set.
The following code illustrates the pattern detected by this rule:
@implementation Session
- (BOOL)refreshWithRetries:(NSInteger)retries elapsed:(NSInteger)elapsed {
// FLAGGED: Unexplained numeric literal
if (retries > 300) {
return NO;
}
// FLAGGED: Unexplained numeric literal
while (elapsed < 5000) {
elapsed++;
}
// FLAGGED: Unexplained numeric literal
NSInteger timeout = 86400;
// FLAGGED: Unexplained numeric literal
static NSInteger cachedWindow = 3600;
if (retries > 3) {
return NO;
}
if (elapsed == 0) {
return NO;
}
// A named constant is the fix, not the defect.
static const NSInteger kSecondsPerDay = 86400;
const NSInteger maxWindow = 500;
NSInteger budget = kMaxItems;
// Round scale factors carry their own meaning.
NSInteger millis = 1000;
NSInteger page = 1024;
// Hexadecimal literals are masks and flags, not quantities.
NSUInteger mask = 0xFF00;
// NSInteger timeout = 86400;
NSLog(@"NSInteger timeout = 86400;");
NSLog(@"%ld %ld %ld %ld %ld %ld %lu", (long)timeout, (long)cachedWindow, (long)kSecondsPerDay,
(long)maxWindow, (long)budget, (long)millis + (long)page, (unsigned long)mask);
return YES;
}
@endRemediation
Give the value a name once and use the name everywhere - static const NSTimeInterval kSessionLifetime = 86400;, or a FOUNDATION_EXPORT constant when it has to be shared across files. Where the quantity is a duration, prefer a type that carries the unit, such as NSTimeInterval with a named constant built from 60 * 60 * 24, so the derivation is visible rather than folded into a single opaque number.