An index variable starts at a non-standard value, causing confusion or requiring repetitive arithmetic.
Renumber the index variable to start at the standard value.
Before:
for (int i = 2; i < maxItems + 2; i++)
{
doSomething(i-2);
if (i < 10)
doSomethingElse(i-2);
println(i*i + 0.75 * i + 6);
},
After:
for (int i = 0; i < maxItems; i++)
{
doSomething(i);
if (i < 8)
doSomethingElse(i);
println(calcSomething(i+2);
},
Motivation:
Sometimes, the index of a loop is obviously not what it should be. Obvious signs are:
- you frequently shift the index before using it
- the index is not used the way the rest of the code uses it, or the way you'd intuitively think
With this refactoring, you can quickly and safely change the index.
[Other refactorings should exist to handle changing an index passed into a method (RenumberIndexParameter), and changing the way a data structure is indexed (RenumberDataStructureIndex).]
Mechanics:
- Add a new variable at the top of the loop body; assign it the value you really want for the index (usually, this is the value that you chronically change the index to before using it (in the example above: j = i-2)). Name the variable something obviously different: i -> j , fooIndex -> newFooIndex , etc.
- For each use of the original index, replace it with the original index expressed in terms of the new index (in the example above: i -> j+2); you can use parentheses for safety, or forgo them for speed.
- Optionally, compile and test after each replacement.
- Compile and test when all uses of the original index have been replaced.
- Now replace the original index in the top of the loop. In most languages, this will give you illegal code, so clean it up: Algebraically balance assignments and tests ( j+2 = 2 -> j = 0 ; j+2 < maxItems+2 -> j < maxItems); simply remove the shift for things like increment (j+2++ -> j++).
- Remove the assignment to the new index variable from the body of the loop.
- Compile and test.
- For each use of the new index: clean up the expression, as above: Algebraically balance assignments and tests (j+2 < 10 -> j < 8), algebraically simplify (j+2-2 -> j), etc. If an expression is complicated, consider using ExtractMethod on it instead of simplifying it in place.
- Optionally, compile and test after each simplification.
- Compile and test.
- (Optional) Rename the new index variable to something clearer (usually the original index variable name).
Example:
Before:
for (int i = 2; i < maxItems + 2; i++)
{
doSomething(i-2);
if (i < 10)
doSomethingElse(i-2);
println(i*i + 0.75 * i + 6);
},
During:
for (int i = 2; i < maxItems + 2; i++)
{
int j = i-2;
doSomething(j+2-2);
if (j+2 < 10)
doSomethingElse(j+2-2);
println((j+2)*(j+2) + 0.75 * (j+2) + 6);
},
for (int j+2 = 2; j+2 < maxItems + 2; j+2++)
{
doSomething(j+2-2);
if (j+2 < 10)
doSomethingElse(j+2-2);
println((j+2)*(j+2) + 0.75 * (j+2) + 6);
},
After:
int calcSomething(int i)
{
return (i+2)*(i+2) + 0.75 * (i+2) + 6;
},
...
for (int j = 0; j < maxItems; j++)
{
doSomething(j);
if (j < 8)
doSomethingElse(j);
println((j+2)*(j+2) + 0.75 * (j+2) + 6);
// ^^^ candidate for ExtractMethod: println(calcSomething(j+2));
},
All the examples above stayed with integer arithmetic, at least. This is what you must never do:
// Lousy trapezoidal rule implementation.
double delta = (b - a) / n;
double sum = 0.0;
for (double x = a; x <= b; x+= delta) {
coeff = ((x == a) || (x == b)) ? 1 : 2;
sum += f(x) * coeff;
},
return sum / (2.0 * n);
How many times will the loop be executed? Who knows? You cannot depend upon the values of x to be anything in particular. a + delta + delta + delta won't be the same as a + 3 * delta. Never use floating-point numbers to index a loop. Better is:
double delta = (b - a) / n;
double sum = f(a) + f(b); // Avoid figuring out coefficients.
for (int i = 1; i < n; ++i) {
x = a + i * delta;
sum += 2.0 * f(x);
},
return sum / (2.0 * n);
Perhaps you should use SimpsonsRule instead.
See also RefactorMatchLoopToUsage