Can you remove elements from a std::list while iterating through it?

Viewed 273596

I've got code that looks like this:

for (std::list<item*>::iterator i=items.begin();i!=items.end();i++)
{
    bool isActive = (*i)->update();
    //if (!isActive) 
    //  items.remove(*i); 
    //else
       other_code_involving(*i);
}
items.remove_if(CheckItemNotActive);

I'd like remove inactive items immediately after update them, inorder to avoid walking the list again. But if I add the commented-out lines, I get an error when I get to i++: "List iterator not incrementable". I tried some alternates which didn't increment in the for statement, but I couldn't get anything to work.

What's the best way to remove items as you are walking a std::list?

14 Answers

You have to increment the iterator first (with i++) and then remove the previous element (e.g., by using the returned value from i++). You can change the code to a while loop like so:

std::list<item*>::iterator i = items.begin();
while (i != items.end())
{
    bool isActive = (*i)->update();
    if (!isActive)
    {
        items.erase(i++);  // alternatively, i = items.erase(i);
    }
    else
    {
        other_code_involving(*i);
        ++i;
    }
}

You want to do:

i= items.erase(i);

That will correctly update the iterator to point to the location after the iterator you removed.

You need to do the combination of Kristo's answer and MSN's:

// Note: Using the pre-increment operator is preferred for iterators because
//       there can be a performance gain.
//
// Note: As long as you are iterating from beginning to end, without inserting
//       along the way you can safely save end once; otherwise get it at the
//       top of each loop.

std::list< item * >::iterator iter = items.begin();
std::list< item * >::iterator end  = items.end();

while (iter != end)
{
    item * pItem = *iter;

    if (pItem->update() == true)
    {
        other_code_involving(pItem);
        ++iter;
    }
    else
    {
        // BTW, who is deleting pItem, a.k.a. (*iter)?
        iter = items.erase(iter);
    }
}

Of course, the most efficient and SuperCool® STL savy thing would be something like this:

// This implementation of update executes other_code_involving(Item *) if
// this instance needs updating.
//
// This method returns true if this still needs future updates.
//
bool Item::update(void)
{
    if (m_needsUpdates == true)
    {
        m_needsUpdates = other_code_involving(this);
    }

    return (m_needsUpdates);
}

// This call does everything the previous loop did!!! (Including the fact
// that it isn't deleting the items that are erased!)
items.remove_if(std::not1(std::mem_fun(&Item::update)));

Use std::remove_if algorithm.

Edit:
Work with collections should be like:

  1. prepare collection.
  2. process collection.

Life will be easier if you won't mix this steps.

  1. std::remove_if. or list::remove_if ( if you know that you work with list and not with the TCollection )
  2. std::for_each

I have sumup it, here is the three method with example:

1. using while loop

list<int> lst{4, 1, 2, 3, 5};

auto it = lst.begin();
while (it != lst.end()){
    if((*it % 2) == 1){
        it = lst.erase(it);// erase and go to next
    } else{
        ++it;  // go to next
    }
}

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2

2. using remove_if member funtion in list:

list<int> lst{4, 1, 2, 3, 5};

lst.remove_if([](int a){return a % 2 == 1;});

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2

3. using std::remove_if funtion combining with erase member function:

list<int> lst{4, 1, 2, 3, 5};

lst.erase(std::remove_if(lst.begin(), lst.end(), [](int a){
    return a % 2 == 1;
}), lst.end());

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2

4. using for loop , should note update the iterator:

list<int> lst{4, 1, 2, 3, 5};

for(auto it = lst.begin(); it != lst.end();++it){
    if ((*it % 2) == 1){
        it = lst.erase(it);  erase and go to next(erase will return the next iterator)
        --it;  // as it will be add again in for, so we go back one step
    }
}

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2 

Removal invalidates only the iterators that point to the elements that are removed.

So in this case after removing *i , i is invalidated and you cannot do increment on it.

What you can do is first save the iterator of element that is to be removed , then increment the iterator and then remove the saved one.

Iterating backwards avoids the effect of erasing an element on the remaining elements to be traversed:

typedef list<item*> list_t;
for ( list_t::iterator it = items.end() ; it != items.begin() ; ) {
    --it;
    bool remove = <determine whether to remove>
    if ( remove ) {
        items.erase( it );
    }
}

PS: see this, e.g., regarding backward iteration.

PS2: I did not thoroughly tested if it handles well erasing elements at the ends.

do while loop, it's flexable and fast and easy to read and write.

auto textRegion = m_pdfTextRegions.begin();
    while(textRegion != m_pdfTextRegions.end())
    {
        if ((*textRegion)->glyphs.empty())
        {
            m_pdfTextRegions.erase(textRegion);
            textRegion = m_pdfTextRegions.begin();
        }
        else
            textRegion++;
    } 

I'd like to share my method. This method also allows the insertion of the element to the back of the list during iteration

#include <iostream>
#include <list>

int main(int argc, char **argv) {
  std::list<int> d;
  for (int i = 0; i < 12; ++i) {
    d.push_back(i);
  }

  auto it = d.begin();
  int nelem = d.size(); // number of current elements
  for (int ielem = 0; ielem < nelem; ++ielem) {
    auto &i = *it;
    if (i % 2 == 0) {
      it = d.erase(it);
    } else {
      if (i % 3 == 0) {
        d.push_back(3*i);
      }
      ++it;
    }
  }

  for (auto i : d) {
      std::cout << i << ", ";
  }
  std::cout << std::endl;
  // result should be: 1, 3, 5, 7, 9, 11, 9, 27,
  return 0;
}
Related