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

Paul Sandoz paul.sandoz at oracle.com
Fri Jun 14 10:55:38 UTC 2013

On Jun 14, 2013, at 12:12 PM, Remi Forax <forax at univ-mlv.fr> wrote:

The following does not throw CME: List l = new ArrayList<>(Arrays.asList(1, 2)); for (Integer i : l) { l.remove(1); // 2 is never encountered } Where as the following does: List 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.

More information about the core-libs-dev mailing list