Node.removeAllChildren() is implemented like this:
/**
*
* <code>detachAllChildren</code> removes all children attached to this
* node.
*/
public void detachAllChildren() {
if(children != null) {
for ( int i = children.size() - 1; i >= 0; i-- ) {
detachChildAt( i );
}
logger.info("All children removed.");
}
}
In my opinion, detachAllChildren should create a new children list, instead of emptying the existing one. I think that because if you do:
Afterwards your childrenOfNodeNow is empty too, which is not what user might expect.
So I guess that breaking the Node contract is a bit hardcore now so I suggest:
1) Change my variable name from "childrenOfNodeBeforeDetach" to "childrenOfNodeInTheVeryMomentYouUseMe" :D.
2) Add a couple of comments to code, specifying that:
a) getChildren returns the actual list of cildren managd by the node.
b) detachAllChildren empties the existing list without creating a new one, so beware if you keep external references to that list.
Anyway I have to say that, despite how much I like jME, code comments could improve a lot ;)
jME returns references to the internal data structures of the scene graph in a lot of areas, so really what is needed is either a rule that you need to always copy data that needs to persist after scenegraph changes, or change jME to actively protect its internal data (so no more holding references to internal transform variables, etc.)
i dislike the idea of creating garbage. if you replace the children list by a new one, everyone holing the old list will work on an outdated list and won't be notified of any changes.
Protecting internal data is a major refactoring. Also, if we want to keep it fast it would imply making Node (and other core classes) iterators themselves:
Node.nextChildren();
Node.getCurrentChildren();
Node.resetChildrenIndex();
Node.getChildren(int n);
…
I don't like this pattern. Also, returning references to internal structures is much faster, I don't see a problem as long as it is clarified in Javadocs. Current version is better because at least user can copy the list himself. If structures were protected, there would be not way to access them and we might not wish to be so restrictive.
For the removeAllChildren issue, I also think replacing the list is not an option either. However, I fail to see why anyone should currently hold a reference to those lists, given they are internal data structures. IMO they should be only used for inline purposes.
But anyway, after all, it's an API and user will need to learn to use it. In brief, I stick to my suggestion of over-clarifying this kind of things in code comments and method naming.
Defensive copying of the internal data structures would IMO be too slow for jME. Imagine a Controller for a skeleton with ~30 Bones - the Controller would first get all the Bones by doing a recursive hierarchy walk through the skeleton (creating 25 defensively copied child lists), then get each Bones' location, rotation and maybe even scale (creating 90 copied Vector3fs and Quaternions), modifying them, and setting the modified values in the Bones (another 90 copies). That's over 200 Object creations per frame for one character's skeleton. Then there's the matter of deforming all attached meshes - would their geometry buffers be copied as well?
I know that short lived objects can be optimized a lot, but have just recently seen a case where that didn't seem to happen (does anybody remember HamsterOfDeath's recent demonstration of autoboxing slowness?)
Also defensive protection of geometry buffers would make data sharing impossible - that would just be a disaster for my MMO project, especially the SharedKeyframeController I was just about to release which allows me to render roughly 50 times as many animated meshes than the original KeyframeController does.
Plus, as HamsterofDeath already pointed out, there is currently no way of being notified when the data changes in the object we obtained it from.
Some hefty disadvantages IMO, and on the pro side, I can only see the single advantage of not confusing programmers who are not yet fully aware of the differences/implications of passing "by value" vs "by reference". I think that a link to a good programming tutorial would help those folks a lot more than preventing them from seeing what they are lacking.
I may well be overlooking some easy remedies to get rid of the disadvantages or some other advantages. Please enlighten me!
Another typically used approach is to have two accessor methods… one set/getRef* and another set/getCopy* which do as their names imply… it is completely up to the programmer to decide whether they need really a copy, or a reference. It clutters the API, but is fairly easy to remember.