foreach and concurrent modifcation

[aldacron]

I've not seen this documented anywhere, though admittedly I've read little about the intimate details of 1.5 features. When using the new foreach syntax on a collection, you'll run into trouble with code like this:


for(SomeObject o : someCollection)
{
   if(!o.someCondition())
       someCollection.remove(o);
}


The above will throw a ConcurrentModificationException. I suppose that's because the foreach is working with an Iterator behind the scenes, which of course isn't happy that you tried to remove the object directly.

I don't see this as being a bug, as that's how iterators are supposed to behave - but it surely is annoying. It cost me about 30 minutes today trying to figure out how the hell I was concurrently modifying a list from a single thread. So whenever you need to conditionally remove an item from collection in a loop like that, use an iterator rather than the
foreach. And of course, use clear() to empty the whole thing at once after a loop.

[blahblahblahh]

Presumably there is some way to access the underlying iterator's .remove() method?

[rreyelts]

Presumably there is some way to access the underlying iterator's .remove() method?

No.

God bless,
-Toby Reyelts

Games published by our own members! Check 'em out!

[princec]

So the whole feature is about as useful as a chocolate fireguard then. As expected.

Cas

[rreyelts]

So the whole feature is about as useful as a chocolate fireguard then. As expected.

I wouldn't say it is useless, but it is very light syntactic sugar. It might be nice if they extended the syntax so that you could specify the name of the iterator, too. Something like:

1   2   3

// declares an Iterator<Foo> fooIt, and assigns it the value of foos.iterator(); for ( Foo f : fooIt : foos ) { }

God bless,
-Toby Reyelts

[puf]

I was just thinking about this a few days ago. The easiest way to solve the problem is to add the objects to a secondary collection during the forEach and then remove them all in one go after the loop.

So it should be something like this:

Collection<SomeObject> removableObjects = new Collection<SomeObject>();
for (SomeObject o: someCollection)
{
 if (!o.someCondition())
   removableObjects.add(o);
}
someCollection.removeAll(removableObjects);

It actually allows the JVM to more efficiently remove the objects all in one go, although I doubt whether it will make much difference at the moment.

            Puf

[blahblahblahh]

It actually allows the JVM to more efficiently remove the objects all in one go, although I doubt whether it will make much difference at the moment.

            Puf

Isn't that highly dependent upon data structure, though?

For instance, isn't it much better to iterate over a huge linked list only once, removing as you go, than to iterate over it twice, but with a reduced number of reference re-assignments (assuming best-case scenario, that many removed items are contiguous)?

If the items are discontiguous, so that exactly the same number of ops are needed for the removes in each case, then the double iteration is always going to be much slower.

Bascially, it's shockingly short-sighted syntactic sugar. Sigh. /me wonders what happened in the java decision process to let *this* through

[puf]

You're right, it is HIGHLY dependant on the exact implementation.  I probably shouldn't have made a claim that far fetched. Sorry about that.

But I happen to like the syntactic sugar that the forEach is offering, so I'm looking for workarounds for the problems. Since I've had similar problems with standard Iterators, I was looking for a solution that works in all cases. Having a second collection of objects should always work, although not at the best possible speed.

Given the available development time saved versus the deployment time lost, it might be worth using in some cases.

[swpalmer]

The foreach thing is nice for when you don't need to modify the collection as you go.  It's as simple as that. If you need to modify the collection then simply use the iterator as you would with 1.4.... but you can use Generics this time to make it a little more typesafe.