Iterating through a vector and deleting contents

Question

Is this a good or standard practice to code like this to loop through a vector while deleting unwanted elements from it without losing performance. If there is a faster way please suggest it. This vector is of the form std::vector<AnimationState*> activeAnimations;

void AnimationPack::removeDeadAnimations() { int counter = 0; std::remove_if(activeAnimations.begin(), activeAnimations.end(), [&](AnimationState*& animation) { if (animation->isActive()) { counter++; return true; } else return false; }); activeAnimations.erase(activeAnimations.end() - counter, activeAnimations.end()); }

Edited version

void AnimationPack::removeDeadAnimations() { activeAnimations.erase(std::remove_if(activeAnimations.begin(), activeAnimations.end(), [&](AnimationState*& animation) { if (animation->isActive()) return true; else return false; }),activeAnimations.end()); }

Edited Code (As suggested from comments)

void AnimationPack::removeDeadAnimations() { activeAnimations.erase(std::remove_if(activeAnimations.begin(), activeAnimations.end(), [](AnimationState*& animation) { return animation->isActive(); }), activeAnimations.end()); }

auto it = std::remove_if(v.begin(), v.end(), ...); v.erase(it, v.end()); is pretty well idiomatic – it's hard to answer without knowing what you find scary about it. — ildjarn
– ildjarn, Commented Jun 6, 2018 at 17:45
propose this code to them and take it as an opportunity to teach them about remove_if and whatever else they find confusing, this is very common. — kmdreko
– kmdreko, Commented Jun 6, 2018 at 17:49
You can probably just write return animation->isActive();. — François Andrieux
– François Andrieux, Commented Jun 6, 2018 at 17:51
Please do not edit the question in a way that invalidates the comments and answers. It destroys important context for future readers. If you have further questions about the "fixed" code, either post another question or append the new code at the end of the question, noting that it's a new iteration. — François Andrieux
– François Andrieux, Commented Jun 6, 2018 at 17:54
Note that if activeAnimations contains owning pointers you must remember to delete them yourself. If that's the case, consider using std::vector<std::unique_ptr<AnimationState>> or std::vector<AnimationState> instead. — François Andrieux
– François Andrieux, Commented Jun 6, 2018 at 18:04

AMA · Accepted Answer · 2018-06-06 18:11:08Z

Yes, it is called an erase-remove idiom.

Quote from Wikipedia:

The erase–remove idiom is a common C++ technique to eliminate elements that fulfill a certain criterion from a C++ Standard Library container.

erase can be used to delete an element from a collection, but for containers which are based on an array, such as vector, all elements after the deleted element have to be moved forward, to avoid "gaps" in the collection.

The algorithm library provides the remove and remove_if algorithms for this.

These algorithms do not remove elements from the container, but move all elements that don't fit the remove criteria to the front of the range, keeping the relative order of the elements. This is done in a single pass through the data range.

remove returns an iterator pointing to the first of these elements, so they can be deleted using a single call to erase.

Thinkal VB · Accepted Answer · 2018-06-06 19:02:31Z

Removes and delete the element from the vector while iterating through it.

 void AnimationPack::removeDeadAnimations() { activeAnimations.erase(std::remove_if(activeAnimations.begin(), activeAnimations.end(), [&](AnimationState*& animation) { if (animation->isActive()) return false; else { delete animation; return true; } }), activeAnimations.end()); }

You are capturing everything by reference now while in fact you don't need to capture anything in the lambda. You should change [&] to [].

Collectives™ on Stack Overflow

Iterating through a vector and deleting contents

2 Answers 2

Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Related