[riena-dev] TreeRidget questions and comments

["Volanakis, Elias" <evolanakis@xxxxxxxxxxxxxx>]

Hi,

I've been working on the TreeRidget this week. Check out the example
client to see it in action. Below are a few questions and remarks.


1. It is not clear to me what treeRidget.setSelection(object) (and
similar methods) should do. Is the object a TreeNode or is the object
the actual value that is wrapped in the tree node? How is this handled
in the swing implementation? I could not find an example where this is
used.

Example:

Object someValue;
DefaultObservableTreeNode root = new
DefaultObservableTreeNode(someValue);
DefaultObservableTreeModel model = DefaultObservableTreeModel(root);
treeRidget.bindToModel(model);

(a) treeRidget.setSelection(root);   or
(b) treeRidget.setSelection(someValue)

Personally, I tend to vote for using the nodes (a). This is easier to
implement, because all nodes are unique. If we go for (b) we have to
think what happens when "someValue" is wrapped into multiple different
tree node instances. I understand that from the POV of the developer (b)
may appear to be easier, because you may be able skip "unwrapping" the
value from the node.


2. Similarly, it is not clear to me what treeRidget.setSelection(index)
should do? How is this handled in the swing implementation? 

For trees, selecting by index is tricky, because the index is bound to
change when the tree structure changes. 

treeRidget.setSelection(new int[] { 2, 5, 6 });

(a) do we want to support this for trees? 

(b) if we want it, what is the unique numbering scheme (depth first,
breadth first, do we count elements that are collapsed - probably not
because otherwise lazy loading could become problematic, etc.)

(c) a scheme that is easier to handle could be ridget.select(parentNode,
childIndex) however we have to change the interface for that

Personally would prefer not to support it at all (a).


3. Both implementations (SWT/Swing) currently silently ignore
programmatic collapse / expand calls, when the ridget is not bound to a
model. This seems to be desired since it is defined this way in the
tests.

treeRidget.expandTree(); // (a) does nothing
treeRidget.bindToModel(root);
treeRidget.expandTree(); // (b) expands the tree

What is the motivation for that? From my POV an exception in case (a)
would be preferable. The same would apply to #collapseTree();
#expand(treeNode); #collapse(treeNode)


4. Can we add the #getUserObject() from DefaultTreeNode /
DefaultObservableTreeNode to the ITreeNode interface? Currently it only
exists on the implementation class and creates the need for casting. 

Example:

ITreeNode treeNode = new DefaultObservableTreeNode(someValue);
Object value = null;
if(treeNode instanceof DefaultObservableTreeNode) {
  value = ((DefaultObservableTreeNode).getUserObject());
}

>From my POV this only makes sense if they are nodes that do not wrap a
value. Is that likely? Also I would prefer getValue() instead of
getUserObject().


5. What are the plans regarding the various implementations of
ITreeNode/ITreeModel in the org.eclipse.riena.ui.ridgets.tree package?
Is that something we will support later, or is it stuff that could be
removed?

If you look at the hierarchy there are basically two important classes
DefaultObservableTreeNode and DefaultTreeNode. The first supports events
when the node data and/or structure is changed, making it easy to keep
the tree up to date. The second has a few specialized subclasses like
DynamicTreeNode, ToolTipTreeNode that are not used outside of the tree
package (at least in the code I have). It seems *currently* possible
that we could use DefaultObservableTreeNode everywhere (instead of
DefaultTreeNode) and get rid of the DefaultTreeNode and subclasses. Of
course this may not make sense if we plan to support these things later.


Looking forward to your input.

Regards,
Elias.

---
Elias Volanakis
Technical Lead
Innoopract, Inc.
351 NW 12th Avenue
Portland, Oregon 97209
Tel: +1-503-552-1457
Fax: +1-503-715-4915
Mobile: +1-503-929-5537
evolanakis@xxxxxxxxxxxxxx
http://rapblog.innoopract.com