In C ++, is there an idiomatic way to protect against a situation in which starting a set of actions leads to a mutation of a collection?

Question

Say you have a class foo that wraps a collection of some called objects. foo has a member function run() that iterates through the collection and calls each object of the function. foo also has a remove(...) member that will remove the called object from the collection.

Is there an idiom-style RAII protector that you can put in foo.run() and foo.remove(...) so that the deleted objects are called by calling foo.run() will be delayed until security destructor? Can I do something in the standard library? Does this template have a name?

My current code seems inelegant, so I'm looking for a solution with best practice.

Note: this is not about concurrency. An unsafe solution is excellent. The problem is reinstallation and self-promotion.

Here is an example of a problem, without the inelegant defender of deferring removal.

 class ActionPlayer { private: std::vector<std::pair<int, std::function<void()>>> actions_; public: void addAction(int id, const std::function<void()>& action) { actions_.push_back({ id, action }); } void removeAction(int id) { actions_.erase( std::remove_if( actions_.begin(), actions_.end(), [id](auto& p) { return p.first == id; } ), actions_.end() ); } void run() { for (auto& item : actions_) { item.second(); } } }; 

and then in another place:

 ... ActionPlayer player; player.addAction(1, []() { std::cout << "Hello there" << std::endl; }); player.addAction(42, [&player]() { std::cout << "foobar" << std::endl; player.removeAction(1); }); player.run(); // boom 

Edit ... ok, here's how I can do it using the RAII lock object. The following should handle calls that cause calls and callbacks to run as part of execution, assuming the recursion ends (unless it is a user error). I used cached std :: functions, because in the real version of this code, the equivalent of addAction and removeAction are template functions that cannot be stored in a uniformly typed vanilla container.

 class ActionPlayer { private: std::vector<std::pair<int, std::function<void()>>> actions_; int run_lock_count_; std::vector<std::function<void()>> deferred_ops_; class RunLock { private: ActionPlayer* parent_; public: RunLock(ActionPlayer* parent) : parent_(parent) { (parent_->run_lock_count_)++; } ~RunLock() { if (--parent_->run_lock_count_ == 0) { while (!parent_->deferred_ops_.empty()) { auto do_deferred_op = parent_->deferred_ops_.back(); parent_->deferred_ops_.pop_back(); do_deferred_op(); } } } }; bool isFiring() const { return run_lock_count_ > 0; } public: ActionPlayer() : run_lock_count_(0) { } void addAction(int id, const std::function<void()>& action) { if (!isFiring()) { actions_.push_back({ id, action }); } else { deferred_ops_.push_back( [&]() { addAction(id, action); } ); } } void removeAction(int id) { if (!isFiring()) { actions_.erase( std::remove_if( actions_.begin(), actions_.end(), [id](auto& p) { return p.first == id; } ), actions_.end() ); } else { deferred_ops_.push_back( [&]() { removeAction(id); } ); } } void run() { RunLock lock(this); for (auto& item : actions_) { item.second(); } } }; 

Answer 1

The usual way is to create a copy of vector . But this can lead to a restart of remote actions.

 void run() { auto actions_copy{actions_}; for (auto& item : actions_copy) { item.second(); } } 

Other options if remote actions are not allowed

• Add bool to save if any action is deleted
• Use general / weak ptr
• use std::list if it is known that the current action will not be deleted.

Answer 2

Add a run flag that says you are listing actions_ . Then, if removeAction is called with the flag set, you save the id in the vector for later removal. You may also need a separate vector to store the actions that are added in the enumeration. After you are done with actions_ , you will remove the ones you want to remove and add the ones that were added.

Something like

 // within class ActionPlayer, add these private member variables private: bool running = false; std::vector<int> idsToDelete; public: void run() { running = true; for (auto& item : actions_) { item.second(); } running = false; for (d: idsToDelete) removeAction(d); idsToDelete.clear(); } // ... 

You can make a similar change for deferred addAction calls (what you will need to do if any of the actions can add an action, since adding can cause the vector to allocate more memory will invalidate all iterators by vector).

Answer 3

I would change the structure a bit. Instead of allowing a direct change to the ActionPlayer , I would force all changes through an external modifier class. In this example, I made it an abstract modifier class that can have different specific implementations (e.g. DeferredModifier , InstantModifier , NullModifier , LoggedModifier , TestModifier .etc.). Now your actions only need a reference to the abstract base class of the modifier and a call to any add / remove.etc. function as needed. This allows you to share the modification policy with the implementation of the action and introduce various change policies into the actions.

This should also make it possible to simplify support for simultaneous modification, since you no longer need to switch the run / not running state to postpone changes.

This example shows a simple way to play back actions in order (this is the property that I assume you want to keep). A more advanced implementation can scan the list of changes backwards by removing all add / remove pairs, and then group the modifications / deletes to minimize copying when changing the list of actions.

Something like:

 class ActionPlayer { friend class Modifier; ... void run(Modifier &modifier) { } private: void addAction(...) { ... } void removeAction(...) { ... } } class Modifier { public: virtual ~Modifier() {} virtual addAction(...) = 0; virtual removeAction(...) = 0; } class DelayedModifier : public Modifier { struct Modification { virtual void run(ActionPlayer&) = 0; } struct RemoveAction : public Modification { int id; Removal(int _id) : id(_id) {} virtual void run(ActionPlayer &player) { player.removeAction(id); } } struct AddAction : public Modification { int id; std::function<void(Modifier&)>> action; AddAction(int _id, const std::function<void(Modifier&)> &_action) : id(_id), action(_action) {} virtual void run(ActionPlayer &player) { player.addAction(id, action) }; } ActionPlayer &player; std::vector<Modification> modifications; public: DelayedModifier(ActionPlayer &_player) player(_player) {} virtual ~DelayedModifier() { run(); } virtual void addAction(int id, const std::function<void(Modifier&)> &action) { modifications.push_back(AddAction(id, action)); } virtual void removeAction(int id) { modifications.push_back(RemoveAction(id)); } void run() { for (auto &modification : modifications) modification.run(player); modifications.clear(); } }; 

So now you write:

 ActionPlayer player; { DelayedModifier modifier(player); modifier.addAction(...); modifier.addAction(...); modifier.run(); actions.run(modifier); }