std::list problem and crash

Started by
4 comments, last by suliman 16 years, 6 months ago
This did work on vc++6.0 now i use vc++2005 so that might give some answers. When removing objects from this list it works like 94% of the times but otherwise it crashes with debug assertion failed expression: list iterator not incrementable

	
std::list<gameObject *>::iterator kill;
std::list<gameObject *>::iterator it;							
for ( it = myList.begin(); it != myList.end(); it++ ){			
	gameObject * uu=*it;


	//do stuff, like check removeMe if it should be removed


	//remove
	if(uu->removeMe)
	{

		kill=it;
		it++;
		www.objectList.erase(kill);
		delete uu;
	}

	}



It seemes sometimes removing an object (actually the list contains object pointers i know) from the list will corrupt it, causing a crash, but most often the list stays intact. I have the same problem with another list of similar type. Any suggestions? Erik
Advertisement
www.objectList.erase(kill);

This line invalidates the iterator "kill", as well as any other iterators pointing to the element being erased (e.g. "it").

For ordered containers (such as std::list), erase(it) returns a legal, valid iterator to the next element, or end() if there isn't one. Your loop can be rewritten as:

std::list<gameObject *>::iterator it;	for ( it = myList.begin(); it != myList.end(); ){	gameObject * uu=*it;	//do stuff, like check removeMe if it should be removed	//remove	if(uu->removeMe)	{		it = myList.erase(it);		delete uu;	}	else	{		++it;	}}


Why your code works even 1% of the time with iterator debug checks enabled on VS2005 is beyond me, but the above should allow it to work properly. Note that I don't increment it when I erase(), since I'm already assigning it to the return, which is already the next element.

It may also be worth pointing out that the error message is complaining about the "list iterator", not the list itself. This is your first hint you might (do, in this case) have an invalid iterator as the problem, rather than a corrupted container (although using an invalid iterator could certainly cause container corruption as a side effect).
Use the SC++L:
#include <algorithm>bool shouldRemove(gameObject * object) {     // decide whether to delete the object          if (object->removeMe) {         delete object;         return true;     }     return false;}// delete the objectsmyList.erase(std::remove_if(myList.begin(), myList.end(), shouldRemove), myList.end());

Also try to replace your list content with boost.shared_ptr - this saves you from manually deleting the removed game objects.

HTH,
Pat.
Quote:Original post by MaulingMonkey
www.objectList.erase(kill);

This line invalidates the iterator "kill", as well as any other iterators pointing to the element being erased (e.g. "it").


He presumably understands this, which is why he made 'kill' as a copy of 'it' before incrementing.

The problem is that the code, as written, increments 'it' *twice* when something is removed (once directly and once due to how the for loop is written). Presumably, this causes a detectable when you need to remove the last item ;) and also a more subtle problem the rest of the time: if two elements in a row need to be removed, the second one won't be.

You can't get around this by just not incrementing 'it' manually, because then the copying has no effect - both iterators are invalidated (since they have the same value).

Which is why, to do it yourself, OP, you take the rest of MM's advice:

Quote:For ordered containers (such as std::list), erase(it) returns a legal, valid iterator to the next element, or end() if there isn't one. Your loop can be rewritten as:

std::list<gameObject *>::iterator it;	for ( it = myList.begin(); it != myList.end(); ){	gameObject * uu=*it;	//do stuff, like check removeMe if it should be removed	//remove	if(uu->removeMe)	{		it = myList.erase(it);		delete uu;	}	else	{		++it;	}}



But why struggle with that, when you can use the standard library algorithms (as suggested by darookie)? [smile]
Quote:Original post by Zahlman
Quote:Original post by MaulingMonkey
www.objectList.erase(kill);

This line invalidates the iterator "kill", as well as any other iterators pointing to the element being erased (e.g. "it").


He presumably understands this, which is why he made 'kill' as a copy of 'it' before incrementing.


My brain appears to grok nonlocal initialization poorly at the moment ;_;.

In my defense, I was distracted by the shiny mismatched variable names (e.g. "www.objectList" vs "myList") and crimes against proper whitespace usage. I must have forgotten having editing out that increment before applying those changes.

I knew it seemed odd for the code to not 'asplode all the time >_>
thanks a lot guys! Now i can play my game for more then 8 seconds. I guess that explains why it only crashed sometimes (due to the fact that it worked unless it was the last object in the list)

Upgrading all old crappy lists now.

Erik

This topic is closed to new replies.

Advertisement