Reputation: 3
Let's say I have the following code which, in short, has:
BaseClass
with a pointer to a PointerClass
ChildClass
that inherits from BaseClass
HolderClass
that has an std::vector
of ChildClass
s and an std::vector
of BaseClass*
s:The entire code is as follows:
#include <stdlib.h>
#include <time.h>
#include <vector>
class PointerClass {
public:
int num;
double num2;
PointerClass() {
srand(time(NULL));
num = rand() % 100;
num2 = rand() % 100 / 2.0;
}
};
class BaseClass {
public:
PointerClass *pointerClass;
};
class ChildClass: public BaseClass {
public:
ChildClass() {
pointerClass = new PointerClass();
}
};
class HolderClass {
public:
std::vector<BaseClass*> basePointerVec;
std::vector<ChildClass> childVec;
HolderClass() {
}
void addParentClass() {
ChildClass childClass = ChildClass();
childVec.push_back(childClass);
basePointerVec.push_back(&childClass);
}
};
int main(int argc, const char * argv[]) {
HolderClass holderClass = HolderClass();
for (int count = 0; count < 20; count++) {
holderClass.addParentClass();
}
for (int count = 0; count < holderClass.basePointerVec.size(); count++) {
delete holderClass.basePointerVec[count]->pointerClass;
}
return 0;
}
My problem is, after adding a pointer to the ChildClass
into the std::vector<BaseClass*> basePointerVec
and the actual ChildClass
into the std::vector<ChildClass> childVec
in the addParentClass()
method in HolderClass
, the data in basePointerVec
andchildVec
are completely different.
Furthermore, when I try to free the PointerClass
es from the childVec
, everything works fine. But when I try to free them from the basePointerVec
instead, I get an error telling my I'm trying to free a pointer I have not allocated memory for.
And sure enough, when I use breakpoints to inspect everything, I find some funky behavior. It seems that every time I call ChildClass childClass = ChildClass();
in addParentClass()
, every pointer in basePointerVec
gets changed to point to the newly created ChildClass
's BaseClass
.
The point of me doing this in my actual program is to take advantage of polymorphism and have multiple classes inherit from BaseClass
.
So my question is, why is every pointer in the vector being changed to point to the newly created class and how can I fix it?
P.S. sorry for the length of this question, it's as short as I could possibly make it
Upvotes: 0
Views: 1649
Reputation: 3472
You are creating a local variable that is destroyed as soon addParentClass
goes out of scope. Also see the answers to this question which explains what happens when you don't use new
. So the vector of BaseClass
pointers are pointing to an object that is destroyed, and the childVec
vector of objects are creating new copies when you use push_back
, from this page:
The new element is initialized as a copy of value.
This is why the two vectors are pointing to different objects. You could create a destructor for the classes and add debugging prints to the destructor and the constructor to see when the objects are created/destroyed, this should give you a better idea of what is happening in what order.
Upvotes: 0
Reputation: 838
childVec.push_back(childClass);
The push_back method of the class vector copies the object. So the added object is not the same as childClass in this case.
You can't delete the pointers from basePointerVec because they were not allocated with new but they were allocated locally and they are deleted at the end of addParentClass. So, the code of addParent is wrong because the pointer you push in the vector is no longer valid after the end of the method and may cause segmentation faults(in best case). Here is a suggestion of improvement:
void addParentClass() {
ChildClass* childClass = new ChildClass();
childVec.push_back(*childClass);
basePointerVec.push_back(childClass);
}
The memory is now allocated dynamically, you should make sure to free these pointers with delete.
EDIT:
void addParentClass() {
childVec.push_back(ChildClass());
basePointerVec.push_back(&childVec.back());
}
And if you are using C++11:
void addParentClass() {
childVec.emplace_back();
basePointerVec.push_back(&childVec.back());
}
Upvotes: 1
Reputation: 76297
The other answers already addressed the specific problem in your code. Here are two general points about your program above that will help you IMHO avoid all sorts of problems in the future:
When you define a class, esp. one which can be subclassed, always make the dtor public virtual
except if you have a specific reason not too.
Always store resources in classes with the RAII principle:
Except for a good reason, you should never have a naked member pointer. Use a shared_ptr
member instead.
Except for a good reason, you should never have a vector
of naked pointers. Again use a vector of shared pointers.
Using this, hopefully, you won't need to track in your head what got destroyed when before something was copied to whatever, etc. It will make it harder to mix up heap objects and stack objects.
Upvotes: 1
Reputation: 31647
When addParentClass
returns, the childClass
object is destroyed. Therefore, the pointer to that object, that you put into basePointerVec
cannot be used anymore.
Upvotes: 0
Reputation: 16318
Look here:
void addParentClass() {
ChildClass childClass = ChildClass();
childVec.push_back(childClass);
basePointerVec.push_back(&childClass);
}
Your objects are not allocated on the heap. They are values and in basePointerVec
you put their addresses (which don't make sense anyway after addParentClass()
returns). Don't try to delete those, that will crash your program. They will be automatically deleted when childVec
goes out of scope.
Upvotes: 0