Should we delete before or after erase for an pointer in the vector

Question

Hi ,

Should we delete before or after erase. My understanding is both are OK. Is it correct?

In addition, is there any case we won't want to delete the element while erasing it? I believe there must be , otherwise, the erase will be happy to take the responsibility.

std::vector<foo*> bar;
...
for (vector<foo*>::iterator itr = bar.begin(); itr != bar.end(); itr++)
{
   delete (*itr);  //before OR
   bar.erase(itr);
   delete (*itr);  //after???
}

Answer 1

Using an iterator to erase an element invalidates the iterator. You should delete the item before it is erased.

You should also use the return value from erase for your next loop iteration.

Answer 2

Doing an erase will invalidate the vector iterator. So *iter will invoke undefined behavior. Hence you need to do the delete after the erase. Also, you can not erase the elements from a vector while iterating through it (because of the same reason, iter becomes invalid so iter++ is invalid). In this case you can remove the erase call from inside the loop and do clear of the vector outside the loop.

Answer 3

"itr" must be used like this;

for (vector<foo*>::iterator itr = bar.begin(); itr != bar.end(); )
{
   delete (*itr);
   itr = bar.erase(itr);
}

However, I'd prefer to first delete all elements, and then clear the vector;

for (vector<foo*>::iterator itr = bar.begin(); itr != bar.end(); ++itr)
   delete (*itr);
bar.clear();

Answer 4

The nature of vector that erasing first element causes entire array to shift forward, to reduce this operation try following:

std::vector<foo*> v1;
//...
while(!v1.empty())
{
    delete v1.back();
    v1.pop_back( );
}

By the way - this method doesn't invalidate any iterators (only on erased items)

Answer 5

In addition, is there any case we won't want to delete the element while erasing it?

How could the vector possibly know if anyone else needs the objects pointed to? How could it even know that the pointees are stored on the heap? It is perfectly possible to have pointers to static or automatic objects in the vector, or even dangling pointers.

C++0x allows you to express that the vector should own the pointees:

std::vector<std::unique_ptr<foo>> vec;

Now you don't have to manually delete anything. By erasing the unique pointers, their respective pointees are deleted too. Containers of native pointers are very rare in modern C++.

If you don't have a C++0x compiler, you can use std::vector<boost::shared_ptr<foo> > or boost::ptr_vector<foo> instead. Modern compilers supply shared_ptr in the std::tr1 or stdnamespace as well if you #include <memory>.