Bug 190115

This should improve performance of debugger views, since we would not have to refresh the nodes after we find that the initial children were wrong. Therefore adding dependent issue #185579.

Created attachment 101836 [details]
The API change with test included.

Please review the addition of
protected Children updateChildren(Children origChildren) method to Node.

Y01 Although making package private method protected is easy change, it also dangerous. Having the method protect allows much more than just decide whether a node is leaf or not. It allows for example to change children on each query to getChildren, etc. That is not your usecase, so please find some more conservative API change.

If you just want to defer check for isLeaf in a FilterNode.Children, do it in FilterNode.updateChildren directly, maybe with some new FilterNode constructor?

Y01 Well, I did not change a package private method, I've just made it final and added a new protected method Children updateChildren(Children origChildren)
I do not need to change FilterNode, I use a subclass of AbstractNode in debugger and since isLeaf() is final, there is no way to override it.
I've changed FilterNode in this patch just to use the newly added API.

It's true that Children updateChildren(Children origChildren) is called per every isLeaf() or getChildren() call. This allows more flexibility. FilterNode probably needs to be called every time for the case that children of the original node change. But if you think that this API change should solve just the requirement of initial children provider, I can change it to call updateChildren(Children) just once.

After a bit of brainstorming with Martin we'd like to explore following solution:

class Children {
  public static Children lazy(Callable<Children> factory) {

  }
}

one could take these children and pass them into constructor of Node. As soon as Node.updateChildren method is called, those children would be converted into real ones. By default all methods of the returned children would delegate to those created by the factory.

Created attachment 101939 [details]
New patch that introduces Children.lazy()

Children.lazy() factory method is introduced as was suggested. Test included. (lazy() methods have to be renamed in existing tests because of the collision with the newly introduced method).

I was surprised that you had to change lazy() to islazy() in all tests. But I guess it is because of clash with the new static method. OK. Possibly we could name the factory method differently. createLazy(...) or just create(...)?


The patch contains relicts from the previous attempt. E.g. Children updateChildren(Children) method (even annotated with @since tag). I think it is no longer necessary. Just do the check for LazyChildren in already existing updateChildren if possible.

Yes, the lazy() method in tests clashed with the new static method.
I like the short name, but maybe we should rename it to "createLazy" to prevent from similar possible clashes in existing implementations...

I've intentionally left there the package-private variant of the original change, since it can be handy for some future changes... but O.K., I'll simplify it as it was...

Created attachment 101957 [details]
The final patch attached.

Looks OK.

Thanks for the review, I'll push that change tomorrow.

Pushed as changeset:   177345:c6ca84f004ec
http://hg.netbeans.org/main/rev/c6ca84f004ec

Integrated into 'main-golden', will be available in build *201009110000* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/c6ca84f004ec
User: mentlicher@netbeans.org
Log: #190115 Introduce Children.createLazy() to provide node's children lazily.