Jump to content
  • Advertisement
Sign in to follow this  
PiCroft

stl:list iterator error: list iterators incompatible

This topic is 2606 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I've got code which loops through a list using an iterator. It performs a check on that object and if the condition fails, the item is deleted using the list::erase() method.


std::list<cHBGameObject*>::iterator i;

//remove any object that has passed the cutoff
if(!lObjects.empty())
{
i = lObjects.begin();

while (i != lObjects.end())
{
if((*i)->checkIfOverDeletionThreshold(xDeletionPoint))
lObjects.erase(i);
else
++i;
}

}


In this case, the crash occurs if there is only 1 remaining object and the object is deleted, then when it returns to the "while" statement, the error is thrown.

I can sort of see where I'm going wrong, I think when it hits "while", because the list is empty, it may be that the iterator no longer points to anything valid. I have looked up examples to make sure I'm not doing it wrong but every analogous example I have seen have suggested this approach to looping through a list where you may delete objects. Besides, being able to delete objects in this manner is one of lists' advantages over vectors, no?

Could someone kindly point out where I am going wrong?

Share this post


Link to post
Share on other sites
Advertisement
The erase method returns a new iterator pointing to the next element. Try something like this:


while(i != objects.end()) {
if(i->delete())
i = objects.erase(i);
else
++i;
}

Share this post


Link to post
Share on other sites
Apologies, I fixed it, here was the problem:

std::list<cHBGameObject*>::iterator i;

//remove any object that has passed the cutoff
if(!lObjects.empty())
{
i = lObjects.begin();

while (i != lObjects.end())
{
if((*i)->checkIfOverDeletionThreshold(xDeletionPoint))
lObjects.erase(i);
else
++i;
}

}


the bolded bit needed to be changed to:


i = lObjects.erase(i);


So that i recieved a valid pointer after the erasure.

Sorry for the wast of time, though if someone else stumbles on this problem, I hope this helps :D

Edit: Thanks Erik, I just spotted that myself :)

Share this post


Link to post
Share on other sites
It doesn't look like you reassign i after a deletion. erase should return an iterator to the next element in the list.
// Try changing
iObject.erase(i)
// To this
i = lObject.erase(i);

To simply things, you might just use std::remove_if, which is already provided in the <algorithm> header.

You would just need to setup the predicate to do the same test you do now. You could even do this as a lambda if you have a compiler with support for C++11.

(Code below is untested).


std::remove_if(lObjects.begin(), lObjects.end(), [&xDeletionPoint](cHBGameObject* i) -> bool
{
return (*i)->checkIfOverDeletionThreshold(xDeletionPoint);
});


[edit]
Was too slow :)

Share this post


Link to post
Share on other sites
When you erase something you invalidate the iterator. Try this instead:



//...
if((*i)->checkIfOverDeletionThreshold(xDeletionPoint))
i = lObjects.erase(i);
else
++i;
// ...


EDIT: was waaaay too slow

Share this post


Link to post
Share on other sites

It doesn't look like you reassign i after a deletion. erase should return an iterator to the next element in the list.
// Try changing
iObject.erase(i)
// To this
i = lObject.erase(i);

To simply things, you might just use std::remove_if, which is already provided in the <algorithm> header.

You would just need to setup the predicate to do the same test you do now. You could even do this as a lambda if you have a compiler with support for C++11.

(Code below is untested).


std::remove_if(lObjects.begin(), lObjects.end(), [&xDeletionPoint](cHBGameObject* i) -> bool
{
return (*i)->checkIfOverDeletionThreshold(xDeletionPoint);
});


[edit]
Was too slow :)


Just be careful here, because remove_if does not *actually* remove the elements from the list. To do that you need to do,


lObjects.erase(std::remove_if(lObjects.begin(), lObjects.end(), Predicate()), lObjects.end());


where Predicate is a unary functor that calls the checkIfOverDeletionThreshold() on the cHBGameObject pointers.

-Josh

Share this post


Link to post
Share on other sites
std::list has member functions remove() and remove_if(). These perform both the remove and erase action, and are efficient because it avoids traversing the list multiple times.

In general, you don't want to be using std::list though, std::vector is almost always a better choice. The cache coherency and allocation minimisation will mostly trump the theoretical advantages of std::list. You rarely need random insertion, and the cost of random removal can be minimise by using remove_if(), or the swap and pop trick if the order isn't important.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!