Bug 190115 - Add a possibility to update node children when someone asks for them.
Add a possibility to update node children when someone asks for them.
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Nodes
7.0
PC Linux
: P3 (vote)
: 7.0
Assigned To: Martin Entlicher
issues@platform
: API, API_REVIEW_FAST
Depends on:
Blocks: 185579
  Show dependency treegraph
 
Reported: 2010-09-02 16:37 UTC by Martin Entlicher
Modified: 2010-09-11 03:38 UTC (History)
0 users

See Also:
Issue Type: ENHANCEMENT
:


Attachments
The API change with test included. (5.62 KB, patch)
2010-09-02 16:44 UTC, Martin Entlicher
Details | Diff
New patch that introduces Children.lazy() (31.12 KB, patch)
2010-09-08 14:27 UTC, Martin Entlicher
Details | Diff
The final patch attached. (7.56 KB, patch)
2010-09-09 06:58 UTC, Martin Entlicher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Entlicher 2010-09-02 16:37:03 UTC
Children need to be passed to the Node's constructor. Therefore it's not possible to provide Children lazily when someone asks for them.
This causes problem e.g. in debugger's viewmodel where we have to decide if the node is a leaf node or not for every node that is created, even when nobody ask for the children or leaf property.

A mechanism that can be used to provide node's children lazily is necessary.
Comment 1 Martin Entlicher 2010-09-02 16:39:10 UTC
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.
Comment 2 Martin Entlicher 2010-09-02 16:44:31 UTC
Created attachment 101836 [details]
The API change with test included.
Comment 3 Martin Entlicher 2010-09-02 16:45:39 UTC
Please review the addition of
protected Children updateChildren(Children origChildren) method to Node.
Comment 4 Jaroslav Tulach 2010-09-03 08:05:26 UTC
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?
Comment 5 Martin Entlicher 2010-09-06 12:41:43 UTC
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.
Comment 6 Jaroslav Tulach 2010-09-06 14:44:15 UTC
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.
Comment 7 Martin Entlicher 2010-09-08 14:27:00 UTC
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).
Comment 8 Jaroslav Tulach 2010-09-08 15:15:12 UTC
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.
Comment 9 Martin Entlicher 2010-09-09 06:38:00 UTC
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...
Comment 10 Martin Entlicher 2010-09-09 06:58:06 UTC
Created attachment 101957 [details]
The final patch attached.
Comment 11 Jaroslav Tulach 2010-09-09 08:45:49 UTC
Looks OK.
Comment 12 Martin Entlicher 2010-09-09 15:12:22 UTC
Thanks for the review, I'll push that change tomorrow.
Comment 13 Martin Entlicher 2010-09-10 17:03:06 UTC
Pushed as changeset:   177345:c6ca84f004ec
http://hg.netbeans.org/main/rev/c6ca84f004ec
Comment 14 Quality Engineering 2010-09-11 03:38:08 UTC
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.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo