Iterator doesn't work properly

[eker676]

Here is a snippet of my code that has the problem

for(std::vector<int>::iterator i = list.begin(); i != list.end(); ++i)
{
	if( ( (*i) % 3) == 0)
	{
		outfile3 << (*i) << std::endl;
		list.erase(i);
	}		
}

This snippet searches a vector of integers for items divisible by 3 using an iterator. Then it writes the number to a file and erases that number from the vector.
The problem is that when the program encounters a value divisible by 3 it deletes it from the vector and then crashes.
Here is the error.

Debug Assertion Failed!

("this->_HasContainer()",0)

_Myt& operator++()
		{	// preincrement
		_SCL_SECURE_VALIDATE(this->_Has_container());

EDIT:
Funny thing if I compile the program in VC++ in the Release version the program works perfectly.

[jsmith]

erasing elements from a vector invalidates all iterators to the vector, per the standard.

[eker676]

This error only occurs when trying to compile the program in debug version of VC++. The program works fine if it is built as a release version.

Could you suggest a solution to this problem?

[jsmith]

You are taking advantage of non-standard behavior. It just so happens to work in release mode in your particular test case.

You cannot do what you are doing above. Once you erase an element from the vector, you must not use the iterator again.

The best way to do this is to use a combination of std::remove_if() and the erase() method that takes a first and last iterator.

[guestgulkan]

I agree with Mr Smith - erasing an element should invalidate the iterator.

This does not work for me in either debug or release mode.

[Disch]

This error only occurs when trying to compile the program in debug version of VC++. The program works fine if it is built as a release version.

This is because the Debug version does more runtime checks to catch errors like this. The error is in fact occuring in Release as well -- however it will only happen under the right circumstances (ie: if the vector needs to reallocate the buffer somewhere else due to a resize). It's much easier to detect bugs like this when Debug complains rather than waiting for your program to crash under just the wrong circumstances (leaving you wondering why your program only crashes sometimes and works just fine other times).

One possible solution here is to use erase()'s return value:

std::vector<int>::iterator i = list.begin();
while(i != list.end())
{
  if( ((*i) % 3) == 0 )
  {
    outfile3 << *i << std::endl;
    i = list.erase(i);
  }
  else
    ++i;
}

EDIT -- I type too slow ;_;

[eker676]

Thanks all. Disch I used your solution and it works great.

[jsmith]

Just as an FYI... if you have a large vector, repeat erases will be hideously expensive. (I suspect you don't; but just to keep in mind, erases from anywhere but the end in a vector are very expensive. Consider using a deque instead).

[eker676]

Okay, I'll keep that in mind. Thanks!

[jsmith]

And for that matter, although you can implement what you want in 1 line of code using boost::lambda, remove_if, and list.erase(), that is no more efficient than Disch's algorithm (it's just less typing if you are familiar enough with boost::lambda to get it right on the first try--which is no easy task).

[eker676]

I'm not familiar with the boost libraries but I think I might educate myself.

[jsmith]

So it would be something like

myList.erase( std::remove_if( myList.begin(), myList.end(), boost::lambda::_1 % 3 == 0 ),
   myList.end() );