I'm trying to remove items from a C++ linked list using erase
and a list iterator:
#include <iostream>
#include <string>
#include <list>
class Item
{
public:
Item() {}
~Item() {}
};
typedef std::list<Item> list_item_t;
int main(int argc, const char *argv[])
{
// create a list and add items
list_item_t newlist;
for ( int i = 0 ; i < 10 ; ++i )
{
Item temp;
newlist.push_back(temp);
std::cout << "added item #" << i << std::endl;
}
// delete some items
int count = 0;
list_item_t::iterator it;
for ( it = newlist.begin(); count < 5 ; ++it )
{
std::cout << "round #" << count << std::endl;
newlist.erase( it );
++count;
}
return 0;
}
I get this output and can't seem to trace the reason:
added item #0
added item #1
added item #2
added item #3
added item #4
added item #5
added item #6
added item #7
added item #8
added item #9
round #0
round #1
Segmentation fault
I'm probably doing it wrong, but would appreciate help anyway. thanks.
The core problem here is you're using at iterator value, it
, after you've called erase
on it. The erase
method invalidates the iterator and hence continuing to use it results in bad behavior. Instead you want to use the return of erase
to get the next valid iterator after the erased value.
it = newList.begin();
for (int i = 0; i < 5; i++) {
it = newList.erase(it);
}
It also doesn't hurt to include a check for newList.end()
to account for the case where there aren't at least 5 elements in the list
.
it = newList.begin();
for (int i = 0; i < 5 && it != newList.end(); i++) {
it = newList.erase(it);
}
As Tim pointed out, here's a great reference for erase
- http://www.cplusplus.com/reference/stl/list/erase/
When you erase an element at position it
, the iterator it
gets invalidated - it points to a piece of memory that you just freed.
The erase(it)
function returns another iterator pointing to the next element to the list. Use that one!
You're invalidating your iterator when you erase()
within the loop. It would be simpler to do something like this in place of your erase loop:
list_item_t::iterator endIter = newlist.begin();
std::advance(endIter, 5);
newList.erase(newlist.begin(), endIter);
You might also be interested in the erase-remove idiom.
I do this:
for(list<type>::iterator i = list.begin(); i != list.end(); i++)
{
if(shouldErase)
{
i = list.erase(i);
i--;
}
}
Edited because I'm a bonehead that can't read apparently lol.