🔒 Locked 🎮 Unity

c++ std::list

Started by squashed_bug Mar 23, 2007 at 8:39 PM 4 replies 5.6k views

Original Post

Mar 23, 2007 at 8:39 PM


	std::list<IWorldObject*>::iterator it;
	for(it = objects.begin(); it != objects.end(); it++){
		(*it)->Update();
	
	}

The problem is (*it)->Update() may remove itself from the objects list, if this occurs on the last element in the list then the iterator becomes invalid and when the iterator is incremented the "list iterator not incrementable" assertion fails. Anyway to work around this?

Zakwayda

Mar 23, 2007 at 9:01 PM

Quote:
Original post by squashed_bug
*** Source Snippet Removed ***

The problem is (*it)->Update() may remove itself from the objects list, if this occurs on the last element in the list then the iterator becomes invalid and when the iterator is incremented the "list iterator not incrementable" assertion fails.

Anyway to work around this?

How exactly do the objects remove themselves from the list? Perhaps you could post that code so we could see what it is exactly that you're doing.

In any case, that's probably not the best way to go about it. An alternative you might consider is to have the Update() function return a Boolean value indicating whether the object on which it's called is still active after the update (that is, whether it's still 'alive').

Using this method you can write something like this (note: not tested or compiled):

for (...::iterator it = objects.begin(); it != objects.end(); ) {    if ((*it)->Update()) {        ++it;    } else {        delete *it;        it = objects.erase(it);    }}

An even better alternative would be to make use of the 'erase-remove' idiom. However, we begin to see a problem with your current method; the 'raw' pointers that you're storing are not self-managing, and having to delete them manually complicates things and leaves a great deal of room for error. Fortunately, there's a solution to that problem too :)

SimonForsman

Mar 23, 2007 at 9:08 PM

I definitly agree with jyk on that one.

Your objects shouldn't even have to know about the list they are stored in.

squashed_bug

Mar 23, 2007 at 10:31 PM

I have been using this..

	std::list<IWorldObject*>::iterator it, jt;	for(it = objects.begin(), jt = it, jt++; it != objects.end(); it = jt++){		(*it)->Update();		if(jt == objects.end()) break;	}

which does solve the imediate problem.

The objects themselves don't know about the lists they're in, the removal on the list is in the IWorldObject deconstructor.

Ill give you some code pick over..

class IWorldObject;class World : public Engine::ITask{private:	std::list<IDrawable*> drawables;	std::list<IWorldObject*> objects;public:	void Update();	ICamera *camera;	void AddDrawable(IDrawable *d);	void AddObject(IWorldObject *object);	void RemoveObject(IWorldObject *object);	void RemoveDrawable(IDrawable *d);};class IWorldObject : public Physics::IPhysicalObject{public:	World *world;	IWorldObject(World *world);	~IWorldObject();};#include "engine.h"void World::Update(){		std::list<IWorldObject*>::iterator it, jt;	for(it = objects.begin(), jt = it, jt++; it != objects.end(); it = jt++){		(*it)->Update();		if(jt == objects.end()) break;	}	// Draw the objects	std::list<IDrawable*>::iterator kt;	glLoadIdentity();	camera->SetView();	for(kt = drawables.begin(); kt != drawables.end(); kt++){		glPushMatrix();		(*kt)->Draw();		glPopMatrix();	}}void World::AddDrawable(IDrawable *d){	drawables.push_back(d);}void World::AddObject(IWorldObject *object){	AddDrawable(object);	objects.push_back(object);}void World::RemoveObject(IWorldObject *object){	RemoveDrawable(object);	objects.remove(object);}void World::RemoveDrawable(IDrawable *d){	drawables.remove(d);}IWorldObject::IWorldObject(World *world){	this->world = world;	world->AddObject(this);}IWorldObject::~IWorldObject(){	world->RemoveObject(this);}

Do you think it would be better to set a 'isAlive' attribute in IWorldObject then calling delete(this).
eg

// note: conc is a sub class of IWorldObjectvoid Conc::Update(){	if(isExploded){		delete(this); // or isAlive = true;	} else {		if(timer.Expired()) Explode();	}}

hmm maybe it would be best just to implement some memory management.

Thanks for the help.

iMalc

Mar 23, 2007 at 10:52 PM

Using:

delete(this)

on the member function of an object that is stored in a container would cause a disastrous double-delete if it were not that you are storing only pointers, not objects.

Just do what jyk suggests. In fact I have used this exact same approach for this exact same problem (updating objects in my world list), and it works nicely!

[Edited by - iMalc on March 23, 2007 11:52:18 PM]

Zahlman

Mar 24, 2007 at 8:02 AM

Quote:
Original post by squashed_bug
The problem is (*it)->Update() may remove itself from the objects list

"Doctor, it hurts when I do that."

Objects shouldn't know about their containers. After all, you might some day want to use that object outside of a container :s

A standard solution is to make Update() communicate back whether or not the object should be removed, and then use the update function as a predicate for removing objects via the erase-remove idiom. Another problem is storing raw pointers for polymorphism; you'll have a much more fun time with some kind of smart pointer.

// return true if alive after the update.bool IWorldObject::Update();typedef boost::shared_ptr<IWorldObject> objhandle;bool aliveObject(objhandle& handle) {  return handle->Update();}std::list<objhandle> objects;objects.erase(std::remove_if(objects.begin(), objects.end(), std::not1(aliveObject)), objects.end());

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.