Delete during iteration std :: vector (indirectly)

Question

This question has been asked several times, but mine is a slightly different case. Let's say I have std :: vector observers that I notify when a specific event occurs:

void SomeClass::doThing() {
    // do things ...
    // notify observers
    for (auto* o : mObservers) {
        o->thingHappened();
    }
}

What if in the implementation the thingHappenedobserver calls method in SomeClassto remove himself from the observers? What are some of the best ways to handle this?

One possibility is to make a copy mObserversbefore the for loop and use it instead, but an extra copy can be wasteful.

Another possibility is to delegate the changes to the array that will be launched after the loop finishes, possibly setting a lock (only logical) before the loop starts, and when this lock is set, methods that mutate the vector delegate themselves to be called after the loop is executed, when the lock is set to false (can be done with the lambdas vector ... rather cumbersome).

Answer 1

If you have control over the signature thingHappened(), you can change it to return boolto indicate whether to delete it. Then you can remove all the values ​​that return true(or false; depends on the semantics you want).

std::remove_if std::partition .

void SomeClass::doThing() {
    // do things ...
    // notify observers
    auto newEnd = std::remove_if(mObservers.begin(), mObservers.end(), [](auto *o) {
        return o->thingHappened();
    });
    // assuming mObservers is a vector
    mObservers.erase(newEnd, mObservers.end());
}

Answer 2

- . std::list // . , , , , .

for (auto it = the_list.begin(); it != the_list.end();)
{
    auto next = std::next(it);
    it->call_the_possibly_removing_function();
    it = next;
}

Answer 3

, thingHappened SomeClass, ? ?

.

• , .
• , , . , . , .
• , , , .
• , .

---

void SomeClass::removeObserver(Observer* o) {
   if ( this->isIterating  )
   {
      observersToRemove.push_back(o);
   }
   else
   {
      // Code for real removal of the observer
   }
}

void SomeClass::doThing() {
   this->isIterating = true;
   for (auto* o : mObservers) {
      o->thingHappened();
   }

   for ( auto* o : observersToRemove )
   {
      // Code for real removal of the observer
   }

   observersToRemove.clear();

   this->isIterating = false;
}

Answer 4

R Sahu . , , , . , .

, :

#include <functional>
#include <utility>
#include <vector>

// Note that this is not threadsafe
template <typename Type>
class MutableLock {
    bool locked = false;
    Type value;
    // std::function gives us a more general action,
    // but it does come at a cost; you might want to consider using
    // other techniques.
    std::vector<std::function<void(Type&)>> actions;

public:
    class AutoLocker {
        MutableLock& lock;

        friend class MutableLock<Type>;

        explicit AutoLocker(MutableLock& lock)
            : lock{ lock }
        {
        }

    public:
        ~AutoLocker()
        {
            lock.unlock();
        }
    };

    MutableLock() = default;

    // The [[nodiscard]] is a C++17 attribute that
    // would help enforce using this function appropriately
    [[nodiscard]] AutoLocker lock()
    {
        locked = true;
        return AutoLocker{ *this };
    }

    void unlock()
    {
        for (auto const& action : actions) {
            action(value);
        }
        actions.clear();

        locked = false;
    }

    template <typename F>
    void action(F&& f)
    {
        if (!locked) {
            f(value);
        } else {
            actions.emplace_back(std::forward<F>(f));
        }
    }

    // There needs to be some way to expose the value
    // not under the lock (so that we can use it when
    // we call `lock()`).
    //
    // Even if your `Type` is not a range, this would
    // be fine, as member functions of a template class
    // aren't instantiated unless you call them.
    //
    // However, you may want to expose other ways to
    // access the value
    auto begin() { return std::begin(value); }
    auto end() { return std::end(value); }
    auto begin() const { return std::begin(value); }
    auto end() const { return std::end(value); }
};

:

#include <algorithm>
#include <iostream>

class Observer {
public:
    virtual void thingHappened() = 0;

protected:
    ~Observer() = default;
};

class SomeClass {
    MutableLock<std::vector<Observer*>> observers;

public:
    void addObserver(Observer* observer)
    {
        observers.action([observer](auto& observers) {
            observers.push_back(observer);
        });
    }

    void remove(Observer const* observer)
    {
        observers.action([observer](auto& observers) {
            observers.erase(std::remove(observers.begin(), observers.end(), observer), observers.end());
        });
    }

    void doSomething()
    {
        auto lock = observers.lock();
        for (auto* observer : observers) {
            observer->thingHappened();
        }
        // when `lock` goes out of scope, we automatically unlock `observers` and
        // apply any actions that were built up
    }
};

class Observer1 : public Observer {
public:
    SomeClass* thing;

    void thingHappened() override
    {
        std::cout << "thing 1\n";
        thing->remove(this);
    }
};

int main()
{
    SomeClass thing;
    Observer1 obs;
    obs.thing = &thing;

    thing.addObserver(&obs);
    thing.doSomething();
    thing.doSomething();
}

^{On Coliru}