On 06/14/2013 09:45 PM, Mike Duigou wrote:
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 <fo...@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?
I don't think so, I think the issue is usually because the real code is
more something like:
List<Integer> l = new ArrayList<>(Arrays.asList(1, 2, null));
for (Integer i : l) {
blob.foo(l, i);
}
and in another module somewhere in the application
void bar(List<Integer> l, Integer i) {
if (i == null) {
l.set(l.size() - 1, 3); // change the last value to 3
}
and it appear that sometimes blob.foo calls several layers of methods
that end up to call bar.
So while obviously your code is how to fix the problem, usually people
don't even notice the problem
because the code of the loop and the code that does the mutation are far
away from each other.
Mike
Rémi
