I thought the following code would work but it crashes when the target widget is at the end of the vector.
for(std::vector<AguiWidget*>::iterator it = children.begin(); it != children.end(); ++it) { if((*it) == widget) it = children.erase(it); } I want it to go through and delete any instance it finds of widget. I understand this method is N^2 but since this is event driven it is fine. I just don't know why this should fail. When it does, 'it' == widget.
Thanks
You can use the erase-remove idiom to erase all elements that are equal to widget.
children.erase(remove(children.begin(), children.end(), widget), children.end()); You should probably stick to lists if you want to use erase like that. But the problem is that you invalidate your iterator, and then try to increment it. Try this instead.
for(std::vector<AguiWidget*>::iterator it = children.begin(); it != children.end();) { if(*it == widget) children.erase(it++); else ++it; } Notice that I'm not incrementing the iterator inside the for-loop statement.
++it sends you off into goofy-land.
You do realize you're comparing pointers, not dereferences, right?
Can you tell us what happens if you use the remove-erase idiom? This is going to be fast(er than your code) and correct:
children.erase(std::remove_if(children.begin(), children.end(), std::bind1st(std::equal_to<AguiWidget*>(), widget))); Also, don't forget to delete the pointers first.
for_each_if(children.begin(), children.end(), std::bind1st(std::equal_to<AguiWidget*>(), widget), Delete()); Of course, you'll have to be sure no two pointers point at the same object.
To complement Blastfurnace's answer, you can also do so with a simple for loop, if you do it backward.
for (widgets::reverse_iterator it = children.rbegin(), end = children.rend(); it != end; ++it) { if (*it == widget) { children.erase(it.base()); } }