Why is this vector iterator not incrementable?

Question

i'm trying to delete the vector's content and i'm getting an error - vector iterator is not incrementable, why is that?

this is my destructor

City::~City()
{
    vector <Base*>::iterator deleteIterator;
    for (deleteIterator = m_basesVector.begin() ; deleteIterator != m_basesVector.end() ; deleteIterator++)
        m_basesVector.erase(deleteIterator);
}  

thanks.

Answer 1

erase invalidates the iterator. You can't use it any more. Luckily for you, it returns an iterator that you can use:

vector <Base*>::iterator deleteIterator = m_basesVector.begin();
while (deleteIterator != m_basesVector.end()) {
    deleteIterator = m_basesVector.erase(deleteIterator);
}

Or:

m_basesVector.clear();

Are you responsible for freeing the memory referred to by the pointers in the vector? If that's the reason that you're iterating (and your real program has more code that you haven't shown, that frees those objects in the loop), then bear in mind that erasing from the beginning of a vector is a slow operation, because at each step, all the elements of the vector have to be shifted down one place. Better would be to loop over the vector freeing everything (then clear() the vector, although as Mike says that's not necessary if the vector is a member of an object that's being destroyed).

Answer 2

The problem is that you are trying to use an iterator while using the erase() function. erase(), push_back(), insert(), and other modifying functions invalidate iterators in STL.

Just use the clear() function:

City::~City()
{
    m_basesVector.clear();
}  

Answer 3

Vector iterators are incrementable, but if you delete elements, the vector contents are modified and thus the iterator is invalid.

So, if you delete objects, you should use the return value of erase() that gives you the next valid iterator.

Answer 4

Any iterator pointing to the deleted element or to the elements after the one that is deleted gets invalidated when the vector's erase method is called. Erase method returns a valid iterator pointing to the next element in the vector. You should use that iterator to continue your looping & not increment the invalidated iterator. You may also use the clear method to remove all the elements in the vector. However, you will need to remember to explicitly de-allocate any allocated memory for the elements.

Answer 5

If you are trying to free the data in the vector, do this:

for (std::vector<Base*>::iterator it = v.begin(), e = b.end(); it != e; ++it) 
    delete *it;

Answer 6

This code leaks all the contents of the vector - you have to delete *deleteIterator in the loop too. You can avoid all of this by using Base instead of Base* as the vector contents, then clear() will destruct them for you. Or use boost::ptr_vector which automates destruction if you do need raw pointers.

Calling erase() in a forward iteration like this can be very costly if the vector is large, as every element above the current position has to be moved down to ensure elements remain contiguous. Avoid manual erase of the type you propose, for this and other reasons.