Vector Iterators, Erase Loops and Frustration

[abek42]

Hi All,

I've been flummoxed by this bizarre behaviour and would like insight on how to get around it.
--Snip--
[code=c]
struct polygon
{
int i; int j;
}

void doStuff(vector< polygon>* myPolygons){
vector<polygon> tempPoly;

tempPoly.swap(* myPolygons)
vector<polygon> ::iterator itrV;

for(itrV=tempPo ly.begin(); itrV!=tempPoly. end();itrV++){ //1
polygon p = *(itrV);
if(p.i==-1){
itrV = tempPoly.erase( itrV); //0
}
tempPoly.swap(* myPolygons);
}
[/code]
--Snip--
Code fails at //1 (in VC2008, Debug mode) with an Assert Error in Vector around Line:117. The itrV apparently holds an invalid reference
Most other people make the mistake at //0 where they just call erase but don't use the returned iterator. I am doing that... so why does VC++ hate me?

Any inputs (excluding the idea that I can turn off the _SCL_SECURE_VAL IDATE_RANGE checking) I would like to understand what is the issue with the code and what is the right way to write erase loops for vectors.

Regards,
Abe

P.S. : myPolygons may start with 0,1,2... n elements and all might get deleted inside the loop

[abek42]

Couldn't wait to hear back... I resolved it with what I feel is a dirty hack.

[code=c]
struct polygon
{
int i; int j;
}

void doStuff(vector< polygon>* myPolygons){
vector<polygon> tempPoly;

tempPoly.swap(* myPolygons)
vector<polygon> ::iterator itrV;

for(itrV=tempPo ly.begin(); itrV!=tempPoly. end();itrV++){ //1
polygon p = *(itrV);
if(p.i!=-1){
(*myPolygons).p ush_back(p);
}
}
}

[/code]

Regards,
Abe

[Banfa]

Most other people make the mistake at //0 where they just call erase but don't use the returned iterator. I am doing that... so why does VC++ hate me?

It's not VC++ its STL behaviour but that is an aside.

OK you didn't fall into the trap of not using the return value of erase, however you did fall into the trap of not reading the documentation for erase or reading it but not fully comprehending its implications.

erase returns the iterator of the item after the one just erased, that means it returns the iterator of the next item to look at. However you have an it++ in the for loop post condition so you immediately increment the operator off the next item to examine and onto the one after that one. Worst still if erase returned end(), which it could, you just performed the equivalent of end()++, something which I am pretty sure results in undefined behaviour, in your case I guess an assertion error.

If you call erase then it returns the next iterator otherwise you have to increment the current iterator. You should increment the iterator if you call erase. Pseudo code is something like

for(it = begin(); it != end();
{
    if (EraseCondition IS true)
    {
        it = erase()
    }
    else
    {
        it++;
    }
}