How to get rid of weak_ptrs in a container

Question

I have a class that stores weak_ptrs in a container and later does something if the weak_ptr is not expired:

class Example
{
public:
    void fill(std::shared_ptr<int> thing)
    {
        member.push_back(thing);
    }
    void dosomething() const
    {
        for (const auto& i : member)
            if (!i.expired())
                ;// do something. the weak_ptr will not be locked
    }
private:
    std::vector<std::weak_ptr<int>> member;
};

If Example is an object that lives forever and fill is used regularily, the vector allocates memory for elements continously, but they are never removed after they expired.

Is there any automatic C++ way to get rid of the expired weak_ptrs in the container or is there a better way to store a variable number of them?

My naive way would be to iterate over the container each time fill is called and remove all the expired weak_ptrs. In scenarios where Example has many elements in the container and fill is frequently called this seems to be very inefficient.

Answer 1

Does the shared_ptr<int> have to be a shared_ptr<int>?

How about a shared_ptr<IntWrapper>?

#include <iostream>
#include <forward_list>
using namespace std;

class IntWrapper {
public:
    int i;

    static forward_list<IntWrapper*>& all() {
        static forward_list<IntWrapper*> intWrappers;
        return intWrappers;
    }
    IntWrapper(int i) : i(i)  {
        all().push_front(this);
    }
    ~IntWrapper() {
        all().remove(this);
    }
};

void DoSomething() {
    for(auto iw : IntWrapper::all()) {
        cout << iw->i << endl;
    }
}

int main(int argc, char *argv[]) {
    shared_ptr<IntWrapper> a = make_shared<IntWrapper>(1);
    shared_ptr<IntWrapper> b = make_shared<IntWrapper>(2);
    shared_ptr<IntWrapper> c = make_shared<IntWrapper>(3);
    DoSomething();
    return 0;
}

Answer 2

I would rather use a custom deleter for the shared_ptr. But this implies here to change the interface of the Example class. The advantage using custom deleter is that there is no need to check for expired objects in the collection. The collection is directly maintained by the custom deleter.

Quick implementation :

#include <memory>
#include <iostream>
#include <set>

template <typename Container>
// requires Container to be an associative container type with key type
// a raw pointer type
class Deleter {
    Container* c;
public:
    Deleter(Container& c) : c(&c) {}
    using key_type = typename Container::key_type;
    void operator()(key_type ptr) {
        c->erase(ptr);
        delete ptr;
    }
};

class Example {
public:
    // cannot change the custom deleter of an existing shared_ptr
    // so i changed the interface here to take a unique_ptr instead
    std::shared_ptr<int> fill(std::unique_ptr<int> thing) {
        std::shared_ptr<int> managed_thing(thing.release(), Deleter<containter_type>(member));
        member.insert(managed_thing.get());
        return managed_thing;
    }

    void dosomething() const {
        // we don't need to check for expired pointers
        for (const auto & i : member)
            std::cout << *i << ", ";

        std::cout << std::endl;
    }

    using containter_type =  std::set<int*>;
private:
    containter_type member;
};

int main()
{
    Example example;
    auto one = example.fill(std::unique_ptr<int>(new int(1)));
    auto two = example.fill(std::unique_ptr<int>(new int(2)));
    auto three = example.fill(std::unique_ptr<int>(new int(3)));
    example.dosomething();
    three.reset();
    example.dosomething();
}

Answer 3

Since you clarified that you are actually using a std::map and not a std::vector, it might be easiest to remove the expired elements on-the-fly in doSomething(). Switch back from a range-based for loop to a normal iterator based design:

void dosomething() const
{
    auto i = member.begin();
    while( i != member.end() ) {
      if( i->expired() ) { i = member.erase( i ); continue; }
      ;// do something. the weak_ptr will not be locked
      ++i;
    }
}