CODEWITHSUNDEEP
c++pointersvectorreference
user3794186
user3794186

Reputation: 709

C++ Vector Push_back producing Weird Side Effect

While I was using STL vector to store class objects,, I observed a very weird side effect, where push_back method modifies the Existing Data!

Basically I have a class containing several fields as follows:

class MyClass {
    MyClass(std::string s, int i) {
        StringValue = s;
        IntValue = i;
    }

    std::string StringValue;
    int IntValue;
}

I have a vector that contains the POINTERS to MyClass objects.. and then I am basically pushing back references to objects:

std::vector<MyClass*> MyVector;
MyClass c1("CLASS 1", 1);
MyClass c2("CLASS 2", 2);

MyVector.push_back(&c1);
// Result:
// *MyVector[0]    ==>    ("Class 1", 1)



MyVector.push_back(&c2);
// Result:
// *MyVector[0]    ==>    ("Class 2", 2)   ??why 2??
// *MyVector[1]    ==>    ("Class 2", 2)

Do you see the strange result that I got?? I've set breakpoints after each push_back statement,,, and this weird thing happened.

The first push_back statement worked fine. But then the second push_back statement MODIFIED the content of the first element, which doesn't make sense to me..

I'm assuming that it has something to do with me storing References instead of actual objects inside the vector.... but I can't figure out what is wrong.

How can I handle this issue? Any insights?

Thanks


UPDATE:

(simplified code)

MyClass c1("CLASS 1", 1);
MyClass c2("CLASS 2", 2);

MyClass temp;
while (int i=0; i<2; i++) {
    temp = c1;
    MyVector.push_back(temp);
}

You guys are right,, I get what I'm doing wrong here.. The actual object gets destructed in every loop.. What's the best way to fix this while keeping the current structure?? I'ts hard to explain but I would like to keep this structure (keeping temporary buffer outside the loop).. is this possible?

Upvotes: 0

Views: 1189

Answers (2)

Marine Galantin
Marine Galantin

Reputation: 2279

For anyone looking for a clear and concise explanation:

Why does an object get destroyed when pushing it to a vector?

You are creating a temporary object:

m_foos.push_back(Foo(a));
//               ^^^^^^

Solution: The function emplace_back is what you want to use:

m_foos.emplace_back(a);

Check out: push_back vs emplace_back

EDIT--- Also, I noted that when compiling with CLion, the error is not fixed. It is also known to be an error on Visual Studio. In other words, be careful with your IDE.

Upvotes: 0

RichieHindle
RichieHindle

Reputation: 281405

I'm going to out on a limb and use my psychic powers to derive the real code, rather than the simplified code in the question. Your real code looks like... drumroll...

class MyClass {
    // ...
};

void addInstance(std::vector<MyClass*>& MyVector, int i) {
    MyClass c("", i);
    MyVector.push_back(&c);
}

int main() {
    addInstance(MyVector, 1);
    addInstance(MyVector, 2);
    // ...
 }

Here's a working example to demonstrate the problem, which does indeed output "2, 2" (although that's not guaranteed because you're invoking undefined behaviour):

http://ideone.com/PxiUx9

Edit: My psychic powers said "auto variable in a function" when (now that we have the updated question) it was really "auto variable in a loop". Not too far wrong. :-)

You're storing the address of an automatic variable beyond its lifetime, which is not allowed. The address is being re-used on each call, hence each pointer stored is the same, and happens to point to the memory that was last used to store the most recently created instance of MyClass (though that's not guaranteed).

You need to (preferably) store copies of those automatic variables rather than pointers to them, or (less preferably) create them with new and later delete them with delete.

To store copies, you need to use a std::vector<MyClass>. Here's an example of how you might do that: http://ideone.com/4rIijM Note that once your class gets more complex you might need to define a copy constructor, destructor and assignment operator - look up the "rule of three". If you're using C++11, also look up the "rule of five".

Upvotes: 5

Related Questions