std::unique_ptr, Default Copy Constructor, and Abstract Class

Question

I have class representing a tree object that uses unique pointers, some nodes that make up the tree, and a function that constructs pointers to an abstract node class based on some arguments (It makes pointers to subclasses, since abstract node is abstract)

class AbstractNode
{
    vector<unique_ptr<AbstractNode>> children;
public:
    AbstractNode(arguments...);
    // other stuff...
};
class Tree
{
    unique_ptr<AbstractNode> baseNode;
    // other stuff...
}
unique_ptr<AbstractNode> constructNode(AbstractNodeTypes type);

There are a variety of subclasses of abstractNode that will be contained in the tree. The subclasses provide different implementations for some virtual functions in that class.

I want to be able to copy my tree by creating a new set of nodes with the same class types that are distinct copies of the nodes in the original tree.

---

Here is the problem:

If I write my own copy constructor for the AbstractNode class that deep copies the children, I'll have to write copy constructors for all of the subclasses of AbstractNode, which seems annoying since the only thing that won't copy correctly are the children pointers. It will also be annoying to use copy constructors here because I will need to cast the children to the correct types before I call them, I think.

Is there some way I can get the compiler to let me use the default copy constructor to set up everything except for the children. It can leave those as null pointers or something? Then I can write a simpler function that just recursively adds children to copy a tree.

If that is impossible, is there any non-ugly solution to this problem that anyone knows of?

Answer 1

The typical way to solve this problem is to have a virtual clone function similar to what Kerrek SB describes in his answer. However I would not bother writing your own value_ptr class. It is simpler to just reuse std::unique_ptr as your question presents. It will require a non-default copy constructor in AbstractNode, but does not require explicit or unsafe casting:

class AbstractNode
{
    std::vector<std::unique_ptr<AbstractNode>> children;
public:
    AbstractNode() = default;
    virtual ~AbstractNode() = default;
    AbstractNode(AbstractNode const& an)
    {
        children.reserve(an.children.size());
        for (auto const& child : an.children)
            children.push_back(child->clone());
    }

    AbstractNode& operator=(AbstractNode const& an)
    {
        if (this != &an)
        {
            children.clear();
            children.reserve(an.children.size());
            for (auto const& child : an.children)
                children.push_back(child->clone());
        }
        return *this;
    }

    AbstractNode(AbstractNode&&) = default;
    AbstractNode& operator=(AbstractNode&&) = default;
    // other stuff...

    virtual
        std::unique_ptr<AbstractNode>
        clone() const = 0;
};

Now a ConcreteNode can be implemented. It must have a valid copy constructor which may be defaulted depending upon what data members ConcreteNode adds to the mix. And it must implement clone(), but that implementation is trivial:

class ConcreteNode
    : public AbstractNode
{
public:
    ConcreteNode() = default;
    virtual ~ConcreteNode() = default;
    ConcreteNode(ConcreteNode const&) = default;
    ConcreteNode& operator=(ConcreteNode const&) = default;
    ConcreteNode(ConcreteNode&&) = default;
    ConcreteNode& operator=(ConcreteNode&&) = default;
    // other stuff...

    virtual
        std::unique_ptr<AbstractNode>
        clone() const override
        {
            return std::unique_ptr<AbstractNode>(new ConcreteNode(*this));
        }
};

I suggest having clone return a unique_ptr instead of a raw pointer just to ensure that there is no chance that a new'd pointer is ever exposed without an owner.

For completeness I've also shown what the other special members would look like.

At first I was thinking that C++14's make_unique would be nice to use here. And it can be used here. But personally I think in this particular example it really doesn't carry its weight. Fwiw, here is what it would look like:

    virtual
        std::unique_ptr<AbstractNode>
        clone() const override
        {
            return std::make_unique<ConcreteNode>(*this);
        }

Using make_unique you have to first construct a unique_ptr<ConcreteNode>, and then rely on the implicit conversion from that to unique_ptr<AbstractNode>. This is correct, and the extra dance probably will get optimized away as soon as inlining is fully enabled. But the use of make_unique just seems like an unnecessary obfuscation here when what you really clearly need is a unique_ptr<AbstractNode> constructed with a new'd ConcreteNode*.