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.

Wordplay

May be a dupe of #3055067 – Rheinlander

RC - not really, this is completely different. – Wordplay

Don't put pointers in like that. What if an exception is thrown between the time stuff is in the vector and the place where you delete it all? You'd skip over it and leak. Use a smart pointer, or a pointer container, never raw pointers. – Clayclaybank

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.

Majka

Thanks for explaining. So when the vector goes out of scope, the pointer will be removed automatically, is that right? – Wordplay

@bodacydo: Right. If you make the delete *it calls right before the vector goes out of scope, don't worry about calling clear(). – Jurisdiction

Answer 2

Boost ptr_vector to the rescue!

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

Tude

I need to pass a vector<T*> to a function. Can ptr_vector<T> be cast to vector<T*>? – Pentose

Answer 3

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

Hurrah

Answer 4

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.

Katiekatina

Answer 5

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.

Cartage

Answer 6

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.

Eddington

I now understand invalidation better. Thanks for your answer! – Wordplay

Answer 7

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.

Seaway

I corrected the mistake of it being used for i. Thanks for spotting it. What if I did i = 0xAABBCCDD in the loop - that clearly changes i and not it, would that invalidate the vector? – Wordplay

Answer 8

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.

Kingsly

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

Rarefaction

You are allowed to delete null pointers (which will do nothing), so the if isn't really needed. – Cruzeiro

Answer 10

Another way of iterating + deleting is by using while(!empty) loop. The advantage of this technique is that an element is removed from container first, deleted afterwords. This is safe for any container:

while (!vector_day.empty()) {
    Day* day = vector_day.back();
    vector_day.pop_back();
    delete day;
}

Burney