Difference in behavior using normal pointer and a shared_ptr

Question

I have been trying to write an adjacency list program using c++. So For that I use a vector of list and each list contains the vertex whose adjacency list it is followed by other members connected to it in an undirected graph in terms of Node class below

Here is the Node structure I use:

typedef vector<shared_ptr<list<Node>>> VecSPLisNode;
typedef vector<Vertex> VecVertex;
class Graph {
    shared_ptr<VecVertex> mVertices;
    shared_ptr<VecSPLisNode> mVecNodeList;
    void addEdge(const Edge& e);
}

class Edge {
public:
    Vertex mStart, mEnd;
    int mWeight;
public:
    Edge(Vertex start=-1, Vertex end=-1, int wt=-1) :
        mStart(start), mEnd(end), mWeight(wt) {
    }
};
class Vertex {
public:
    int mId;
    bool mVisited;
public:
    Vertex(int info=-1, bool visited=false) : mId(info), mVisited(visited) {
    }
};

Now I want to save the same instance of vertex so thats its mVisited is updated in each of the list as and when Vertex is visited. So I want to use a pointer rather than save by value. The problem is if I save pointer to vertex above in Node as shared_ptr even when I update the pointer to a particular vertex as visited, in another list the vertex still comes up as unvisited. But same thing doesnt happen with Normal pointer. Below are two ways I did this.

shared_ptr way:

class Node {
public:
    shared_ptr<Vertex> vert;
    int weight;
    Node(shared_ptr<Vertex> v, int wt) :vert(v), weight(wt) {}
};

void Graph::addEdge(const Edge& e) {
        ((*mVecNodeList)[e.mStart.mId-1])->push_back(Node(make_shared<Vertex>((*mVertices)[e.mEnd.mId-1]), e.mWeight));
        ((*mVecNodeList)[e.mEnd.mId-1])->push_back(Node(make_shared<Vertex>((*mVertices)[e.mStart.mId-1]), e.mWeight));

}

Pointer way:

class Node {
public:
    Vertex* vert;
    int weight;
    Node(Vertex* v, int wt) :vert(v), weight(wt) {}
};

void Graph::addEdge(const Edge& e) {
        ((*mVecNodeList)[e.mStart.mId-1])->push_back(Node(&((*mVertices)[e.mEnd.mId-1]), e.mWeight));
        ((*mVecNodeList)[e.mEnd.mId-1])->push_back(Node(&((*mVertices)[e.mStart.mId-1]), e.mWeight));
    }

Answer 1

Should be just

void Graph::addEdge(const Edge& e) {
        ((*mVecNodeList)[e.mStart.mId-1])->push_back(Node((*mVertices)[e.mEnd.mId-1], e.mWeight));
        ((*mVecNodeList)[e.mEnd.mId-1])->push_back(Node((*mVertices)[e.mStart.mId-1], e.mWeight));

}

Your mistake is using make_shared - it takes object, makes copy of it on the heap and creates new shared pointer pointing to that copy, you essentially duplicate your object here. You just want to pass your shared pointer (by value or const ref).

P.S in regular pointer version there is no need for "&(*" operation either, it just works because they cancel out.

Answer 2

It is because make_shared<T>(arg...) is an optimized shorthand of std::shared_ptr<T>(new T(arg...)). You made a copy of Vertex here and you modified the copy.

• http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared

So you should replace make_shared<Vertex>((*mVertices)[e.mEnd.mId-1]) with (*mVertices)[e.mEnd.mId-1].