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

0 votes
asked Feb 27, 2009 by ashelly

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?

11 Answers

0 votes
answered Jan 27, 2009 by msn

You want to do:

i= items.erase(i);

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

0 votes
answered Jan 27, 2009 by anand

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.

0 votes
answered Jan 27, 2009 by mykola-golubyev

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
0 votes
answered Jan 27, 2009 by mike

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 != items.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)));
0 votes
answered Feb 27, 2009 by michael-kristofik

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;
    }
}
0 votes
answered Jan 5, 2011 by rafael-gago

The alternative for loop version to Kristo's answer.

You lose some efficiency, you go backwards and then forward again when deleting but in exchange for the extra iterator increment you can have the iterator declared in the loop scope and the code looking a bit cleaner. What to choose depends on priorities of the moment.

The answer was totally out of time, I know...

typedef std::list<item*>::iterator item_iterator;

for(item_iterator i = items.begin(); i != items.end(); ++i)
{
    bool isActive = (*i)->update();

    if (!isActive)
    {
        items.erase(i--); 
    }
    else
    {
        other_code_involving(*i);
    }
}
0 votes
answered Jan 18, 2012 by marcin-skoczylas

I think you have a bug there, I code this way:

for (std::list<CAudioChannel *>::iterator itAudioChannel = audioChannels.begin();
             itAudioChannel != audioChannels.end(); )
{
    CAudioChannel *audioChannel = *itAudioChannel;
    std::list<CAudioChannel *>::iterator itCurrentAudioChannel = itAudioChannel;
    itAudioChannel++;

    if (audioChannel->destroyMe)
    {
        audioChannels.erase(itCurrentAudioChannel);
        delete audioChannel;
        continue;
    }
    audioChannel->Mix(outBuffer, numSamples);
}
0 votes
answered Jan 4, 2013 by david-cormack

Here's an example using a for loop that iterates the list and increments or revalidates the iterator in the event of an item being removed during traversal of the list.

for(auto i = items.begin(); i != items.end();)
{
    if(bool isActive = (*i)->update())
    {
        other_code_involving(*i);
        ++i;

    }
    else
    {
        i = items.erase(i);

    }

}

items.remove_if(CheckItemNotActive);
0 votes
answered Jan 3, 2015 by alex-bagg

If you think of the std::list like a queue, then you can dequeue and enqueue all the items that you want to keep, but only dequeue (and not enqueue) the item you want to remove. Here's an example where I want to remove 5 from a list containing the numbers 1-10...

std::list<int> myList;

int size = myList.size(); // The size needs to be saved to iterate through the whole thing

for (int i = 0; i < size; ++i)
{
    int val = myList.back()
    myList.pop_back() // dequeue
    if (val != 5)
    {
         myList.push_front(val) // enqueue if not 5
    }
}

myList will now only have numbers 1-4 and 6-10.

0 votes
answered Jan 26, 2016 by j-kalz

You can write

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

You can write equivalent code with std::list::remove_if, which is less verbose and more explicit

items.remove_if([] (item*i) {
    bool isActive = (*i)->update();
    if (!isActive) 
        return true;

    other_code_involving(*i);
    return false;
});

The std::vector::erase std::remove_if idiom should be used when items is a vector instead of a list to keep compexity at O(n) - or in case you write generic code and items might be a container with no effective way to erase single items (like a vector)

items.erase(std::remove_if(begin(items), end(items), [] (item*i) {
    bool isActive = (*i)->update();
    if (!isActive) 
        return true;

    other_code_involving(*i);
    return false;
}));
Welcome to Q&A, where you can ask questions and receive answers from other members of the community.
Website Online Counter

...