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

[Remi Forax]

On 06/14/2013 12:55 PM, Paul Sandoz wrote:
> 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?

In the codes I was referring to, there was always a way to know that the 
remove was done at the end by example by knowing that the last element 
was a special sentinel or by using a counter.
So is the following program bugged ?

List<Integer> l = new ArrayList<>(Arrays.asList(1, 2, null));
for (Integer i : l) {
   if (i == null) {
     l.set(l.size() - 1, 3);  // change the last value to 3
   }
}


>> 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?

buggy code or not buggy code, that is the question.

>
> Paul.

Rémi