What is the right way to free a std::vector of pointers in C++?

Question

I searched StackOverflow but couldn't find the answer to this question.

Suppose I have a std::vector<Day *> vector_day - that is - a vector of pointers to Day object. Now I push_back to vector_day many elements:

vector_day.push_back(new Day(12));
vector_day.push_back(new Day(99));
vector_day.push_back(new Day(71));
...

Now at some point I no longer need vector_day. What is the right way to free the memory?

It this the correct way:

for (std::vector<Day *>::iterator i = vector_day.begin(); i != vector_day.end(); ++i) {
    delete *i;
}

Doesn't this invalidate the vector on each deletion? I am very confused.

Thanks, Boda Cydo.

Answer 1

The best way is not to put pointers into the vector in the first place if you don't absolutely need to.

But if you do really need to have a vector of pointers, then the way you are doing it is just fine (but .clear() the vector afterwords, if it won't be immediately destroyed, so that it's not full of dangling pointers)

The statement

delete *it;

has no effect on the iterator. It does not change the iterator, invalidate the iterator, or remove the pointer referred to by the iterator from the collection. All it does is free the memory that the pointer referred to by the iterator points at. The pointer itself must be removed from the collection separately.

Answer 2

Operations that add or remove elements from the array may invalidate the iterators, check the documentation for specific rules for the different container types. With delete, you're acting on the data contained in the array elements, not on the array's shape. Iterators traverse the container's shape, they don't care about its contents.

Answer 3

First of all, you switched from i to it, but I assume that's just a typo.

But to answer your quest, no, that's fine. You are not changing it, you are changing *it.

Answer 4

You should probably be using some kind of managed pointer, most likely a shared pointer.

If you delete the vector while someone else is still holding on to one of those pointers, you're going to get some very nasty behaviour if they try to dereference it. A shared pointer will save you that headache.

If you can guarantee that nothing else will reference the pointers after the vector is deleted, then you can still benefit from using an auto pointer. It will manage deallocation for you when the vector is destroyed. The overhead is minimal, and it makes your life a lot easier.

Answer 5

It's just fine. You are deleting *i (object pointed by vector's element) and not i (vector's element), so the vector is not invalidated.

See this question for a case where the developer also wanted to delete all is, and for the solution for it (vector_day.clear()) after the loop.

Answer 6

Another C++ way to do this is to define a helper struct:

struct delete_ptr { // Helper function to ease cleanup of container
    template <typename P>
    void operator () (P p) {
        delete p;
    }
};

and then use the algorithms:

std::for_each(vector_day.begin(), vector_day.end(), delete_ptr());
vector_day.clear();

Answer 7

In general in C++ you should hide memory management as much as possible to avoid memory errors. Unless you're doing a lot of copying of the pointers and care a lot about performance I would just use a shared_ptr.

It's part of the TR1 standard and is available in most modern C++ compilers out of the box (http://anteru.net/2008/09/01/260/) and is great for fire and forget memory management.

Answer 8

Boost ptr_vector to the rescue!

Does exactly what you need, without the need to iterate and delete the contents of the std::vector

Answer 9

Here's a handy class I wrote a while ago while dealing with the same issue. I was converting some code from old RogueWave-based vectors and lists to STL-based vectors and lists, and needed some way to emulate RW's clearAndDestroy() method for pointer lists. The clearAndDestroy() method can be overridden to handle different structure types (I only included vector here for brevity).

class StlUtils
{
   public:

      /**
       * This method provides a templated way to destroy a std::vector
       * full of pointers.  It is basically a replacement for the RW
       * vector class' clearAndDestroy methods.  The list argument is
       * returned empty.
       *
       * @param list the list of pointers to be destroyed.
       */
      template<class T> static void clearAndDestroy(
         std::vector<T*> &itemList)
      {
         for_each(itemList.begin(), itemList.end(),
                  stl_deleter<T>());
         itemList.clear();
      }

   private:

      /**
       * Templated member function for use with the clearAndDestroy()
       * method.  It provides the method needed by for_each to do the
       * actual deletion.
       */
      template<class T> struct stl_deleter
      {
         void operator() (T* x) {
            if (x != NULL)
               delete x;
         }
      };
};