Search code examples
javaandroidconcurrentmodification

Removing collection items without a ConcurrentModificationException using nested Iterators


I'm enduring this exception and after reading around I understand you can usually deal with it by using an Iterator. But having tried that I find it doesn't work in this scenario, perhaps because I have two nested loops? All the code is running on the main thread (Android).

 private boolean removeRoutesWithTimestampFromTeams(long time) {
    boolean routesRemoved = false;
    Iterator<WhizzyTeam> teamIterator = assignedTeams.iterator();
    while (teamIterator.hasNext()) {
        WhizzyTeam team = teamIterator.next();  // EXCEPTION HERE
        Iterator<Route> routeIterator = team.routes.iterator();
        while (teamIterator.hasNext()) {
            Route route = routeIterator.next();
            if (route.whizzy_assignment_time_in_millis == time) {
                team.removeRoute(route);
                routesRemoved = true;
            }
        }
        if (team.routes == null || team.routes.size() == 0) {
            assignedTeams.remove(team);
            unassignedTeams.add(team);
        }
    }
    return routesRemoved;

I'm relying on object references to identify objects, so copying collections is a very unattractive option. Is there any way to do what I need to do without resorting to copies?

===== WORKING CODE =====

I actually made three mistakes that are fixed here. 1. I used the same iterator in both loops (fixed in the code above too) 2. I should have used Iterator.remove() 3. Within removeRoute() there was another (unnecessary) for loop looping through routes to delete similar siblings

private boolean removeRoutesWithTimestampFromTeams(long time) {
    boolean routesRemoved = false;
    Iterator<WhizzyTeam> teamIterator = assignedTeams.iterator();
    while (teamIterator.hasNext()) {
        WhizzyTeam team = teamIterator.next();
        Iterator<Route> routeIterator = team.routes.iterator();
        while (routeIterator.hasNext()) {
            Route route = routeIterator.next();
            if (route.whizzy_assignment_time_in_millis == time) {
                routeIterator.remove();
                routesRemoved = true;
            }
        }
        if (team.routes == null || team.routes.size() == 0) {
            teamIterator.remove();
            unassignedTeams.add(team);
        }
    }
    return routesRemoved;
}

Solution

  • I see two issues with the snippet you posted. The first one you are using the outer iterator also int inner loop

      while (teamIterator.hasNext()) {
            WhizzyTeam team = teamIterator.next();  // EXCEPTION HERE
            Iterator<Route> routeIterator = team.routes.iterator();
            while (teamIterator.hasNext()) {
    

    I do think you should use routeIterator, in place of teamIterator in the second loop.

    Second you are not using the remove method from the Iterator interface to remove the item from your collection. From the doc

    The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.