I'm facing a design issue in my program. I have to manage Nodes object which are part of a root ChainDescriptor.
Basically it looks like the following:
class ChainDescriptor
{
public:
~ChainDescriptor()
{
//delete the nodes in nodes...
}
void addNode(Node *);
Node * getNode();
const std::list<Node *>& getNodes() const;
std::list<Node *> m_nodes;
};
class Node
{
public:
Node(Node *parent);
void addChild(Node *node);
Node * getChild(const std::string& nodeName);
private:
Node * m_parent;
std::list<Node*> m_childs;
};
The ChainDescriptor class owns all the nodes and is responsible of deleting them. But these classes need now to be used in another program, a GUI with undo/redo capabilities, with the problematic of the "ownership". Before modifying the existing code in depth, I'm considering the different solutions:
- using
shared_ptr
and respectivelist<shared_ptr<...> >
- using
weak_ptr
and respectivelist<weak_ptr<...> >
In the example above, I don't really know where to use shared_ptr
and weak_ptr
properly.
Any suggestion?
You can use
shared_ptr
form_childs
andweak_ptr
form_parent
.However, it might be still reasonable to retain the raw pointer to the parent
Node
and don't use any weak pointers at all. The safeguarding mechanism behind this is the invariant that non-null parent always exists.Another option is using
shared_ptr
inChainDescriptor
only and retaining all raw pointers inNode
. This approach avoids weak pointers and has a clean ownership policy (parent nodes own their children).Weak pointers will help you to manage the memory automatically, but the backside of this are fuzzy ownership logic and performance penalties.
shared_ptr
is owning smart pointer andweak_ptr
is referencing smart pointer.So in your situation I think the
ChainDescriptor
should useshared_ptr
(it owns the nodes) andNode
should useweak_ptr
form_parent
(it only references it) andshared_ptr
form_childs
(it owns them).Trying to just replace raw pointers with some sort of smart pointer will in general not work. Smart pointers have different semantics than weak pointers, and usually, these special semantics need to be taken into account at a higher level. The "cleanest" solution here is to add support for copy in
ChainDescriptor
, implementing a deep copy. (I'm supposing here that you can cloneNode
, and that all of theNode
are always owned by aChainDescriptor
.) Also, for undo, you may need a deep copy anyway; you don't want modifications in the active instance to modify the data saved for an undo.Having said that, your nodes seem to be used to form a tree. In this case,
std::shared_ptr
will work, as long as 1) allNode
are always "owned" by either aChainDescriptor
or a parentNode
, and 2) the structure really is a forest, or at least a collection of DAG (and, of course, you aren't making changes in any of the saved instances). If the structure is such that cycles may occur, then you cannot useshared_ptr
at this level. You might be able to abstract the list of nodes and the trees into a separate implementation class, and haveChainDescriptor
keep ashared_ptr
to this.(FWIW: I used a reference counted pointer for the nodes in a parse tree I wrote many years ago, and different instances could share sub-trees. But I designed it from the start to use reference counted pointers. And because of how the tree was constructed, I was guaranteed that there could be no cycles.)
The usual implementation would be for each node to have strong reference to its child (i.e. keeps them alive), and each child to have a weak reference back to the parent.
The reason for this is to avoid circular references. If only strong references were used, then you'd have a situation where the parent refcount never drops to zero (because the child has a reference), and the child refcount never drops to zero (because the parent has a reference).
I think your
ChainDescriptor
class is okay to use strong references here though.