Do I need to reset a shared_ptr before removing it from a vector?

Question

I've written a very simple C++ program using std::shared_ptr.

Here's the code :

/*
** Resource class definition
*/
class Resource
{
    public:
        std::string m_Name;
        Resource(void){}
        Resource(std::string name)
            :   m_Name(name)
        {

        }
        std::string const &GetName(void) const
        {
            return (this->m_Name);
        }
};

namespace Predicate
{
    /*
    ** Predicate - Delete a specific node according to its name
    */
    template <typename T>
    struct DeleteByName
    {
        DeleteByName(std::string const &name);
        bool operator()(T &pData);
        std::string m_Name;
    };

    //Initialization

    template <typename T>
    DeleteByName<T>::DeleteByName(std::string const &name)
        :   m_Name(name)
    {

    }

    //Surcharges

    template <typename T>
    bool DeleteByName<T>::operator()(T &pData)
    {
        if (pData->GetName() == this->m_Name)
        {
            pData.reset();
            return (true);
        }
        return (false);
    }
}

/*
** Remove a specific node according to its name - WORKS
*/
static void RemoveByName__CLASSIC__OK(std::string const &name, std::vector<std::shared_ptr<Resource>> &resourceList)
{
    std::vector<std::shared_ptr<Resource>>::iterator It = resourceList.begin();
    std::vector<std::shared_ptr<Resource>>::iterator It_dest;

    for (; It != resourceList.end(); ++It) {
        if (!(*It)->GetName().compare(name))
        {
            It_dest = It;
        }
    }
    It_dest->reset();
    resourceList.erase(It_dest);
}

/*
** Remove a specific node according to its name - NOT WORK
*/
static void RemoveByName__CLASSIC__NOT_OK(std::string const &name, std::vector<std::shared_ptr<Resource>> &resourceList)
{
    std::vector<std::shared_ptr<Resource>>::iterator It = resourceList.begin();

    for (; It != resourceList.end(); ++It) {
        if (!(*It)->GetName().compare(name))
        {
            It->reset();
            resourceList.erase(It);
        }
    }
}

static std::vector<std::shared_ptr<Resource>>::const_iterator FindByName__PREDICATE__OK(
    std::string const &name, std::vector<std::shared_ptr<Resource>> &resourceList)
{
    return (std::find_if(resourceList.begin(),
            resourceList.end(), Predicate::FindByName<std::shared_ptr<Resource>>(name)));
}

/*
** Remove a specific node according to its name using std::remove_if algorithm with the predicate 'DeleteByName' - WORKS
*/
static void RemoveByName__PREDICATE__OK(std::string const &name, std::vector<std::shared_ptr<Resource>> &resourceList)
{
    if (FindByName__PREDICATE__OK(name, resourceList) != resourceList.end())
        resourceList.erase(std::remove_if(
            resourceList.begin(), resourceList.end(), Predicate::DeleteByName<std::shared_ptr<Resource>>(name)));
}

/*
** Entry point
*/
int main(void)
{
    std::vector<std::shared_ptr<Resource>> resourceList;

    std::shared_ptr<Resource> rsc_A(new Resource("resource_a"));
    std::shared_ptr<Resource> rsc_B(new Resource("resource_b"));
    std::shared_ptr<Resource> rsc_C(new Resource("resource_c"));

    resourceList.push_back(rsc_A);
    resourceList.push_back(rsc_B);
    resourceList.push_back(rsc_C);

    PrintResourceList(resourceList);

    RemoveByName__PREDICATE__OK("resource_as", resourceList);

    PrintResourceList(resourceList);

    getchar();
    return (0);
}

I just want to know if I erase a node from a std::vector containing a shared pointer, if I have to call the method 'reset' to destroy the shared pointer before calling the 'erase' method. I think if I just destroy the node without any call of the function 'reset' the shared pointer should be still destroyed. Is that right?

Plus, I don't understand why the function 'RemoveByName__CLASSIC__NOT_OK' fails. I don't understand why I have to declare an 'It_dest' to store the iterator during the loop (see the method 'RemoveByName__CLASSIC__OK') and finally erase the node at the end of the function. This problem just happened using shared pointer. Does any one have an idea?

Answer 1

You don't have to reset the shared_ptr manually, that is done in the destructor. When you erase it, the object gets destroyed and thus decreases the reference count.

Your RemoveByName__CLASSIC__NOT_OK function failes because you are using the iterator after erasing the pointed element. After std::vector::erase, the iterator will be invalid and cannot be used anymore. erase returns the next iterator.

static void RemoveByName__CLASSIC__NOT_OK(std::string const &name, std::vector<std::shared_ptr<Resource>> &resourceList)
{
    for (auto It = resourceList.begin(); 
         It != resourceList.end(); ) {
        if (!(*It)->GetName().compare(name))
        {
            It = resourceList.erase(It);
        }
        else
        {
            ++It;
        }
    }
}

I consider the implementation with remove_if more readable.