Dynamically deleting items out of vector ...

[tmason]

Hello,

I have the following code in which I wish to dynamically delete items out of a vector based on the condition of the item in the vector.

The below code works but throws the error: "vector iterator not incrementable" at certain times.

It seems like this happens when the last item in the vector get's deleted. How do I solve this problem?

Thank you for your time.

for (std::vector<PhysicsObject*>::iterator it = CurrentBoxes.begin(); it != CurrentBoxes.end();/*it++*/)
{

	(*it)->CalculatePhysicsOrientation();

	(*it)->DrawMe();

	glm::vec3* CurrentBoxPosition = (*it)->GetCurrentPosition();

	if (CurrentBoxPosition->y < -750.0f) {

		it = CurrentBoxes.erase(it);

	}

	++it;

}

[JLBorges]

for (std::vector<PhysicsObject*>::iterator it = CurrentBoxes.begin(); it != CurrentBoxes.end();/*it++*/)
{

	(*it)->CalculatePhysicsOrientation();

	(*it)->DrawMe();

	glm::vec3* CurrentBoxPosition = (*it)->GetCurrentPosition();

	if (CurrentBoxPosition->y < -750.0f) {

		it = CurrentBoxes.erase(it);
                // erase returns the iterator following the removed element. 
                // that is the element we want to look at next
	}

	else ++it; // ***else added ***
                   // current element was not erased, so move to the next one 
}

[tmason]

It seems like changing the code to the following to work but if there is a smarter way to do the following then I am all ears.

for (std::vector<PhysicsObject*>::iterator it = CurrentBoxes.begin(); it != CurrentBoxes.end();/*it++*/)
{

	(*it)->CalculatePhysicsOrientation();

	(*it)->DrawMe();

	glm::vec3* CurrentBoxPosition = (*it)->GetCurrentPosition();

	if (CurrentBoxPosition->y < -750.0f) {

		it = CurrentBoxes.erase(it);

	}

	if (it != CurrentBoxes.end()) {
		
		++it;

	}
}

[tmason]

Thanks @JLBorges!

That was fast...

[keskiverto]

Erase returns:

An iterator pointing to the new location of the element that followed the last element erased by the function call. This is the container end if the operation erased the last element in the sequence.

Lets say that the sequence is ABCD.
Point is at B.

If you erase B, the point will be at C. If you don't, then point is still at B.
Now you advance point.

If you did erase B, the next iteration looks at D.


In other words, you want to look at C after the B, don't you? You should move to C either by erase or by ++.

[tmason]

I see; so it++ instead of ++it?

Is that what you are saying?

I see that you are saying I could do both but I am not sure how I would use purely the erase() method versus the ++it/it++ methods.

Thank you for your time.

[keskiverto]

JLBorges did already give you the if..else. I was too slow.

[tmason]

OK, thanks to you both!

[JLBorges]

The erase-remove idiom: http://en.wikipedia.org/wiki/Erase-remove_idiom

// http://www.stroustrup.com/C++11FAQ.html#for
for( PhysicsObject* p : CurrentBoxes ) { p->CalculatePhysicsOrientation(); p->DrawMe() ; }

http://www.stroustrup.com/C++11FAQ.html#lambda
static const auto is_erasable = [] ( PhysicsObject* p ) { return p->GetCurrentPosition().y < -750.0f ; }

// http://en.cppreference.com/w/cpp/algorithm/remove (with erase-remove idiom)
CurrentBoxes.erase( std::remove_if( CurrentBoxes.begin(), CurrentBoxes.end(), is_erasable ), CurrentBoxes.end() ) ;

[tmason]

Wow, OK, I need to brush up on modern c++.

I haven't used lambas or even heard of the erase-remove idiom before ...

Thanks again, @JGBorges.