RFR : 8016446 : (m) Add override forEach/replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap

[Paul Sandoz]

On Jun 14, 2013, at 12:12 PM, Remi Forax <forax at univ-mlv.fr> wrote:
>> 
>> The following does not throw CME:
>> 
>>             List<Integer> l = new ArrayList<>(Arrays.asList(1, 2));
>>             for (Integer i : l) {
>>                 l.remove(1);  // 2 is never encountered
>>             }
>> 
>> Where as the following does:
>> 
>>             List<Integer> l = new ArrayList<>(Arrays.asList(1, 2, 3));
>>             for (Integer i : l) {
>>                 l.remove(1);
>>             }
>> 
>> Because the hasNext implementation does not check for modification. It's sad this also occurs for the default implementation of Iterable.forEach :-(
>> 
>> This behaviour sucks.
> 
> devil advocate: why exactly, the iteration is finished when you remove the element ?

The latter because a CME is thrown; the former because hasNext returns false.

The above is an example of how a bug can be hidden depending on the state (# elements) of the collection.


> 
>> It would be a shame for overriding forEach/forEachRemaining implementations to conform to such behaviour when they can implement stronger/consistent failure guarantees.
> 
> While I could agree with you in theory, in practice I have seen several times codes that rely on this behaviour,
> usually there is a bunch of method calls between the for loop and the list.remove() so this is not something that can be easily fixed.

A bug none the less, yes?


> And because I think it's more important that users should be able to use either for(:) or forEach without thinking too much,
> because otherwise nodoby will "modernize" their code, we have no choice but stick to the iterator behaviour for forEach
> i.e. no modCount check at the end.
> 

So you argument is based on the the premise that existing buggy code should continue work?

Paul.