What happens if you call erase() on a map element while iterating from begin to end?

C++StlIterator

C++ Problem Overview


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.

C++ Solutions


Solution 1 - C++

##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
    }
}

Solution 2 - C++

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 ...
     }
}

Solution 3 - C++

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.

Attributions

All content for this solution is sourced from the original question on Stackoverflow.

The content on this page is licensed under the Attribution-ShareAlike 4.0 International (CC BY-SA 4.0) license.

Content TypeOriginal AuthorOriginal Content on Stackoverflow
QuestionMatthew SmithView Question on Stackoverflow
Solution 1 - C++Martin YorkView Answer on Stackoverflow
Solution 2 - C++AlaaShakerView Answer on Stackoverflow
Solution 3 - C++ravenspointView Answer on Stackoverflow