Search code examples
c++loopsdata-structuresiteratorstdvector

How do you remove elements from a std::vector while iterating?


I have a std::vector<std::string> m_vPaths; I iterate over this vector and call ::DeleteFile(strPath) as I go. If I successfully delete the file, I remove it from the vector.

My question is: can I get around having to use two vectors? Is there a different data structure that might be better suited for what I need to do?

Example

Using iterators almost does what I want, but the problem is that once you erase using an iterator, all iterators become invalid.

std::vector<std::string> iter = m_vPaths.begin();
for ( ; iter != m_vPaths.end(); iter++) {
    std::string strPath = *iter;
    if (::DeleteFile(strPath.c_str())) {
        m_vPaths.erase(iter);   
        // Now my interators are invalid because I've used erase,
        // but I want to continue deleting the files remaining in my vector.    
    }
}

I can use two vectors and I will no longer have a problem, but is there a better, more efficient method of doing what I'm trying to do?

In case it is unclear, m_vPaths is declared like this (in my class):

std::vector<std::string> m_vPaths;

Solution

  • Check out std::remove_if:

    #include <algorithm> // for remove_if
    #include <functional> // for unary_function
    
    struct delete_file : public std::unary_function<const std::string&, bool> 
    {
        bool operator()(const std::string& strPath) const
        {
            return ::DeleteFile(strPath.c_str());
        }
    }
    
    m_vPaths.erase(std::remove_if(m_vPaths.begin(), m_vPaths.end(), delete_file()),
                    m_vPaths.end());
    

    Use a std::list to stop the invalid iterators problem, though you lose random access. (And cache performance, in general)


    For the record, the way you would implement your code would be:

    typedef std::vector<std::string> string_vector;
    typedef std::vector<std::string>::iterator string_vector_iterator;
    
    string_vector_iterator iter = m_vPaths.begin();
    while (iter != m_vPaths.end())
    {
        if(::DeleteFile(iter->c_str()))
        {
            // erase returns the new iterator
            iter = m_vPaths.erase(iter);
        }
        else
        {
            ++iter;
        }
    }
    

    But you should use std::remove_if (reinventing the wheel is bad).