How to store a list of smart pointers inside a c++ class?

Question

I have a class Node that must have a list of its input Edge's. These input Edge's, however, are not meant to be modified by Node, only accessed. I have been advised to use smart pointers for that matter, like this:

class Node
{
private:
    std::vector<std::unique_ptr<Edge>> inEdges;
//...
public:
    void inline AddEdge(std::unique_ptr<Edge>& edge) // could be const here too
    {
        this->inEdges.push_back(edge);
    }
//...
}

So in the runtime I can create a list of nodes and edges and assign edges to each node:

int main()
{
    std::vector<std::unique_ptr<Nodes>> nodes;
    std::vector<std::unique_ptr<Edges>> edges;
    nodes.push_back(std::make_unique<Node>(0, 0.5, 0.0));
    nodes.push_back(std::make_unique<Node>(1, 0.5, 0.0));
    edges.push_back(std::make_unique<Edge>(*nodes[0], *nodes[1], 1.0));
    nodes[1]->AddEdge(edges[0]);
}

The compiler gives the error

Error 1 error C2280: 'std::unique_ptr>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)' : attempting to reference a deleted function c:\program files (x86)\microsoft visual studio 12.0\vc\include\xmemory0 593 1 sirs

It used to work with raw pointers inside the Node's std::vector<Edge*>, given that the signature of AddEdge would be void AddEdge(Edge& edge);, pushing back &edge inside the vector.

What is wrong with my code? How should I proceed to correct it? Given that std::vector cannot store references, because these are not assignable.

PS: I do not want to transfer ownership of the pointers to the Node objects...

Answer 1

You should only move instances of std::unique_ptr in order to get them in a std::vector. Otherwise you need std::shared_ptr.

class Node
{
private:
    std::vector<std::unique_ptr<Edge>> inEdges;
//...
public:
    void AddEdge(std::unique_ptr<Edge>&& edge)
    {
        inEdges.push_back(std::move(edge));
    }
//...
}

I've changed AddEdge to take an r-value reference in order to support move semantics properly.

To call:

node.AddEdge(std::move(edge));

Or:

node.AddEdge(std::make_unique<Edge>(/*args*/));

As an aside. If you do find you're passing std::unique_ptr by reference, it might be a sign you should be using std::shared_ptr.

Also, inline is redundant on methods declared inside a class.

Answer 2

std::vector::push_back copies its argument, and std::unique_ptr can't be copied, it can only be moved.

You'll have to move the passed std::unique_ptr:

void inline AddEdge(std::unique_ptr<Edge> edge)
{
    this->inEdges.push_back(std::move(edge));
}

Then call it like so:

nodes[1]->AddEdge(std::move(edges[0]));

---

Also, inEdges doesn't have to be a vector for std::unique_ptr, as they should be non-owning. A std::unique_ptr implies ownership, but as you said, Node doesn't own the Edges.

Use std::vector<Edge*> instead (or std::observer_ptr when it is implemented in the near future), as raw pointers (normally) imply non-ownership. You just have to be careful that the lifetime of the std::vector<Edge*> doesn't exceed the lifetime of the std::unique_ptrs, or else the pointers will be dangling.

Answer 3

You are trying to push a copy of the std::unique_ptr into the vector. The vector is attempting to create a copy of the unique_ptr, which isn't allowed since it would defeat the purpose of unique ownership.

You should first consider which object owns which object, and only use std::unique_ptr if there is indeed a case for unique ownership.

There are a couple things you can do:

1. std::move() the pointer into the vector, which will cause the previous copy to become invalid (see: move semantics).
2. Use std::shared_ptr if you want multiple objects to hold copies of the pointer. Since it looks like you want to have multiple copies of the pointer, this is probably the direction you want to go.

Its probably worth while in your case to read up on std::weak_ptr since I think you may want objects to refer to one another, without creating circular references. Weak pointers work on shared_ptr's without actually incrementing the reference count, and are important in some cases where you want to make sure objects aren't hanging onto references too long and creating memory leaks.

Hope this helps.