C++ How to Save Elements in a Vector from a Function

Question

Consider the following code. In list1 I'm adding elements saved on the heap to a vector and printing. It works as expected. In list2 I'm adding elements onto the vector from a function which also allocates elements on the heap.

I understand I have to allocate Node on the heap in addNode as otherwise it would be deallocated when the function returns. However, via the final print statement I can see the nodes on the heap are still allocated, yet they don't show up in my vector.

#include <iostream>
#include <vector>

using namespace std;


/*
Simple node class for demo
 */
class Node
{
public:
  string val;
  Node(string value) { this->val = value; }
};

Node *addNode(vector<Node *> list)
{
  // allocate space for node on the heap so it isn't destroyed after function returns
  auto node = new Node("foo");
  // add pointer to node onto vector
  list.push_back(node);
  return node;
}

/*
Simple function for printing vector contents
 */
template <typename T>
void printVector(T d)
{
  cout << "Vector has size " << d.size() << " and elements: ";
  for (auto p = d.begin(); p < d.end(); p++)
  {
    cout << (*p)->val << ",";
  }
  cout << "\n";
}

int main()
{
  // make a new vector
  vector<Node *> list1;
  // add elements allocated on the heap
  list1.push_back(new Node("foo"));
  list1.push_back(new Node("foo"));
  list1.push_back(new Node("foo"));
  printVector(list1); // prints: "Vector has size 3 and elements: foo,foo,foo,"

  // make a new vector
  vector<Node *> list2;
  // add elements allocated on the heap from a function
  addNode(list2);
  addNode(list2);
  // save one of the nodes to a variable for demonstration
  auto node = addNode(list2);
  printVector(list2); // prints: "Vector has size 0 and elements:"
  cout << node->val << "\n"; // prints: "foo"

  return 0;
}

Can someone explain how to add elements to a vector from a function?

Answer 1

In addNode(), you are passing the vector by value, thus a copy of the caller's vector is made, and then you are adding the Node* to the copy, the original vector is unaffected.

You need to pass the vector by reference instead:

Node* addNode(vector<Node*> &list)

Same with printVector(). You are passing the vector by value, you are just not modifying the copy, but you should still pass the vector by (const) reference to avoid making a copy at all:

template <typename T>
void printVector(const T &d)

---

On a side note, you are leaking the Nodes you create. You need to delete them when you are done using them:

int main()
{
  vector<Node*> list1;
  list1.push_back(new Node("foo"));
  list1.push_back(new Node("foo"));
  list1.push_back(new Node("foo"));
  printVector(list1);

  for(auto *p : list1)
    delete p;

  vector<Node*> list2;
  addNode(list2);
  addNode(list2);
  auto node = addNode(list2);
  printVector(list2);
  cout << node->val << "\n";

  for(auto *p : list2)
    delete p;

  return 0;
}

Better to use std::unique_ptr to manage that for you:

#include <iostream>
#include <vector>
#include <memory>

using namespace std;

...

Node* addNode(vector<unique_ptr<Node>> &list)
{
  list.push_back(make_unique<Node>("foo"));
  return list.back().get();
}

template <typename T>
void printVector(const T &d)
{
  cout << "Vector has size " << d.size() << " and elements: ";
  for (const auto &p : d)
  {
    cout << p->val << ",";
  }
  cout << "\n";
}

int main()
{
  vector<unique_ptr<Node>> list1;
  list1.push_back(make_unique<Node>("foo"));
  list1.push_back(make_unique<Node>("foo"));
  list1.push_back(make_unique<Node>("foo"));
  printVector(list1);

  vector<unique_ptr<Node>> list2;
  addNode(list2);
  addNode(list2);
  auto node = addNode(list2);
  printVector(list2);
  cout << node->val << "\n";

  return 0;
}