[SalesForce] Adding/removing elements from a `Map` while iterating over it

Background

I'm working on a bugfix for some code that I've written. The code itself generally works fine (bugfix is for an edge case), and the exact code isn't terribly important for this question.

The gist of my code is

Map<Id, SObject> recordsToWorkOn = trigger.newMap.deepClone();
Set<Id> firstRoundOfExclusions = findExclusionsMethod();

recordsToWorkOn.keySet().removeAll(firstRoundOfExclusions);

for(SObject obj :recordsToWorkOn.values()){
    if(secondExclusionTest()){
        // Throw an exception if called async
        // Otherwise, add an error to the record and continue
    }
}

// Other processing logic that shouldn't operate on records excluded in either the 
//   first or second rounds of exclusion.

Question

One of the ways I can think of to perform my second round of exclusions is to remove the key from the map while I'm still iterating over the map.

The following anonymous apex appears to work as intended

Map<Id, Opportunity> testOppMap = new Map<Id, Opportunity>([SELECT Id FROM Opportunity LIMIT 20]);

Integer i = 0;

// Size should be 20, matching the LIMIT statement in the query above
system.debug('size: ' + testOppMap.size());
system.debug(testOppMap);

for(Opportunity opp :testOppMap.values()){
    // Remove every other Opportunity from the Map
    if(Math.mod(i,2) == 0){
        testOppMap.keySet().remove(opp.Id);
    }
    i++;

    system.debug('Opp ' + i + ' in loop: ' + opp.Id);
}

// Size should now be 10
system.debug('size: ' + testOppMap.size());
system.debug(testOppMap);

Size before the loop shows 20, each Opportunity is still available in the Opp loop variable for the entire iteration of the loop after being removed from the Map, size after the loop shows 10.

My question is this:

Is it a good idea to add/remove items from a Map as we iterate over it?

My gut feeling says that this is probably a bad idea (though now that I think about it, map.keySet().removeAll() must do something similar).

It's easy enough to gather Ids of records that match my second exclusion criteria, and removeAll() outside of my loop. I'm just curious to know whether or not there is anything to look out for (edge cases, un-intuitive behavior, etc…).

Best Answer

In addition to my previous answer here, the documentation says that you should not remove elements of a current collection:

To remove elements while iterating a list, create a new list, then copy the elements you wish to keep. Alternatively, add the elements you wish to remove to a temporary list and remove them after you finish iterating the collection.

Modifying a collection during iteration is not explicitly supported, so you should always create a new map/list or remove them all afterwards (e.g. store keys to remove in a set, then use removeAll after the loop).

Related Topic