140

In the following code I loop through a map and test if an element needs to be erased. Is it safe to erase the element and keep iterating or do I need to collect the keys in another container and do a second loop to call the erase()?

map<string, SerialdMsg::SerialFunction_t>::iterator pm_it; for (pm_it = port_map.begin(); pm_it != port_map.end(); pm_it++) { if (pm_it->second == delete_this_id) { port_map.erase(pm_it->first); } }

UPDATE: Of course, I then read this question which I didn't think would be related but answers my question.

Improve this question
1
  • Please note in question that std::remove_if does not work with std:map Commented Nov 4, 2015 at 13:16

3 Answers 3

Reset to default
190

C++11

This has been fixed in C++11 (or erase has been improved/made consistent across all container types).
The erase method now returns the next iterator.

auto pm_it = port_map.begin(); while(pm_it != port_map.end()) { if (pm_it->second == delete_this_id) { pm_it = port_map.erase(pm_it); } else { ++pm_it; } }

C++03

Erasing elements in a map does not invalidate any iterators.
(apart from iterators on the element that was deleted)

Actually inserting or deleting does not invalidate any of the iterators:

Also see this answer:
Mark Ransom Technique

But you do need to update your code:
In your code you increment pm_it after calling erase. At this point it is too late and is already invalidated.

map<string, SerialdMsg::SerialFunction_t>::iterator pm_it = port_map.begin(); while(pm_it != port_map.end()) { if (pm_it->second == delete_this_id) { port_map.erase(pm_it++); // Use iterator. // Note the post increment. // Increments the iterator but returns the // original value for use by erase } else { ++pm_it; // Can use pre-increment in this case // To make sure you have the efficient version } }
Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

Is the order of evaluation of the increment in the postfix expression pm_it++ guaranteed to be executed before the function is entered?
@David Rodríguez - dribeas: Yes. The standard guarantees that all argument expressions will be fully evaluated before the function is called. It is the result of the post increment that is passed to the erase function(). So yes the post increment of pm_it will be done before erase() is called.
NOTE: Almost line for line matches the associative container example in Scott Meyer's "Effective STL" Item 9.
for (auto pm_t = port_map.begin(); pm_it != port_map.end(); ) { ... }
@iboisver: On the vector. The use of erase() invalidates all iterators of the array after the erase point (not just the end), this is a property of Sequence containers. The special property of Associative containers is that iterators are not invalidated by erase or insert (unless they point at element that was erased). Vector and erase usign iterators is covered in detail in the appropriate question stackoverflow.com/a/3938847/14065
|
12

Here's how I do that ...

typedef map<string, string> StringsMap; typedef StringsMap::iterator StrinsMapIterator; StringsMap m_TheMap; // Your map, fill it up with data bool IsTheOneToDelete(string str) { return true; // Add your deletion criteria logic here } void SelectiveDelete() { StringsMapIter itBegin = m_TheMap.begin(); StringsMapIter itEnd = m_TheMap.end(); StringsMapIter itTemp; while (itBegin != itEnd) { if (IsTheOneToDelete(itBegin->second)) // Criteria checking here { itTemp = itBegin; // Keep a reference to the iter ++itBegin; // Advance in the map m_TheMap.erase(itTemp); // Erase it !!! } else ++itBegin; // Just move on ... } }
Improve this answer

1 Comment

If you delete the end of the vector as well (itEnd), then the last check (the while condition) will be against an invalidated iterator (itEnd). Not good.
1

This is how I would do it, approximately:

bool is_remove( pair<string, SerialdMsg::SerialFunction_t> val ) { return val.second == delete_this_id; } map<string, SerialdMsg::SerialFunction_t>::iterator new_end = remove_if (port_map.begin( ), port_map.end( ), is_remove ); port_map.erase (new_end, port_map.end( ) );

There is something odd about 

val.second == delete_this_id

but I just copied it from your example code.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.