proper use of std::vector erase()

Started by
2 comments, last by SiCrane 12 years, 6 months ago
Hi,

I wanted to know how to properly delete certain items from a vector..? I thought I was doing it correctly:

vector<vector<FadeElements>::iterator> Temp;
for(vector<FadeElements>::iterator ii = m_FadeElements.begin(); ii!= m_FadeElements.end();++ii)
{
ii->TimeLeft -= dt;
if(ii->TimeLeft <= 0)
{
Temp.push_back(ii);
}
}

for(vector<vector<FadeElements>::iterator>::iterator ii = Temp.begin(); ii != Temp.end();++ii)
{
m_FadeElements.erase(*ii);
}


I thought storing the iterator in another vector and using this data to erase an item would do, but if I have multiple items that should be deleted within the same frame, I get an error (vector iterator out of range). What is the proper way of doing so? I could think of a solution:

vector<int> Temp;
int i = 0;
for(vector<FadeElements>::iterator ii = m_FadeElements.begin(); ii!= m_FadeElements.end();++ii)
{
ii->TimeLeft -= dt;
if(ii->TimeLeft <= 0)
{
Temp.push_back(i);
}
i++;
}

int Erased = 0;
for(vector<int>::iterator ii = Temp.begin(); ii != Temp.end();++ii)
{
m_FadeElements.erase(m_FadeElements.begin() + *ii - Erased);
Erased += 1;
}


However this is giving me the error "vector iterator + offset out of range" on the 3rd element when I have 5 elements (0, 1, 2, 3, 4) to delete, although *ii = 3 and Erased = 3. Do you see any mistakes in my code, or is there another solution?
Advertisement
erase() returns an new iterator to the element after the one being erased. Use that iterator instead of stepping to the next element when you erase an item from the vector.
[source]
for(vector<FadeElements>::iterator ii = m_FadeElements.begin(); ii!= m_FadeElements.end(); /* no ++ii here */)
{
ii->TimeLeft -= dt;

if(ii->TimeLeft <= 0) {
ii = FadeElements.erase(ii);
} else {
++ii;
}
}
[/source]
Note that you shall not step the iterator in the for loop's header in this case.
Thanks a lot! Beside of it working know, your solution seems a lot faster, too (no additional vector insertions and iterators for each deleted object).

Just an additional question considering vectors, while I'm at it so I don't have to open a new post:

can I optimize a vector iteration loop by doing this:

vector<FadeElements>::iterator end = m_FadeElements.end();
for(vector<FadeElements>::iterator ii = m_FadeElements.begin(); ii != end;++ii)
{

}


It seems like caching the end element is a good idea to me, as it saves 1 function call and propably a vector element access per iteration, however I wonder if the compiler isn't intelligent enough to do this by himself? And what if I am erasing elements like in my example, is this even valid?
In terms of efficiency, using erase() in a loop on a std::vector is O(n[sup]2[/sup]). You may want to instead consider using std::remove_if() or the swap and pop idiom if order doesn't matter.


And what if I am erasing elements like in my example, is this even valid?

No, it's not valid if you're erasing in the loop for std::vector.

This topic is closed to new replies.

Advertisement