myList.erase(reverse_iterator)

[Kathy L]

i want to erase some elements in the list pointed by a reversed iterator,and i wrote like this
list<int>::reverse_iterator it=mylist.rbegin();
mylist.erase(it.base());

but after the erasing of a specific element,the iterator it is no longer valid.so i also need to make sure that the iterator will move to the next element automatically,so i changed my code to this:
it=mylist.erase(it.base());
and that's when i encountered the problem.
the compiling error says that a const_iterator type can't be converted to a reverse_iterator type.

how do i solve this problem?

[rollie]

The problem is erase returns a const_iterator (note, forward iterator) rather than a non-const reverse_iterator. Since there is no implicit conversion between the two, you get the compile error. I think you can do what you want with

mylist.erase((it++).base());

although there is nothing wrong with explicitly creating a temporary iterator to handle the delete.

Also, take a look at the list<T>::remove and list<T>::remove_if methods, which may simplify your code a bit.

--Rollie

[Kathy L]

thanks,rollie.

but here is another problem.

i'm actually simulating a circle using list,so i also need to make sure that the iterator will return to mylist.rbegin() if it==mylist.rend(),

what should i do?

here is my original code:

#include<iostream>
#include<list>
using namespace std;
list<int> l;
int n,k,m;
int main(){
freopen("in.txt","r",stdin);
while(cin>>n>>k>>m&&n){
for(int i=1;i<=n;i++) l.push_back(i);
k%=n;
m%=n;
list<int>::iterator it1=l.begin();
list<int>::reverse_iterator it2=l.rbegin();

while(!l.empty()){
k%=l.size();
m%=l.size();
for(int j=0;j<k;j++){
it1++;
if(it1==l.end())
it1=l.begin();
}
for(int j=0;j<m;j++){
it2++;
if(it2==l.rend())
it2=l.rbegin();
}
if(*it1==*it2){
it2++;
if(it2==l.rend()) it2=l.rbegin();
it1=l.erase(it1);
if(it1==l.end()) it1=l.begin();
}
else{
printf("%d %d ,",*it1,*it2);
it1=l.erase(it1);
it2=l.erase(it2.base());
if(it1==l.end()) it1=l.begin();
if(it2==l.rend()) it2=l.rend();
}
}
printf("\n");
}
return 0;
}

[rollie]

After the erase((itr++).base()); you should just be able to check if itr == l.rend(), as it both deletes the element and increments the iterator.

[Kathy L]

thanks a lot.

[Disch]

ah okay... this is where this came from.

As I mentioned in another thread ( http://cplusplus.com/forum/general/24602/ ), this is bad code:

erase((itr++).base());

The reason this is bad is because it modifies 'itr' twice in the same sequence point, which has undefined behavior and may cause your program to explode if the compiler decides to make the changes in a certain order.

erase modifies the contents of itr (it erases it) and then the ++ operator increments the iterator, which may not be possible if the iterator was erased.

So if the compiler decides to erase first (which is likely, but not guaranteed), you're hosed.


The moral of the story here is to avoid compound statements, as they can lead to some unexpected and nasty bugs unless you really know what's going on. It doesn't hurt to break this up into 2 separate lines of code.

[rollie]

I believe 'itr++' resolves to list<T>::iterator::operator++(). Erase would not be able to manipulate 'itr', because it needs to know the result of the operator++ method, guaranteeing that 'itr++' takes place before anything is erased. It's possible to implement the 'operator++()' method to return any random iterator object; it won't necessarily be the same as the object on which the method is called.

I do agree that having an explicitly defined temp iterator is clearer, but I don't think your logic above is correct. Of course, I've been wrong before :)

[Disch]

I believe 'itr++' resolves to list<T>::iterator::operator++(). Erase would not be able to manipulate 'itr', because it needs to know the result of the operator++ method

Ah crap, I do believe you're right.

My mistake!