user1553248 February 2016

c++ push_back copying object instead of reference

I'm having some issues understanding a problem I've mentioned in the comments below:

class Node {
    public:
        Node(int value): value(value), neighbors() {}
        int value;
        std::vector<std::shared_ptr<Node>> neighbors;
};

    std::vector<Node> nodes;
    nodes.reserve(50);
    for(int j = 0 ; j < 50; ++j) {
        nodes.push_back(Node(j));
    }
    for(int j = 1 ; j < 25; ++j) {
        nodes.at(0).neighbors.push_back(std::make_shared<Node>(nodes.at(j)));
    }

    //The following line is copying nodes.at(0) instead of just 
    //copying a reference to nodes.at(0). Why?
    nodes.at(15).neighbors.push_back(std::make_shared<Node>(nodes.at(0)));

    for(int j = 25 ; j < 50; ++j) {
        nodes.at(0).neighbors.push_back(std::make_shared<Node>(nodes.at(j)));
    }

    std::cout << nodes.at(15).neighbors.at(0).neighbors.size();
    //Prints out 24

    std::cout << nodes.at(0).neighbors.size();
    //Prints out 49

Why is the following line copying nodes.at(0) (which returns a reference to the first element of nodes vector) instead of storing a reference to it?

    nodes.at(15).neighbors.push_back(std::make_shared<Node>(nodes.at(0)));

Answers


kiran February 2016

Vector always stores the copies. You can find more information here : Is std::vector copying the objects with a push_back?


n.m. February 2016

Nothing in your code stores references. Your code copies a node because you allocate a new node in std::make_shared and invoke the copy constructor.

  std::vector<Node> nodes;

It's local to your function. It would be impossible to keep either pointers (shared or not) or references to elements of this vector. Perhaps you want to use a vector of shared pointers instead:

 std::vector<std::shared_ptr<Node>> nodes;

Bear in mind that shared_ptr doesn't work well in presence of cyclic referenced. If your data structure is a general graph, then perhaps shared_ptr is not appropriate for storing neighbours. You may want to consider weak_ptr instead (you will have to keep a container of shared pointers to all nodes separately).


José February 2016

// The following line is copying nodes.at(0) instead of just 
nodes.at(15).neighbors.push_back(std::shared_ptr<Node>(&nodes.at(0))); 

// Notice I didn't call make_shared, because make_shared woul allocate anew Node,
// but you already have allocated your node in your nodes vector (which is global).

// 24 as expected
std::cout << nodes.at(0).neighbors.size() << std::endl;
// 24 as well
std::cout << nodes.at(15).neighbors.at(0)->neighbors.size() << std::endl;

//Insert another element
nodes.at(0).neighbors.push_back(std::make_shared<Node>(nodes.at(30)));

// 25 as expected
std::cout << nodes.at(0).neighbors.size() << std::endl;
// 25 as well
std::cout << nodes.at(15).neighbors.at(0)->neighbors.size() << std::endl;

Let me know If I understood your issue correctly.

You could instead use std::vector<std::shared_ptr<Node>>, and then just push

nodes.at(15).neighbors.push_back(nodes.at(0));

The other solution using a vector of shared ptr would be:

std::vector<std::shared_ptr<Node>> nodes;
nodes.reserve(50);

for (int j = 0; j < 50; ++j)
    nodes.push_back(std::make_shared<Node>(j));

for (int j = 1; j < 25; ++j)
    nodes.at(0)->neighbors.push_back(nodes.at(j));

nodes.at(15)->neighbors.push_back(nodes.at(0));

// 24 as expected
std::cout << nodes.at(0)->neighbors.size() << std::endl;
// 24 as well
std::cout << nodes.at(15)->neighbors.at(0)->neighbors.size() << std::endl;

//Insert another element
nodes.at(0)->neighbors.push_back(nodes.at(30));

// 25 as expected
std::cout << nodes.at(0)->neighbors.size() << std::endl;
// 25 as well
std::cout << nodes.at(15)->neighbors.at(0)->neighbors.size() << std::endl;

Post Status

Asked in February 2016
Viewed 1,418 times
Voted 12
Answered 3 times

Search




Leave an answer