Reputation: 5221
I have a std::multimap
that maps session IDs (int
s) to the pieces of hardware used in that session (the hardware is described by a struct
that contains some hardware specific info).
I have a cleanup function that has to do specific cleanup stuff for each piece of hardware. After the cleanup is done, I need to erase that element from the map, since the hardware is no longer used in that session.
Note that I don't just want to remove a single piece of hardware from a session. Instead the whole session gets torn down, so I want to search the map for the session ID, cleanup the hardware, and remove all those entries from the map.
Here's some code which shows what I am trying to explain:
void MyClass::end_session(const int session_id) {
// session_map_ is a member variable of MyClass
const auto range = session_map_.equal_range(session_id);
for (auto it = range.first; it != range.second; session_map_.erase(it++)) {
// do cleanup for the hardware pointed to by it->second
}
}
Is the loop legal? I know that the iterator passed in to erase()
gets invalidated, but this doesn't invalidate range.first
or range.second
, correct? Also, does session_map_.erase(it++)
work like I would expect it to? That is, I'm assuming that it
gets saved as the argument for erase()
, gets incremented to its new value, and then erase()
is called for the old value (thus invalidating the iterator before the increment). Is that correct?
Upvotes: 2
Views: 582
Reputation: 275966
What you are doing is perfectly safe.
I, however, am leery of complex control flow in business logic. It is too hard to understand at a glance, and comments have a risk of becoming out of sync.
As such I write verbs.
template<class C, class It, class F>
void erase_remove_if( C& c, It b, It e, F&& f) {
auto it = b;
while (it != e) {
if (f(*it))
it = c.erase(it);
else
++it
}
}
template<class C, class F>
void erase_remove_if( C& c, F&& f) {
using std::begin; using std::end;
return erase_remove_if( c, begin(c), end(c), std::forward<F>(f) );
}
now your code reads like:
void MyClass::end_session(const int session_id) {
// session_map_ is a member variable of MyClass
const auto range = session_map_.equal_range(session_id);
// remove said elements after doing some processing:
erase_remove_if( session_map_, range.first, range.second, [&](auto& kv) {
auto&& [key, elem] = kv;
// do cleanup for the hardware pointed to by elem
return true; // erase element
});
}
basically fenceposting errors are so common and hard to prevent that I'd rather have my relatively complex loop control code in one spot.
Upvotes: 1
Reputation: 12831
The documentation says
Other references and iterators are not affected
So your loop runs as expected and erase
works as expected.
Suppose you have 4 elements, in your case the loop runs 4 times without any problem.
https://en.cppreference.com/w/cpp/container/multimap/erase
Upvotes: 2