Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Message-ID: <20020401080331.347.qmail@web20402.mail.yahoo.com>
Date: Mon, 1 Apr 2002 00:03:31 -0800 (PST)
From: Kief Morris <unknownyabanci@yahoo.com>
Reply-To: kief@bitbull.com
Subject: Re: [Collections] TreeNode, TreeIterator,
 IdentityHashSet proposed collections
To: Jakarta Commons Developers List <commons-dev@jakarta.apache.org>
In-Reply-To: <001e01c1d90a$6dd28ca0$e54218d4@oemcomputer>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Stephen, I have some questions/suggestions on your
TreeNode implementation.

It looks like ArrayTreeNode expects users to access
and manipulate the children of a node by returning a
List from getChildren(), which users then use to add,
delete, and access the child nodes. I don't think this
follows best practices as seen in the java.util
collections and other commons collections. 

I would suggest it would be better to keep the list
holding the children better hidden from the user.
Children can be added and removed with methods such as
TreeNode.addChild() and TreeNode.removeChild(). This
gives different implementations more flexibility in
how they store children. 

Where your inner class TreeArrayList now has to check
that the object passed in implements the appropriate
interface, instead TreeNode.addChild() could simply
take that interface as a parameter type and not take
Object, simplifying the code. TreeArrayList can be
dispensed with entirely, the methods implemented from
TreeNode would be able to enforce needed requirements,
such as setting the parent node.

A List or Collection of children can be returned from
TreeNode, but should be a defensive copy of the
internal list. It would be useful to add a method
which returns an Iterator for the children of the
node.

Also, the current implementation requires manual
creation of TreeNodes by user code. I would suggest
having methods which take an Object value parameter,
and handle TreeNode construction internally. In fact,
I would make these the default methods, with
Node-handling methods separately named. e.g.:

/**
 * Add a value to this node's list of children. A 
 * TreeNode is automatically created and its parent
 * set.
 * @return Whether the child List was changed.
 */
boolean addChild(Object);

/**
 * Add a child Node to this node's list of children.
Its
 * parent is automatically set, if it had a different
parent
 * previously it is removed from the old parent's list
 * of children.
 * @return Whether the child List was changed.
 */
boolean addChildNode(TreeNode.Internal);


A final suggestion, the base interface TreeNode could
assume that Collections rather than Lists are used
internally to store children, to allow a lighter
implementation for applications which don't care about
ordering. This would just mean removing the
getChild(int index) method.

A question, in ArrayTreeNode.toString() you use a
method called Reflection.getShortenedClassName(),
which I don't have anywhere in my system (using JDK
1.3). Where does this come from?

Here's my suggested TreeNode:

public interface TreeNode {

	/**
	 * Get the parent of this node
	 * @return the object that is the parent of this node
	 */
	TreeNode getParent();

        /**
         * Add a value to this node's list of
children. A 
         * TreeNode is automatically created and its
parent
         * set.
         * @return Whether the child List was changed.
         */
	boolean addChild(Object value);

        /**
         * Add a child Node to this node's list of
children. Its
         * parent is automatically set, if it had a
different parent
         * previously it is removed from the old
parent's list
         * of children.
         * @return Whether the child List was changed.
         */
	boolean addChildNode(TreeNode.Internal node);

	/**
	 * Remove the first child whose value equals the
supplied object.
	 * @return Whether the child List was changed.
	 */
	boolean removeChild(Object value);

	/**
	 * Remove a child node from this node's list of
children.
	 * @return Whether the child List was changed.
	 */
	boolean removeChildNode(TreeNode.Internal node);

	/**
	 * Get the child values of this node
	 * @return an iterator for the child node values.
	 */
	Iterator children();

	/**
	 * Get the nodes which are children of this node.
	 * @return an iterator for the child nodes.
	 */
	Iterator childNodes();
    
    /** 
     * Returns a Collection of the child values.
Modifying this
     * collection will not modify the internal
structure of the
     * parent TreeNode.
     */
    Collection getChildCollection();
    
    /**
     * Returns a Collection of the child nodes.
Modifying this
     * collection will not modify the internal
structure of the
     * parent TreeNode unless the parent value is
changed.
     */
    Collection getChildNodeCollection();

	/**
	 * Get a tree iterator from this node, returns
TreeNodes for
     * everything in the subtree below the current
node. Does not
     * include the node itself.
	 * Traversal is depth first.
	 * @return the tree iterator
	 */
	Iterator subtree();

	/**
	 * Is this a root node (has no parent)
	 * @return true if no parent
	 */
	boolean isRoot();

	/**
	 * Is this a leaf node (has no children)
	 * @return true if no children
	 */
	boolean isLeaf();

	/**
	 * Get the user object of this node
	 * @return the user object value
	 */
	Object getValue();
	
	/**
	 * Set the user object of this node
	 * @param object  the new user object value
	 */
	void setValue(Object object);

	/**
	 * TreeNode implementors should implement
TreeNode.Internal
	 * instead of just TreeNode. This interface hides the
public
	 * but unsafe setParent method
	 */	
	public interface Internal extends TreeNode {
    	/**
    	 * Set the parent of this node
    	 * @param node  the new parent node
    	 */
    	void setParent(TreeNode node);
	}
}


__________________________________________________
Do You Yahoo!?
Yahoo! Greetings - send holiday greetings for Easter, Passover
http://greetings.yahoo.com/

--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>