STL Containers & Memory Management - list of objects vs. list of pointers to objects

Question

I have had a good look at some other questions on this topic and none of them (to my knowledge) address how to correctly erase items from a stl list of objects which contain dynamicically assigned memory vs. a stl list of objects that don't contain dynamically assigned memory.

I want to use a list of objects. Take this object for example (which contains no dynamically assigned memory):

class MyPoint {

public:
    MyPoint(int _x,int _y)
    {
        x = _x;
        y = _y;
    }

private:
    int x;
    int y;

};

So I might create a list of objects (not pointers to them), add things to it and then erase an element:

list<MyPoint> myList;

myList.push_back(MyPoint(3,4));
myList.push_back(MyPoint(1,2));
myList.push_back(MyPoint(8,8));
myList.push_back(MyPoint(-1,2));

list<MyPoint>::iterator it;

it = myList.begin();
advance(it,2);
myList.erase(it);

My list now contains: (3, 4) (1, 2) (-1, 2)

• QUESTION 1a: do I need to do anything else to the erased object or will the memory be taken care of?

• QUESTION 1b: if the program ends, do I need to do something with the remaining objects in the list? Do I need to delete them all and deal with their memory somehow?

Ok, now consider an alternative version of the class that allowed a point in N-dimensional space. I.e., I could dynamically assign an array of length N to hold the N points inside the class (I have spared you the implementation as that is not in question here). The destructor of the class would then delete the dynamically assigned array using 'delete'.

class MyDynamicPoint {

public:
    MyDynamicPoint(int N)
    {
        points = new int[N];
    }

    ~MyDynamicPoint()
    {
        delete points;
        points = NULL;
    }

private:
    int *points;
};

I might now create a list of pointers to the objects, instead of the objects themselves:

list<MyDynamicPoint*> myList;

myList.push_back(new MyDynamicPoint(8));
myList.push_back(new MyDynamicPoint(10));
myList.push_back(new MyDynamicPoint(2));
myList.push_back(new MyDynamicPoint(50));

list<MyDynamicPoint*>::iterator it;

it = myList.begin();
advance(it,2);
myList.erase(it);

• QUESTION 2a - Is the above correct? I.e. Because this new version of the class would contain some dynamically assigned memory, does this mean I have to create a list of pointers to objects, not the objects themselves?

• QUESTION 2b - Given that I have just erased the pointer from the list, where do I call delete to deal with the fact there is now dynamic memory to be deleted in the objects? Or does the erase method of stl list call the destructor of the object, taking care of it?

Many thanks in advance for any help,

Best,

Adam

Answer 1

I think following should work

MyPoint* ptr = myList.back();

delete ptr;

myList.pop_back();

OR

MyPoint* ptr = myList.back();

delete ptr;

myList.erase(ptr);

Answer 2

QUESTION 1a: do I need to do anything else to the erased object or will the memory be taken care of?

You don't need to do anything.

QUESTION 1b: if the program ends, do I need to do something with the remaining objects in the list? Do I need to delete them all and deal with their memory somehow?

You don't need to do anything.

QUESTION 2a - Is the above correct?

The code is not correct. You're violating The Rule of Three. In particular, the automatically-generated MyDynamicPoint's copy constructor and assignment operator will make a bitwise copy of the points pointer. If you copy an instance of MyDynamicPoint, you'll end up with two object sharing the same points pointer:

• When one of the objects goes of scope, the other becomes unusable.
• When the second object goes out of scope, its destructor will attempt to free memory that's already been freed. This is undefined behaviour.

I.e. Because this new version of the class would contain some dynamically assigned memory, does this mean I have to create a list of pointers to objects, not the objects themselves?

No, it does not mean that. In fact, you should probably continue to store objects by value. However, you do need to fix the rule of three.

QUESTION 2b - Given that I have just erased the pointer from the list, where do I call delete to deal with the fact there is now dynamic memory to be deleted in the objects? Or does the erase method of stl list call the destructor of the object, taking care of it?

Since you have a list of raw pointers, the destructors will not be called automatically. The easiest way to fix that is to either store objects by value, or use std::unique_ptr or std::shared_ptr instead of raw pointers.

Answer 3

When you have a class with data members that have automatic storage duration (i.e. their lifetime is tied to the instance of this class) like this:

class MyPoint {
private:
    int x;
    int y;
};

and you will use list<MyPoint> myList;, then this instance of std::list is also an object with automatic storage duration, that will be cleaned up automatically and by the time the container is destructed, so are the elements it holds. Everything is taken care of.

But the latter version is not very lucky choice... not only that you have a container holding pointers, you even decided to create a data member of class Point that will be allocated dynamically. At first note that everything that has been allocated by calling new should be freed by calling delete and everything allocating by calling new[] should be freed by calling delete[].

In this situation, you are allocating the memory when the object is constructed and cleaning it up when the object is destructed:

MyDynamicPoint(int N)
{
    points = new int[N];
}
~MyDynamicPoint()
{
    delete[] points;
    points = NULL;
}
private:
int *points;

You would achieve the same by using some std::vector or std::array instead of the C-style array and you wouldn't have to take care of the memory management on your own:

MyDynamicPoint(int N) : points(std::vector<int>(N, 0)) { }

private:
std::vector<int> points;

the std::vector object will take care of memory management for you.

And last thing: when you dynamically allocate an element and store it into the container:

myList.push_back(new MyDynamicPoint(8));

you need to free this memory on your own, erasing the pointer from the list is not enough:

list<MyDynamicPoint*>::iterator it;
...
delete *it;
myList.erase(it);

So whatever you want to achieve, always prefer objects with automatic storage duration if the situation allows it. There's nothing worse than being forced to taking care of memory management manually and dealing with unpleasant problems such as memory leaks later.

Answer 4

To question 1, there is nothing you need to do. As you store the objects by value the compiler and the library will handle everything.

However, when you store pointer as in the second case, you need to delete those pointers that you have allocated with new, or you will have a memory leak.

And you have to delete the pointers before doing the erasing, as that can invalidate the iterator:

delete *it;
myList.erase(it);