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

Mike Duigou mike.duigou at oracle.com
Fri Jun 14 19:45:31 UTC 2013


On Jun 14 2013, at 05:41 , Remi Forax wrote:

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

I would consider it bad form at minimum and probably buggy. Why insist on walking right next to the minefield? Sometimes minefields are poorly mapped (spec ambiguity), mines are misplaced (bugs) or you might stray (your bugs).

What is the objection to the much safer and general:

List<Integer> l = new ArrayList<>(Arrays.asList(1, 2, null));
for(ListIterator<Integer> each = l.listIterator(); each.hasNext();) {
  Integer i = each;
  if(i == null) {
     each.set(3);
  }
}

Just the lack of for(:) loop?

Mike


More information about the core-libs-dev mailing list