HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Interface for a tree node in Qt

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
interfacefortreenode

Problem

I have the following C++ interface (in Qt) for a tree node, inspired somewhat by Valve's implementation of a culling octree:

```
/**
* @brief Interface for a recursive tree node.
*/
class ITreeNode
{
public:
/**
* @brief Virtual destructor.
*/
virtual ~ITreeNode() {}

/**
* @brief Adds a child node to this node.
* @param node Node to add.
*/
virtual void addChild(ITreeNode* node) = 0;

/**
* @brief Removes the child node at the given index from this node.
* @param index Index of child to remove.
* @return Child removed, or NULL if the index was invalid.
*/
virtual ITreeNode* removeChild(int index) = 0;

/**
* @brief Removes the given child node if it exists.
* @param node Node to remove.
*/
virtual void removeChild(ITreeNode* node) = 0;

/**
* @brief Returns the child at a given index.
* @param index Index of the child.
* @return Child at this index, or NULL if the index was invalid.
*/
virtual ITreeNode* childAt(int index) const = 0;

/**
* @brief Returns whether the given node is recorded as a child of this node.
* @param node Node to search for.
* @return True if the given node is recorded as a child; false otherwise, or if the node provided is NULL.
*/
virtual bool containsChild(ITreeNode* node) const = 0;

/**
* @brief Returns whether this node is an ancestor of the given node.
* @note This means that this node is present further up the path from the root node to the given node.
* @param node Node to check ancestry of.
* @return True if this node is an ancestor of the given node, false otherwise or if the given node is NULL.
*/
virtual bool isAncestor(const ITreeNode* node) const = 0;

/**
* @brief Returns whether this node is a successor of the given node.
* @note This means that the given node is present further up the path from the root node to this node.
* @

Solution

Is this interface complete? Does it attempt to do too much or too little?

In my opinion, interfaces should be as small as possible, providing only methods that are really required to be called without knowing (or casting to) the actual subclass. I don't know how you use it, but my first impression is: your interface contains too many methods...


Are there situations in which functionality such as this wouldn't necessarily require an interface?

An interface is useful if you have two or more classes that are implemented differently but require the same interface. In your case I don't think that the actual subclasses will implement this interface with significant differences. (Actually I don't see any methods I would expect to behave differently)

So maybe instead of an interface a real base class would be the better choice to avoid code duplicates. You could implement a treenode class (implementing all those methods itself) and inherit from it to only change/add the different behaviours... (Again I can only guess as I don't see your subclasses)


Is it better to have the implementing class manage all children in memory, or allow them to be passed in externally?

Well, your children's nodes are passed in externally (and I think that's the only way that makes sense) - you only have to decide whether to let the parent node take over ownership of its children or not.

As long as you don't share single instances of your nodes between different trees (or at least different parents) or reuse them in any other way, I would prefer the parents to take ownership (delete them when they are no longer needed).

Objects taking ownership of their children is also a well known practice in Qt. As you're using Qt, I would recommend mimicking this behaviour.

Context

StackExchange Code Review Q#46334, answer score: 6

Revisions (0)

No revisions yet.