GoogleCodeArchive / piccolo2d

Automatically exported from code.google.com/p/piccolo2d
0 stars 0 forks source link

PNode#moveToFront change to avoid java.util.ConcurrentModificationException #164

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
If any code loop through the children, nobody can call child#moveToFront.

If it is possible, I wish that you change the method like this:

public void moveToFront() {
    PNode p = parent;
    if (p != null) {
        int i = p.indexOfChild(this);
        int j = p.getChildrenCount()-1;
        Collections.swap(p.getChildrenReference(), i, j);
    }
}

I hope it does the same.

Original issue reported on code.google.com by zimmerma...@gmail.com on 25 Feb 2010 at 3:26

GoogleCodeExporter commented 9 years ago
I vote to mark this as WontFix.

The method is throwing ConcurrentModificationException because that's what is
happening, you are trying to modify the list as you are iterating over it.

The javadoc for PNode.getChildrenReference() reads "This list should not be 
modified."

http://www.piccolo2d.org/doc/piccolo2d.java/release-1.2.1/apidocs/edu/umd/cs/pic
colo/PNode.html#getChildrenReference%28%29

A workaround would be to copy the children list into a new ArrayList

List children = new ArrayList(node.getChildrenReference());
for (Iterator iterator = children.iterator(); iterator.hasNext(); )
{
  PNode child = (PNode) iterator.next();
  child.moveToFront();
}

Original comment by heue...@gmail.com on 26 Feb 2010 at 5:34

GoogleCodeExporter commented 9 years ago
See also Issue 166.

Original comment by heue...@gmail.com on 9 Apr 2010 at 9:33

GoogleCodeExporter commented 9 years ago
Alternative is to return the child collection using Collection.unmodifiableList
wouldn't require a complete copy.

Original comment by allain.lalonde on 9 Apr 2010 at 10:08

GoogleCodeExporter commented 9 years ago
immutable? love this idea.

Original comment by mr0...@mro.name on 9 Apr 2010 at 10:19

GoogleCodeExporter commented 9 years ago
Should I modify getChildrenReference to user Collections.unmodifiableList ?

Original comment by allain.lalonde on 17 Apr 2010 at 2:54

GoogleCodeExporter commented 9 years ago
Immutability would be safest, but since the method is named 
getChildrenReference, it
is possible that some client code is relying on the ability to directly 
manipulate
the list of children.  Therefore, it could be confusing to have this return a
defensive list copy.

Original comment by samrr...@gmail.com on 17 Apr 2010 at 3:18

GoogleCodeExporter commented 9 years ago
Agreed that if a client is directly manipulating the reference, their code will
break, but since getChildReference is documented explicitly as "do not touch", I
think that's ok.

Original comment by allain.lalonde on 17 Apr 2010 at 3:21

GoogleCodeExporter commented 9 years ago
OK if we mark as WontFix then?

Original comment by heue...@gmail.com on 24 Aug 2010 at 2:48

GoogleCodeExporter commented 9 years ago
agreed

Original comment by allain.lalonde on 24 Aug 2010 at 3:45

GoogleCodeExporter commented 9 years ago

Original comment by heue...@gmail.com on 24 Aug 2010 at 3:46