Can not convert to for-each loop when collection is modified while looping

[Luro02]

Summary

For example, the following code will throw an exception:

import java.util.ArrayList;
import java.util.List;

public class Main {
    public static void main(String[] args) {
        var list = new ArrayList<>(List.of("a", "toRemove", "b", "toRemove"));
        
        for (var e : list) {
            if (e.equals("toRemove")) {
                list.remove(e);
            }
        }
        
        System.out.println(list);
    }
}

which would be suggested with

import java.util.ArrayList;
import java.util.List;

public class Main {
    public static void main(String[] args) {
        var list = new ArrayList<>(List.of("a", "toRemove", "b", "toRemove"));
        
        for (int i = 0; i < list.size(); i++) {
            var e = list.get(i);
            if (e.equals("toRemove")) {
                list.remove(e);
            }
        }
        
        System.out.println(list);
    }
}

Lint Name

FOR_CAN_BE_FOREACH

Reproducer

<code>

[lunagl]

The original code is most likely a bug (elements will be skipped), autograder should suggest using Iterator.remove instead

[Luro02]

Technically, the "original code" is not really the original and the suggested code isn't really suggested.

Currently it only suggests something like, "hey your for loop, should be a for-each loop" and doesn't give any detailed suggestions.

Iterator.remove is in my opinion not something that a first year student can figure out themselves, and to be honest, I don't know how that would be done with a for-each loop either?

var iterator = list.iterator();
// NOTE: this crashes when the iterator is empty
for (var value = iterator.next(); iterator.hasNext(); value = iterator.next()) {
    // call iterator.remove here when necessary
}

For now I will make an exception for any code that modifies the collection while looping. This is the way it seems to be in IntelliJ as well.

In the future one could make the above (if worth the time, I don't think collections are modified frequently in loops) into a separate check.

[lunagl]

Iterator.remove is in my opinion not something that a first year student can figure out themselves, and to be honest, I don't know how that would be done with a for-each loop either?

Exactly, that's the problem. So when faced with the issue of removing things while iterating, students will come up with creative (but bad) solutions using regular for loops that are error prone and often simply incorrect.

This is why I would definitely include some reference to Iterator.remove (and maybe Collection.removeIf) when a modification in the loop is detected, to make this correct way of doing things discoverable by students.