Why is this an issue?
When List.remove() is called, the list shrinks, and the indices of all elements following the removed element are decremented by one.
If this operation is performed within a loop that iterates through the elements in ascending order, it will cause the loop to skip the element
immediately following the removed element.
How to fix it
There are three ways how to fix this issue:
- Replace the loop with a call to
Collection.removeIf(). This is the preferred solution.
- Replace the ascending loop with a descending loop. Use this approach if the preferred solution is not possible due to side effects of the loop.
- Adjust the loop counter within the loop body after the call to
Collection.remove(). This approach is not
recommended, because it will raise an issue with rule S127 - "for" loop stop conditions should be invariant
Code examples
Noncompliant code example
If the loop can be replaced with Java 8’s Collection.removeIf method, depending on the side effects of the loop and your Java target
version, then this is the preferred solution for this issue.
void removeFrom(List<String> list) {
// expected: iterate over all list elements
for (int i = 0; i < list.size(); i++) {
if (list.get(i).isEmpty()) {
list.remove(i); // Noncompliant, next element is skipped
}
}
}
Compliant solution
void removeFrom(List<String> list) {
list.removeIf(String::isEmpty); // Compliant
}
Noncompliant code example
If this is not possible due to side effects of the loop, replace the ascending loop with a descending loop. Descending loops are not affected by
decrementing the element indices after the removed element, because they have already been iterated.
void removeFrom(List<String> list) {
// expected: iterate over all list elements
for (int i = 0; i < list.size(); i++) {
if (list.get(i).isEmpty()) {
list.remove(i); // Noncompliant, next element is skipped
}
}
}
Compliant solution
void removeFrom(List<String> list) {
// expected: iterate over all list elements
for (int i = list.size() - 1; i >= 0; i--) {
if (list.get(i).isEmpty()) {
list.remove(i); // Compliant, elements after removed one have already been iterated
}
}
}
Noncompliant code example
Another way to solve this issue is to adjust the loop counter after the call to Collection.remove to account for the index
decrement.
void removeFrom(List<String> list) {
// expected: iterate over all list elements
for (int i = 0; i < list.size(); i++) {
if (list.get(i).isEmpty()) {
list.remove(i); // Noncompliant, next element is skipped
}
}
}
Compliant solution
This is not recommanded because it raises an issue with rule S127.
void removeFrom(List<String> list) {
// expected: iterate over all list elements
for (int i = 0; i < list.size(); i++) {
if (list.get(i).isEmpty()) {
list.remove(i); // Compliant due to counter adjust in next line
i--; // Noncompliant with S127!
}
}
}
Resources
Documentation
