c++ vector of class object pointers

Question

Hi all,

What I am trying to do is essentially create two vectors of objects, where some objects are entered into both lists and some are just entered into one. The first problem I found was that when I used push_back() to add an object into both lists the object was copied so that when I changed it from one list the object did not change in the other. To get around this I tried to create a list of pointers to objects as one of the lists. However when I accessed the pointer later on the data seemed to be corrupted, the data member values were all wrong. Here are some snippets of my code:

Definition of vectors:

vector<AbsorbMesh> meshList;
vector<AbsorbMesh*> absorbList;

... Adding an object to both:

AbsorbMesh nurbsMesh = nurbs.CreateMesh(uStride, vStride);

// Add to the absorption list
absorbList.push_back(&nurbsMesh);
// Store the mesh in the scene list
meshList.push_back(nurbsMesh);

Accessing the object:

if (absorbList.size() > 0)
{
float receivedPower = absorbList[0]->receivedPower;
}

What am I doing wrong?

Answer 1

You are storing the address of an object allocated on stack. The nurbsMesh object gets destroyed as soon as your method which does push_back() ends. If you try to access this pointer afterwards the object is already destroyed and contains garbage. What you need is to retain the object which remains even after the function goes out of scope. To do this allocate the memory for the object from heap using new. But for every new you should have a corresponding delete. But in your case you'll have problems deleting it as you are pushing the same pointer into two vectors. To solve this, you would require some type reference counting mechanism.

Answer 2

There's some details missing, but at a guess.

nurbsMesh goes out of scope between the push_back and the absorbList[0]->receivedPower.

So now your vector of pointers contains a pointer to an object that doesn't exist anymore.

Try adding a copy constructor to your AbsorbMesh class and adding to your vector like this.

absorbList.push_back(new AbsorbMesh(nurbsMesh));
meshList.push_back(nurbsMesh);

don't forget to delete the objects in absorbList, like this

for(vector<AbsorbMesh*>::iterator it = absorbList.begin(); it != absorbList.end(); it++) {
    delete it;
  }

Or store a shared pointer in your vector instead of a bare pointer. Boost has a good shared pointer implementation if you're interested. See the docs here

If you want to have updates to items in one vector modify objects in the other vector, then you'll need to store pointers in both vectors.

Using your original requirements (updating an item in one vector affects items in the other vector, here's how I'd do it with a boost shared pointer. (WARNING, untested code)

vector<boost::shared_ptr<AbsorbMesh> > meshList;
vector<boost::shared_ptr<AbsorbMesh> > absorbList;

boost::shared_ptr<AbsorbMesh> nurb = new AbsorbMesh(nurbs.CreateMesh(uStride, vStride));

meshList.push_back(nurb);
absorbList.push_back(nurb);

...
...

if (absorbList.size() > 0)
{
    float receivedPower = absorbList[0].get()->receivedPower;
}

Answer 3

absorbList.push_back(&nurbsMesh); is wrong

absorbList save pointer to local object. When nurbMesh is destroyed you can not write absorbList[0]->

Answer 4

Object is deleted when you try get pointer from vector.

Try do

vector.push_back(new Object);

Answer 5

Once you have fixed the problem that others mentioned (storing a pointer to an object that's on the stack), you're going to run into another issue. A vector may reallocate, which results in its contents moving to another location.

The safe way to do this, then, is to store pointers in both vectors. Then, of course, you need to ensure that they get deleted... but that's C++ for you.

Answer 6

However when I accessed the pointer later on the data seemed to be corrupted

When you put something on a vector, the vector might move it from one physical location to another (especially e.g. when the vector is resized), which invalidates any pointer to that object.

To fix that, you'll need to store a pointer (possibly a 'smart pointer') in both vectors (instead of having one vector contain the object by value).

If you're going to do this, it might be a good idea to disable the object's copy constructor and assignment operator (by declaring them as private, and not defining them) to ensure that after an object is created it cannot be moved.

Answer 7

There are several things wrong with your example

AbsorbMesh nurbsMesh = nurbs.CreateMesh(uStride, vStride);

This object is allocated on the stack. It is a purely local object. This object is going to be destroyed when you reach the end of the current block surrounded by {}.

absorbList.push_back(&nurbsMesh);

Now you get the pointer to the object that most likely will be destroyed.

meshList.push_back(nurbsMesh)

And this copies an entirely new object on the vector.

It is equally wrong to push the object on the vector first and then push a pointer to the object on the vector using absorbList.push_back( &meshList.back() ) because vector::push_back will reallocate the whole vector, invalidating all pointers.

You might be able to create all AbsorbMesh objects first, push them onto a vector, and then get the pointers to these objects in the vector. As long as you don't touch the vector, you'll be fine.

Alternatively, create objects on the heap using new AbsorbMesh() but make sure to call delete on each pointer thus created. Otherwise you have a memory leak.

Third solution, avoid the trouble and use smart pointers that take care of object destruction for you.

Answer 8

First, as everybody else points out, you can't allocate objects on the stack (i.e., other than by new or something similar), and have them around after leaving the scope.

Second, having objects in an STL container and maintaining pointers to them is tricky, since containers can move things around. It's usually a bad idea.

Third, auto_ptr<> simply doesn't work in STL containers, since auto_ptrs can't be copied.

Pointers to independently allocated objects work, but deleting them at the right time is tricky.

What will probably work best is shared_ptr<>. Make each vector a vector<shared_ptr<AbsorbMesh> >, allocate through new, and at a slight cost in performance you avoid a whole lot of hassle.