# Circular Method Calls

The title may be a little wrong for what I am asking, but that was the closest I could think of.

My problem is I am making a game that includes a World class and a WorldObject class. the World class contains a std::map<unsigned int, WorldObject*> in order to keep track of all objects that are in the world. On the flip side, the WorldObject class has an unsigned int as a unique ID, and a World* to the world that it is currently in.

When it comes time for the object to be removed from the world and deleted, I made it so that can either be done by calling WorldObject::removeFromWorld() or World::removeObject(WorldObject*). The problem is, I'm not entirely sure how to set up these methods so the object is removed from the map in World, and the World* reference is removed from the object all in one fell swoop. I'm still relatively new to C++, so I'm not very sure how memory management works with deleting pointers and such, but this is what I was able to come up with my limited knowledge:

WorldObject.cpp

WorldObject::~WorldObject()
{
world = nullptr;
}

void WorldObject::removeFromWorld()
{
world->removeObject(this);
}


World.cpp

bool World::removeObject(WorldObject* object)
{
if (contains(object))
{
objectList[object->getObjectId()] = nullptr;
delete object;
return true;
}

return false;
}


This seems a little too circular for me, and the main problem that I'm not sure will work or not happens if WorldObject::removeFromWorld() is called. If this method is called, it is placed on the stack and then calls World::removeObject() , but this method deletes the calling object. I know objects can "commit suicide" but I dont know if that makes it so the calling object can be deleted before World::removeObject returns.

Anybody who could confirm if this method works, or even give a more efficient solution (as I'm sure there is) would be greatly appreciated.

Do you really need two ways of deleting an object? If there is no concrete reason for an object to be able to destroy itself, you could scrap the WorldObject::removeFromWorld() method entirely.

Otherwise, your compiler will probably allow you to call a method that destroys the caller, but I wouldn't say this is a good idea. Consider what could happen if you added an innocent line after the removal:

void WorldObject::removeFromWorld()
{
world->removeObject(this);
this->logMyRemoval(); // oops, this doesn't exist anymore!
}


One option is to use smart pointers like Luke B. proposed. Another option is to turn this to a removal request: when you request removal, don't actually perform it, just mark the object for removal. And then once per frame, after every processing is done, remove all objects that are marked for removal in one step.

• After re-thinking the way I'm going to use objects in the game, I am going to take your advice and both remove the WorldObject::removeFromWorld() method, and make the other method create a removal request instead of a strait-up removal. I'm also going to see if I can't look into smart pointers like Luke B. suggested. Both are great answers, but I'm going to accept this one. – sm81095 Sep 16 '13 at 20:03
• +1 for converting to a removal request; I like to use a markForKill method on my entities. – ThorinII Oct 31 '13 at 3:21

As far as I know, if you don't use the object after it's destroyed there is no problem (otherwise you will be accessing a deleted object which is probably undefined behaviour).

You could avoid the trouble of managing the memory by having std::unique_ptr<WorldObject> instead of WorldObject* and you would remove from the map simply by calling objectList.erase(object->getObjectId());

Also, making world = nullptr in your destructor is not necessary, since your WorldObject does not own the World pointer, you would only need a destructor if your WorldObject were to destroy the World object.

• I looked into smart pointers a while back, when I was even less experienced with C++ and they quickly terrified me into never thinking of them again. But I'll take another look at them, thanks for the suggestion. – sm81095 Sep 16 '13 at 20:01