Query regarding the correct use of smart pointers

Question

I have the following piece of code in my entityManager.

void GameObjectManager::updateAll(float frametime)
{   
checkAlive();

    // iterate through the entityManager, updating each entity and then adding it into the quadTree.
    for (auto itr = _gameObjects.begin(); itr != _gameObjects.end(); itr++)
    {
        itr->second->Update(frametime);

        tree->AddObject(itr->second);       
    }   

    // iterate once again through the entityManager but this time checking itself against the quadTree.
    for (auto x = _gameObjects.begin(); x != _gameObjects.end(); x++)
    {
        std::vector<std::unique_ptr<Entity>> returnObjects = tree->GetObjectsAt(x->second->GetPosition().x, x->second->GetPosition().y );       

        for ( int n = 0; n < (int)returnObjects.size(); n++ ) 
        {
            std::unique_ptr<Entity>& collided = returnObjects[n];

            if(object->getEntityName() != collided->getEntityName())
            {
                if(object->GetBoundingRect().intersects(collided->GetBoundingRect()))
                {
                    object->Touch(collided);
                }
            }
        }       
    }   
    tree->Clear();
}

What would be the correct use of smart pointers in this example? When I add the entity to the quad tree should I create a shared_ptr, pass it over as a reference or use std::move? I'm leaning towards one of the first two as moving ownership of the pointer would move it from the std::map and this is something I don't want to be doing.

Are there simple rules I should follow as to when passing information? When should I be using references and when should I be using a shared_ptr?

Answer 1

When ownership semantics are employed I use the following basic approach to references.

• Taking references by function parameter. The function is going to use this object but not going to store a reference to it beyond the lifetime of the function. Once the function has returned you are safe to destroy the object without worrying about dangling references or pointers.
• Returning references. You are free to use the object but do not own it and cannot store a reference to it beyond the lifetime of the object returning the reference.

Changing the quadTree to accept and return references instead of strong pointers would seem to violate these two rules. That's OK but requires an additional change. In your case quadTree is a member variable. If an exception occurs quadTree will still contain references to objects it does not own and may no longer exist. This can be easily rectified by using a quadTree local to the scope of the calling function. This will guarantee that the lifetime of quadTree is not longer than _gameObjects - the actual owner.

This simply expands the first rule to include the lifetime of the object the function belongs to. The changes might look something like this (using pointers rather than references which can be applied just the same).

void GameObjectManager::updateAll(float frametime)
{   
    checkAlive();

    quadTree tree;

    // iterate through the entityManager, updating each entity and then adding it into the quadTree.
    for (auto itr = _gameObjects.begin(); itr != _gameObjects.end(); itr++)
    {
        itr->second->Update(frametime);

        tree.AddObject(itr->second.get());      
    }   

    // iterate once again through the entityManager but this time checking itself against the quadTree.
    for (auto x = _gameObjects.begin(); x != _gameObjects.end(); x++)
    {
        std::vector<Entity*> returnObjects = tree->GetObjectsAt(x->second->GetPosition().x, x->second->GetPosition().y );       

        for ( int n = 0; n < (int)returnObjects.size(); n++ ) 
        {
            Entity* collided = returnObjects[n];

            if(object->getEntityName() != collided->getEntityName())
            {
                if(object->GetBoundingRect().intersects(collided->GetBoundingRect()))
                {
                    object->Touch(collided);
                }
            }
        }       
    }   
    //tree.Clear(); // Called by destructor.
}