• Advertisement
Sign in to follow this  

proper use of std::vector erase()

This topic is 2320 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement