Advertisement

"List iterator not incrementable", error enhanced for loop and remove

Started by January 03, 2015 07:50 PM
8 comments, last by Pether 10 years ago

I am trying to fiddle with lists and .remove, but when it removes something it crashes the program. Searching google the error happens when the iterator goes outside the valid spectrum, and they give help to solve it. But i havent found how to do it with the enhanced for loop? Am i out of luck using such a loop? Or is there something i am missing?


for (auto&& b : bullets) if (!b->alive) bullets.remove(b);
You're modifying the collection you're iterating over.

If you're going to do that you need to use your own for loop, or better yet, use std::remove, and then std::list::erase.

Also, why are you using a linked list? Use std::deque or std::vector unless you have a very specific use case in mind.

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.

Advertisement

Well, in this case, std::list has a remove_if() member function that has better performance by using non-member remove() followed by erase().

Well, in this case, std::list has a remove_if() member function that has better performance by using non-member remove() followed by erase().


Yes, except he has a list of pointers (of some variety). Which means that the performance of std::lsit::remove_if as opposed to say std::remove_if/erase on a vector or deque is not particularly great.

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.

I dont have much experience in lists, vectors or deques. I have earlier programmed in Java and there lists are the go to thing when handling lots of stuff, like bullets.

I know the error comes from modifying the list, but dont know how to fix that error.

I want to go trough the list, or whatever, and remove items.

I changed to remove_if and created a function to check if it's alive or not.

Changed some more stuff so the only pointer i still got is the spritesheet.

The thing works now, but you were saying it isn't the most optimal way?

I dont have much experience in lists, vectors or deques. I have earlier programmed in Java and there lists are the go to thing when handling lots of stuff, like bullets.
I know the error comes from modifying the list, but dont know how to fix that error.

I want to go trough the list, or whatever, and remove items.

I changed to remove_if and created a function to check if it's alive or not.
Changed some more stuff so the only pointer i still got is the spritesheet.

The thing works now, but you were saying it isn't the most optimal way?


In Java a List<> is a generic object of which you can instantiate a concrete instance like ArrayList. In C++ std::list is a linked list. You probably wanted to use an std::vector, which is a dynamic array, similar to ArrayList in Java.

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.

Advertisement

http://baptiste-wicht.com/posts/2012/12/cpp-benchmark-vector-list-deque.html

http://java.dzone.com/articles/c-benchmark-%E2%80%93-stdvector-vs

each type of container has a reason to exist, don't think for a second that there is 1 choice that is "better" it simply isn't true, period ... I've read 4 sets of benchmarks and programmed in embedded C++ for over 8 years (not recently though)

The only thing you need to know though is: The BEST container for a given job is NOT something you can figure out by using logic. Because the performance of the use case is HEAVILY affected by object size, locality, operation ratios, and variations in data set vs cache size (and even branch predictors). So the ONLY way to know what container is best for a job is to write the code and benchmark it.

Personally, I used to start with my default goto item "std::deque" and then switch to vector and list if I was pretty sure they were better (and in my experience about half my simple linear containers ended up being vector, most of the rest deque, and a small percentage ended up as lists - usually the obvious ones that were LARGE and had many RANDOM or FRONT inserts), BTW occasionally you can get the most bang by not switching container types, but by switching container usage (like mentally swapping the MEANING of front and back if your vector has a lot of FRONT inserts but few back inserts)

Hey.

if you want to get max speed out of this you could create a functionobject class that is
used in remove_if(); go with a object so you can pass other params to the class and not the predicate_function set up

this way you can process the whole list and at the same time remove any elements that meet you
criteria.

Its a bit of work but I think its worth it for some lists things

Here Some Thing Like This Here.

.



