Community
Participate
Working Groups
I think the following is a fairly common idiom in EOL: for(element in collection) { if (satisfiesSomeCondition(element)) { delete element; } } where "satisfiesSomeCondition" is a predicate. Currently, the above code will cause a concurrent modification exception to be raised. It'd be nice if we could avoid the need to raise a concurrent modification exception for this case. Three possible solutions: 1) As suggested by Steffen Zschaler (http://www.eclipse.org/forums/index.php?t=msg&th=173542&start=0&S=64723b96997117861839468e72bbd7c9): have foreach clone every element in the collection. 2) Map EOL foreach to a Java while structure. 3) Allow the following to parse: collection.forAll(|e| delete e ); or collection.delete(|e| satisfiesSomeCondition(e));
Just a few comments: on 1) There's actually no need to do a deep clone. All that needs to be cloned is the actual collection structure, there is no problem if it references the same elements. In fact, you would probably want it to, as otherwise you would end up with two copies of each model element and delete would probably delete the wrong one :-) So, a better way to code this maybe for (BElemType a : new Collection(b)) { ... } on 2) The problem is not really the use of for or while. The problem is in using an iterator and modifying the collection it iterates over. on 3) This looks neat. Of course, there already is collection.excluding :-) Steffen
Thanks for the feedback and clarification Steffen. 1) Yes, a shallow clone makes a lot more sense - thanks! 2) I should have mentioned: I was thinking of a while that doesn't use an iterator. Something like: while (not collection.isEmpty()) { element = collection.first; // execute body of the foreach loop } 3) I think allowing the delete keyword in the body of forAll might be better than a new method on Collection: I agree that there may be confusion between a new method Collection#delete and Collection#excluding (there's also Collection#remove, Collection#exlcudingAll, Collection#removeAll)
(In reply to comment #2) > 3) I think allowing the delete keyword in the body of forAll might be better > than a new method on Collection: I agree that there may be confusion between a > new method Collection#delete and Collection#excluding (there's also > Collection#remove, Collection#exlcudingAll, Collection#removeAll) Hang on, if there already is a Collection#remove, shouldn't that be enough? Or is this currently not available to EOL users? On a related note, I'm not sure allowing delete inside forAll is such a good idea: forAll is meant to check a predicate over a collection (at least I assume that's true for EOL as well) and not modify the collection. So allowing delete would mean quite a substantial change to forAll's semantics. Steffen
(In reply to comment #3) > (In reply to comment #2) > > 3) I think allowing the delete keyword in the body of forAll might be better > > than a new method on Collection: I agree that there may be confusion between a > > new method Collection#delete and Collection#excluding (there's also > > Collection#remove, Collection#exlcudingAll, Collection#removeAll) > > Hang on, if there already is a Collection#remove, shouldn't that be enough? Or > is this currently not available to EOL users? Delete is somewhat different to Collection#remove. Delete removes the model element from the model. (So, in EMF, delete will remove the model element from the underlying EResource). Collection#remove doesn't remove the model element from the resource, and is useful for moving elements from one collection to another. Incidentally, attempting to save a model after calling Collection#remove can fail (e.g. dangling hrefs in EMF), but saving a model after calling delete won't cause these types of error. > > On a related note, I'm not sure allowing delete inside forAll is such a good > idea: forAll is meant to check a predicate over a collection (at least I assume > that's true for EOL as well) and not modify the collection. So allowing delete > would mean quite a substantial change to forAll's semantics. I'm not sure if it's good practice, but I use EOL's Collection#forAll to inline small loops. Often I disregard the return value. For example: for (e in myCollection) { e.name.println(); } becomes myCollection.forAll(e|e.name.println());
One further point to consider... if we go for option (1), a shallow copy of the collection, I *think* we may need to do some extra work to ensure that the properties of the collection are preserved. For example, when iterating over an OrderedSet, we may need to ensure that the clone is also an OrderedSet, and not some other type of collection that doesn't preserve ordering and uniqueness.
Really, you would only need to preserve properties that would be exhibited through the iterator (mainly ordering, I guess). Any modifications would go to the original collection anyway. Steffen
Dimitris and I have discussed this offline, and we've decided to add some documentation to cover this behaviour, and not to change anything else. We haven't decided on a good solution yet, but will leave the bug open for further discussion. In short, the main concern with the proposed solutions are that they may introduce unexpected or (hard to understand) results in future passes over a loop. Also, allowing a delete statement in the body of forAll wouldn't avoid the problem, as forAll is implemented as a Java for loop.
It seems that we already have a reasonable solution for this: when a collection is used as the argument to the delete statement, all elements of that collection are deleted. So for the original problem: for(element in collection) { if (satisfiesSomeCondition(element)) { delete element; } } A solution is: delete collection.select(element|satisfiesSomeCondition(element)); This approach also works plain-old collections: delete collection; delete Animal.all; So, I'm closing this bug as resolved. Feel free to re-open if you'd like some other action to be taken.
Fixed in 0.9.0.
