Problems erasing from seq. container

Hello,

I usually wouldn't ask in such an straight-forward way as I'm about to do, but I've throughly reviewed my code, and with my current knowledge I can't see the error... The following code just freezes, apparently because of the 2nd block that erases from the list...


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
int main() {
	int ia[] = { 0, 1, 1, 2, 3, 5, 8, 13, 21, 55, 89 };
	vector<int> ivec(ia, ia +(sizeof(ia)/sizeof(int)));
	list<int> ilist(ia, ia +(sizeof(ia)/sizeof(int)));
	//first erasing block
	vector<int>::iterator i = ivec.begin();
	for( ; i != ivec.end(); ++i) {
		if( (*i % 2) == 0 ) {
			ivec.erase(i);
		}
	}
	//second erasing block
	list<int>::iterator j = ilist.begin();
	for( ; j != ilist.end(); ++j) {	
		if( (*j % 2) != 0 ) {
			ilist.erase(j);
		}
	}
	//At this point this won't even run
	for(vector<int>::iterator p = ivec.begin(); p != ivec.end(); ++p) {
		cout << *p << "\t";
	}
	return 0;
}


Could you please tell me what I'm doing wrong, I really cannot see by myself.

Thank you very much in advance.

Jose
iterators become invalid as soon as you erase them. So if you do this:

1
2
3
4
for(; i != foo.end(); ++i)
{
  foo.erase(i);
}


Erasing invalidates i, then when you do ++i, you're incrementing an invalid iterator.

Loops that erase elements should be structured like so:

1
2
3
4
5
6
7
8
9
10
11
while(i != foo.end())
{
  if( need_to_erase )
  {
    i = foo.erase(i);  // reassign i.  Do not increment
  }
  else
  {
    ++i;  // only increment if you did not erase
  }
}
Oh yes, I can see it now. it seems pretty obvious now, I read the theory but I didn't understand it. I appreciate your guidance a lot !
Now, I know this is probably asking a lot but, do you think you can show me how to do it with a for loop ? (if it's possible of course)

Thank you Disch, you're the best !
A for loop wouldn't be practical in this situation because the increment condition isn't fixed. Sometimes you increment and sometimes you don't... so using for doesn't really make sense. You're better off using while.

The only way to do it reasonably would be to make it look exactly like a while loop:

1
2
3
4
5
6
7
8
9
10
11
12
// while(i != foo.end())  // instead of this...
for(; i != foo.end();)  // do this
{
  if( need_to_erase )
  {
    i = foo.erase(i);  // reassign i.  Do not increment
  }
  else
  {
    ++i;  // only increment if you did not erase
  }
}


Ok sir,

Lesson learned. You're a terrefic instructor, did you know that? it's great to have you around.

Keep up the good work ! you know I can't thank enough !

Good luck !

Sincerely,

Jose.
Happy to help.
Topic archived. No new replies allowed.