threerings/playn — Issue #4: Possible issue with GroupLayerImpl
Author: tmjee
Created: 2011-12-05 01:02:21+00:00
State: open

Currently this is how GroupLayerImpl looks like

/** This group's children. */
public List
children = new ArrayList
();

/**
* @return the index into the children array at which the layer was inserted (based on depth).
*/
public int add(GroupLayer self, L child) {
// if this child has equal or greater depth to the last child, we can append directly and avoid
// a log(N) search; this is helpful when all children have the same depth
int count = children.size(), index;
if (count == 0 || children.get(count-1).depth() <= child.depth()) {
index = count;
} else {
// otherwise find the appropriate insertion point via binary search
index = findInsertion(child.depth());
}

// remove the child from any existing parent, preventing multiple parents
if (child.parent() != null) {
child.parent().remove(child);
}
children.add(index, child);
child.setParent(self);
child.onAdd();
return index;
}

There might be a potential issue of getting ArrayIndexOutOfBound when doing
children.add(index, child);

let's say the children is 5 in size, and the last one children[4] has a depth less than the child we are adding, so the index will be 5 and then we try to do
children.add(5, child)

we'll get ArrayIndexOutOfBound exception, it says in the javadoc of ArrayList
IndexOutOfBoundsException - if the index is out of range (index < 0 || index > size())

Possible fix, if this is an issue would be instead of
children.add(index, child);

we do
if (index >= children.size()) {
children.add(child); // this will add child to the last in the list anyway
}
else {
children.add(index, child);
}

Can some developer verify this please?