Loop advances a different variable from the one it initialises
ID |
objectivec.correctness.jumbled_incrementer |
Severity |
low |
Remediation Complexity |
medium |
Remediation Risk |
medium |
Remediation Effort |
low |
Resource |
Control Flow |
Language |
Objective-C |
Description
Reports a for header whose initialiser sets one variable and whose update clause advances a different one - for (column = 0; column < n; row++). Only headers with a single initialiser and a single update are reported, so a loop advancing two cursors together and a loop that deliberately initialises one variable and advances another alongside it are left alone, as are fast enumeration and a copy of the shape inside a comment.
Rationale
The variable the loop sets up is never advanced, so whatever the condition tests keeps the value it started with unless something in the body happens to change it. In the nested form this rule most often finds - an inner loop whose update advances the outer index because the header was copied from the loop above and only half renamed - the inner loop either never terminates or terminates on the wrong bound, and the outer loop skips iterations because its index was advanced from inside. Neither symptom points back at the header, so the loop is usually rewritten several times before anyone reads the update clause.
The following code illustrates the pattern detected by this rule:
@implementation Grid
- (void)fillRows:(NSInteger)rows columns:(NSInteger)columns {
for (NSInteger row = 0; row < rows; row++) {
// FLAGGED: Loop advances a different variable from the one it initialises
for (NSInteger column = 0; column < columns; row++) {
[self fillRow:row column:column];
}
}
}
- (void)fillRow:(NSInteger)row column:(NSInteger)column { }
- (void)visit:(NSInteger)index { }
- (void)visitItem:(id)item { }
@endRemediation
Advance the variable the condition tests. In a nested loop, check every clause of the copied header against the index the inner loop owns, not just the declaration. Where two variables genuinely have to move together, update both in the header - for (i = 0, j = n; i < j; i++, j--) - so the intent is stated rather than left to the body.