C++ polymorphic class pointer in vector

Question

Let's say I have the following code which, in short, has:

• a BaseClass with a pointer to a PointerClass
• a ChildClass that inherits from BaseClass
• a HolderClass that has an std::vector of ChildClasss 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 PointerClasses 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

Answer 1

    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());
}

Answer 2

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:

1. 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.

2. 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.

Answer 3

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.

Answer 4

When addParentClass returns, the childClass object is destroyed. Therefore, the pointer to that object, that you put into basePointerVec cannot be used anymore.

Answer 5

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.