//-----------------------------------------------------------------------------------------
//this is our Function object used in remove_if we pass our self to
//we need to set the render values before passing in 
//------------------------------------------------------------------------------------------
class cFunctionObject
{
public:
	D3DXMATRIX *View;
	D3DXMATRIX *Projection;
	D3DXMATRIX *LightView;
	D3DXMATRIX *LightVolume;
	
	
	cFunctionObject(void)
	{
	}//end
	/////////////////////////////////////////////////////////

	~cFunctionObject(void)
	{
	}//end 
	////////////////////////////////////////////////////////////////
const cFunctionObject& cFunctionObject::operator=(const cFunctionObject& s)
{
View		= s.View;
Projection		= s.Projection;
LightView		= s.LightView;
LightVolume	= s.LightView;
		
return *this;
}




//----------------------------------------------------------------------------------------------------
//A function object, or functor, is any type that implements operator(). This operator is referred to as 
//the call operator or sometimes the application operator. The Standard Template Library uses function 
//objects primarily as sorting criteria for containers and in algorithms.
//you want all  the testing for deleting element in here this gets call for each list item
//----------------------------------------------------------------------------------------------------
bool cFunctionObject::operator() (cDropPodSate &droppod) //droppod is what object type your list holds
{ 
//does list menrs update returns true to remove from the list
bool done = droppod.ProcessDropPod(mTerrain, *timedelta);

 if(done == false)
 {
	D3DXMATRIX vp =  (*View) * (*Projection);
	D3DXMATRIX Scale, Rotation;
	D3DXMATRIX World;
	//render the pod		

}//end render pod

 return done;//if its true it gets removed
}//end operator()
//////////////////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////





//----------------------------------------------------------------------------------------
//must call it before using pass things need to the object
//----------------------------------------------------------------------------------------
void SetFunctionObject(D3DXMATRIX *view,
		D3DXMATRIX *projection,
		D3DXMATRIX *lightView,
		D3DXMATRIX *lightVolume)
{
	View		= view;
	Projection		= projection;
	LightView		= lightView;
	LightVolume	= lightVolume;
		

}//end SetFunctionObject
////////////////////////////////////////////////////////////////////////////////////////////////
///////////////////////////////////////////////////////////////////////////////////////////////


};//end class cFunctionObject
//////////////////////////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////////////




And Used Like This.

.


//we are using a function object to pass to remove_if which will do the processing and remove them if function object returns true
//also in this cas does the rendering of the pod as well
cFunctionObject FunctionObject;
FunctionObject.SetFunctionObject(&View,
			&Projection,
			LightView,
			LightVolume);


	

//the remove_if will go through the whole list and remove when pods process is true
DropPodsList.erase(std::remove_if(DropPodsList.begin(), .DropPodsList.end(), FunctionObject), DropPodsList.end());


and thats all there is Give it a try.

That looks like a lot of unnecessary code, ankhd. You do not need void as a function parameter in C++, that's a C-ism that has no place in C++. The assignment operator is completely unnecessary (the default operator does the same without a single line of code and if the compiler needs to copy it, it would need the copy-constructor not the assignment operator). If a constructor and destructor are empty, they should be omitted unless some special situation requires them (none does here).

And with C++11, none of that is needed because lambdas do it right out of the box without so much verbosity:
[source]
DropPodsList.erase(std::remove_if(DropPodsList.begin(), DropPodsList.end(), [&] (const cDropPodSate &dropPod) -> bool
{
return /* some value based on calculatons using anything accessible in the enclosing scope */
}), DropPodsList.end());
[/source]

It's just for a simple game for a course so doing something too advanced right now would just be a 'waste' of time, already pressured on time as is. But will keep it in mind when i get time to work on my own project again. I'll be changing to vectors as well, thanks for the information about that. Kinda jumped into a gamecourse in c++ without knowing c++ in beforehand so some problems should arise or i'm a genious. (Like wasting 3 hours rewriting code trying to get an error that i thought was in the imageclass when it in fact was me using a shorthand for spritesheet as one parameter instead of the full name..)

This topic is closed to new replies.

Advertisement