CODEWITHSUNDEEP
c++containersstd
Roman Kazmin
Roman Kazmin

Reputation: 981

Safety deleting elements of the container

Could you suggest safety deleting of element of std::vector in to cases: 1. Clear all elements of the vector; 2. Erase one element depending on condition.

What are the dangers of this code:

    typename std::vector<T*>::iterator it;
    for (it=std::vector<T*>::begin();it!=std::vector<T*>::end();it++)
    {
        if (*it) delete *it;
    }

Thank's.

Upvotes: 0

Views: 130

Answers (2)

doctorlove
doctorlove

Reputation: 19232

For example, for you first two questions which appeared to regard clearing a vector

std::vector<int> v;//maybe have elements or not...

  1. Clear the vector by calling clear

    v.clear();

  2. If you wish to remove elements while you walk over the vector which satisfy a condition (i.e. a predicate) using v.erase(it) you need to be careful. Fortunately, erase returns the iterator after the removed position, so something like

    for (auto it=v.begin();it!=v.end();)
{
     if (predicate(it))
         it = v.erase(it);
     else
         ++it;
}

    Obviously you can use std::remove_if instead (with std::erase) if you want to shrink your vector slightly, avoiding a "awfully performant hand-written remove_if" as a commenter described this.

If you want to delete things as you loop round, you can either hand-do a loop yourself or use an algorithm - but smart pointers make far more sense. I suspect you are wanting to delete the items possibly in addition to shrinking the vector since you said " I found that element of this vector can be used after its deleting". If you do want to store raw pointers in a container, just delete the items, you don't the the if in your suggested code. You need to consider when you plan on doing this.
Note: changing the contents of an iterator doesn't invalidate an iterator.

You should consider using smart pointers, for exception safety etc.

Upvotes: 1

4386427
4386427

Reputation: 44264

You don't remove the element from the vector. So the vector element is pointing to the location as before, i.e. the same T. However, since you have deleted that T you can not dereference the pointer anymore - that would be UB and may crash your program.

delete is calling the destructor of T (great, it's something you shall do) but deleteis not changing the vector. Consequently the iterator is valid all the time.

Either you should remove the element for which you have called delete or at least set the vector element to nullptr.

typename std::vector<T*>::iterator it;
for (it=std::vector<T*>::begin();it!=std::vector<T*>::end();it++)
{
    delete *it;
    *it = nullptr;  // Only needed when you don't erase the vector element
}

That solution requires that you always check for nullptr before using any element of the vector.

In most cases the best solution is however to remove the element from the vector.

In the case where you destroy all elements by calling delete on every element, simply call clear on the vector after the loop.

Upvotes: 2

Related Questions