Is It Ever Good Practice To Modify The Index Variable Inside a FOR Loop?
Given the code:
for (int i = 1; i <= 5; i++)
{
// Do work
}
Is is ever acceptable to change the value of i
from within the loop?
For example:
for (int i = 1; i <= 5; i++)
{
开发者_StackOverflow中文版 if( i == 2)
{
i = 4;
}
// Do work
}
In my opinion, it is too confusing. Better use a while
loop in such case.
It is acceptable, however, I personally think this should be avoided. Since it's creating code that will be unexpected by most developers, I find that it's causing something much less maintainable.
Personally, if you need to do this, I would recommend switching to a while loop:
int i=1;
while (i <= 5)
{
if (i == 2)
i = 4;
++i;
}
This, at least, warns people that you're using non-standard logic.
Alternatively, if you're just trying to skip elements, use continue:
for (int i = 1; i <= 5; i++)
{
if (i == 2 || i == 3)
continue;
}
While this is, technically, a few more operations than just setting i
directly, it will make more sense to other developers...
YES
You see that frequently in apps that parse data. For example, suppose I'm scanning a binary file, and I'm basically looking for certain data structures. I might have code that does the following:
int SizeOfInterestingSpot = 4;
int InterestingSpotCount = 0;
for (int currentSpot = 0; currentSpot < endOfFile; currentSpot++)
{
if (IsInterestingPart(file[currentSpot])
{
InterestingSpotCount++;
//I know that I have one of what I need ,and further, that this structure in the file takes 20 bytes, so...
currentSpot += SizeOfInterestingSpot-1; //Skip the rest of that structure.
}
}
An example would be deleting items which match some criteria:
for (int i = 0; i < array.size(); /*nothing*/)
{
if (pred(array[i]))
i++;
else
array.erase(array.begin() + i);
}
However a better idea would be using iterators:
for (auto it = array.begin(); it != array.end(); /*nothing*/)
{
if (pred(*it))
++it;
else
it = array.erase(it);
}
EDIT
Oh sorry, my code is C++, and the question is about C#. But nevertheless the idea is the same:
for (int i = 0; i < list.Length; /*nothing*/)
{
if (pred(list[i]))
i++;
else
list.RemoveAt(i);
}
And a better idea might be of course just
list.RemoveAll(x => !pred(x));
Or in a slightly more modern style,
list = list.Where(pred);
(here list
should be IEnumerable<...>
)
I would say yes, but only in a specific cases.
It may be a bit confusing - if I set i=4
will it be incremented before the next iteration or not?
It may be a sign of a code smell - maybe you should do a LINQ query before and only process relevant elements?
Use with care!
Yes it can be. As there are an extremely enormous amount of possible situations, you're bound to find one exception where it would be considered good practice.
But stopping the theoretica lside of things, i'd say: no. Don't do it.
It gets quite complicated, and hard to read and/or follow. I would rather see something like the continue
statement, although i'm not a big fan of that either.
Personally, I would say that if the logic of the algorithm called for a normally-linearly-iterating behavior, but skipping or repeating certain iterations, go for it. However, I also agree with most people that this is not normal for loop usage, so were I in your shoes, I'd make sure to throw in a line or two of comments stating WHY this is happening.
A perfectly valid use case for such a thing might be to parse a roman numeral string. For each character index in the string, look at that character and the next one. If the next character's numeric value is greater than the current character, subtract the current character's value from the next one's, add the result to the total, and skip the next char by incrementing the current index. Otherwise, just add the current character's value to the running total and continue.
An example could be a for
loop where you want in a certain condition to repeat current iteration or go back to a previous iteration or even skip a certain amount of iterations (instead of a numered continue
).
But these cases are rare. And even for these cases, consider that the for
loop is just one means among while
, do
and other tools that can be used. so consider this as bad practice and try to avoid it. your code will also be less readable that way.
So for conclusion: It's achievable (not in a foreach
) but strive to avoid this using while
and do
etc. instead.
Quoting Petar Minchev:
In my opinion, it is too confusing. Better use a while loop in such case.
And I would say by doing that, you must be aware of some things that could happen, such as infinite loops, premature-canceled loops, weird variable values or maths when they are based on your index, and mainly (not excluding any of the others) execution flow problems based on your index and other variabes modified by the fail loop.
But if you got such a case, go for it.
精彩评论