264

How do I remove from a map while iterating it? like:

std::map<K, V> map;
for(auto i : map)
    if(needs_removing(i))
        // remove it from the map

If I use map.erase it will invalidate the iterators

stanwise
  • 2,392
  • 1
  • 15
  • 21
Daniel
  • 30,896
  • 18
  • 85
  • 139
  • 1
    Similar: http://stackoverflow.com/questions/1038708/erase-remove-contents-from-the-map-or-any-other-stl-container-while-iterating – Christian Ammer Nov 22 '11 at 22:37
  • 1
    Even more similar: http://stackoverflow.com/questions/800955/remove-if-equivalent-for-stdmap – Kleist Nov 22 '11 at 22:47
  • 1
    And: http://stackoverflow.com/questions/180516/how-to-filter-items-from-a-stdmap – Christian Ammer Nov 22 '11 at 22:54
  • 1
    On the same topic: http://stackoverflow.com/questions/263945/what-happens-if-you-call-erase-on-a-map-element-while-iterating-from-begin-to – Agostino Mar 01 '15 at 02:15

6 Answers6

383

The standard associative-container erase idiom:

for (auto it = m.cbegin(); it != m.cend() /* not hoisted */; /* no increment */)
{
  if (must_delete)
  {
    m.erase(it++);    // or "it = m.erase(it)" since C++11
  }
  else
  {
    ++it;
  }
}

Note that we really want an ordinary for loop here, since we are modifying the container itself. The range-based loop should be strictly reserved for situations where we only care about the elements. The syntax for the RBFL makes this clear by not even exposing the container inside the loop body.

Edit. Pre-C++11, you could not erase const-iterators. There you would have to say:

for (std::map<K,V>::iterator it = m.begin(); it != m.end(); ) { /* ... */ }

Erasing an element from a container is not at odds with constness of the element. By analogy, it has always been perfectly legitimate to delete p where p is a pointer-to-constant. Constness does not constrain lifetime; const values in C++ can still stop existing.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • 1
    "not even exposing the container inside the loop body" what do you mean? – Daniel Nov 22 '11 at 22:34
  • 2
    @Dani: Well, contrast this to the 20th-century construction `for (int i = 0; i < v.size(); i++)`. Here we have to say `v[i]` inside the loop, i.e. we must explicitly mention the container. The RBFL on the other hand introduces the loop variable that is directly usable as the value, and so no knowledge of the container is required inside the loop. This is a clue to the intended usage of the RBFL for loops that do *not* have to know about the container. Erasing is the complete opposite situation, where it's all about the container. – Kerrek SB Nov 22 '11 at 22:41
  • 1
    @skyhisi: Just saw myself that and fixed it, thanks! I had it mixed up in my mind with the sequence-erase idiom where you say `it = v.erase(it);` (no `++`). It'll never happen again, promise. – Kerrek SB Nov 22 '11 at 22:43
  • @MooingDuck the [map (C++) Wikipedia page](http://en.wikipedia.org/wiki/Map_%28C%2B%2B%29) states: "Iterators and references are not invalidated by insert and erase operations, except for iterators and references to erased elements." – Silas Parker Nov 22 '11 at 22:50
  • 4
    @skyhisi: Indeed. This is one of the legitimate uses of the post-increment: *First* increment `it` to get the next, valid iterator, and *then* erase the old one. It doesn't work the other way round! – Kerrek SB Nov 22 '11 at 22:55
  • 1
    @skyhisi: I was thinking that the erase/rebalance might cause it to skip a node, but then realized it always iterates in order, so even a rebalance will not cause it to magically skip one. This algorithm is safe. – Mooing Duck Nov 22 '11 at 22:56
  • can't this just be a while instead of a for? – paulm Apr 05 '14 at 00:17
  • 7
    I read somewhere that in C++11, `it = v.erase(it);` now works for maps too.That is, erase() on *all* associative elements now returns the next iterator. So the old kludge that required a post-increment++ within the delete(), is no longer needed. This (if true) is a Good Thing, as the kludge relied on overridden-post-increment-within-a-function-call magic, "fixed" by newbie maintainers to take the increment out of the function call, or to swap it to a preincrement "because that's just a style thing", etc. – Dewi Morgan Jan 29 '15 at 23:11
  • 7
    why would you call `it++` in the `if` _and_ `else` blocks? wouldn't it be enough to call it once _after_ these? – nburk May 14 '15 at 15:21
  • 1
    Code does not work. The erase line should be: `it = m.erase(it);`. The proposed line causes an infinite loop. – Paulo Carvalho Jul 05 '16 at 18:07
  • @PauloCarvalho: Hm, the code [works for me](http://ideone.com/Leyjif)... – Kerrek SB Jul 05 '16 at 18:46
  • Use with care:http://stackoverflow.com/questions/38965452/crash-in-c-stdmap/39011211 – Stepan Yakovenko Aug 18 '16 at 06:24
  • @StepanYakovenko: That code looks... somewhat questionable. I don't think that's related to this answer. – Kerrek SB Aug 18 '16 at 08:36
  • Why not hoist the `end()` iterator here? There can't be no invalidation, can there? – TemplateRex Dec 29 '16 at 15:25
  • 1
    @TemplateRex: Good question, I'm not actually sure. I think I just wrote the code for both associative and sequence containers and didn't adapt it to the additional guarantees for associative containers. (But I'd have to check those.) – Kerrek SB Dec 29 '16 at 15:31
  • Just got the question from this [linked Q&A](http://stackoverflow.com/questions/41381183/is-it-good-practise-to-increment-or-decrement-the-loop-counter-inside-a-for-loop?noredirect=1&lq=1) – TemplateRex Dec 29 '16 at 15:34
  • `m.erase(it++);` -- doesn't this invalidate the iterator before it gets incremented? It may work in practice, due to the particular implementation of the map, but it doesn't seem kosher according to the docs. For that reason, I prefer the second answer by @d-coetzee – Ewat Nov 15 '18 at 23:51
  • 2
    @Ewat: No, that's fine.It's the *erasing* that invalidates the erased iterator; the increment happens before the erasing. This is a poster-child example of the use of post-increment. – Kerrek SB Nov 16 '18 at 01:35
  • @nburk And by that token itsn't it equivalent to just use the normal three-element for loop pattern where the increment happens in the third statement? https://ideone.com/W36yag – ACK_stoverflow Sep 04 '20 at 15:49
  • `it = m.erase(it);` is valid for all containers (now, although it wasn't always in the past). `m.erase(it++);` is valid **only** for containers that guarantee that only the erased iterator and no other iterators are invalidated on erase operations (which includes std::map but not std::vector; ymmv for other containers). So the posted code is correct for std::map and should be correct for _most_ associative containers, but not necessarily all (e.g. it's very likely to be wrong for a flat_map). To be safe you should use the variant in the comment unless you can't use C++11. – Miral Oct 06 '21 at 02:05
  • What about during reverse iteration, there is a compiler error? Edit: nvm, https://stackoverflow.com/questions/1830158/how-to-call-erase-with-a-reverse-iterator – Dominic Feb 14 '22 at 20:39
  • @ACK_stoverflow: Change `it->second == "bar"` to `it->second == "foo"` and you [get a segfault](https://ideone.com/QMXRGr ). The iterator pointing to the erased item is invalided by `erase()`, so you need to do `++it` or `it = m.erase(it)`. – idbrii Dec 05 '22 at 20:01
61

I personally prefer this pattern which is slightly clearer and simpler, at the expense of an extra variable:

for (auto it = m.cbegin(), next_it = it; it != m.cend(); it = next_it)
{
  ++next_it;
  if (must_delete)
  {
    m.erase(it);
  }
}

Advantages of this approach:

  • the for loop incrementor makes sense as an incrementor;
  • the erase operation is a simple erase, rather than being mixed in with increment logic;
  • after the first line of the loop body, the meaning of it and next_it remain fixed throughout the iteration, allowing you to easily add additional statements referring to them without headscratching over whether they will work as intended (except of course that you cannot use it after erasing it).
cdgraham
  • 380
  • 1
  • 10
D Coetzee
  • 789
  • 5
  • 5
  • 4
    I can think of another advantage actually, if the loop calls into code which erases that entry being iterated over or previous ones (and the loop is unaware of it) it will work without any harm done. The only restriction is whether something is erasing what is being pointed to by next_it or successors. A totally cleared list/map can be tested against as well. – Larswad Nov 21 '17 at 15:13
  • This answer is simple and clear, even if the loop is more complex and has multiple levels of logic to decide whether or not to delete, or do other various tasks. I've proposed an edit, though, to make it a little simpler. "next_it" can be set to "it" in the for's init to avoid typos, and since the the init and iteration statements both set it and next_it to the same values, you don't need to say "next_it = it;" at the start of the loop. – cdgraham Nov 07 '18 at 19:06
  • 3
    Keep in mind anyone who uses this answer: You must have "++next_it" inside the for loop, and not in the iteration expression. If you try to move it into the iteration expression as "it = next_it++", then on the last iteration, when "it" is going to be set equal to "m.cend()", you will attempt to iterate "next_it" past "m.cend()", which is erroneous. – cdgraham Nov 07 '18 at 19:14
  • 1
    I see all the advantages as disadvantages. – Lukas Salich Dec 19 '21 at 05:07
12

C++20 has a convenience overload of std::erase_if for std::map.

So you can use that function to do it as a one-liner.

std::map<K, V> map_obj;

// calls needs_removing for each element and erases it, if true was returned
std::erase_if(map_obj, needs_removing);

// if you need to pass only part of the key/value pair
std::erase_if(map_obj, [] (auto& kv) { return needs_removing(kv.first); });
rustyx
  • 80,671
  • 25
  • 200
  • 267
PeterT
  • 7,981
  • 1
  • 26
  • 34
11

Assuming C++11, here is a one-liner loop body, if this is consistent with your programming style:

using Map = std::map<K,V>;
Map map;

// Erase members that satisfy needs_removing(itr)
for (Map::const_iterator itr = map.cbegin() ; itr != map.cend() ; )
  itr = needs_removing(itr) ? map.erase(itr) : std::next(itr);

A couple of other minor style changes:

  • Show declared type (Map::const_iterator) when possible/convenient, over using auto.
  • Use using for template types, to make ancillary types (Map::const_iterator) easier to read/maintain.
Matt
  • 20,108
  • 1
  • 57
  • 70
  • is it possible that this causes a memory leak? It does in my case. But really not sure why. – Khamyl Apr 19 '22 at 15:38
  • Not any more than the other answers. I'd look for the memory leak in or involving the destructor(s) for the `std::map` key and/or value types. – Matt Apr 19 '22 at 16:01
8

In short "How do I remove from a map while iterating it?"

  • With old map impl: You can't
  • With new map impl: almost as @KerrekSB suggested. But there are some syntax issues in what he posted.

From GCC map impl (note GXX_EXPERIMENTAL_CXX0X):

#ifdef __GXX_EXPERIMENTAL_CXX0X__
      // _GLIBCXX_RESOLVE_LIB_DEFECTS
      // DR 130. Associative erase should return an iterator.
      /**
       *  @brief Erases an element from a %map.
       *  @param  position  An iterator pointing to the element to be erased.
       *  @return An iterator pointing to the element immediately following
       *          @a position prior to the element being erased. If no such 
       *          element exists, end() is returned.
       *
       *  This function erases an element, pointed to by the given
       *  iterator, from a %map.  Note that this function only erases
       *  the element, and that if the element is itself a pointer,
       *  the pointed-to memory is not touched in any way.  Managing
       *  the pointer is the user's responsibility.
       */
      iterator
      erase(iterator __position)
      { return _M_t.erase(__position); }
#else
      /**
       *  @brief Erases an element from a %map.
       *  @param  position  An iterator pointing to the element to be erased.
       *
       *  This function erases an element, pointed to by the given
       *  iterator, from a %map.  Note that this function only erases
       *  the element, and that if the element is itself a pointer,
       *  the pointed-to memory is not touched in any way.  Managing
       *  the pointer is the user's responsibility.
       */
      void
      erase(iterator __position)
      { _M_t.erase(__position); }
#endif

Example with old and new style:

#include <iostream>
#include <map>
#include <vector>
#include <algorithm>

using namespace std;
typedef map<int, int> t_myMap;
typedef vector<t_myMap::key_type>  t_myVec;

int main() {

    cout << "main() ENTRY" << endl;

    t_myMap mi;
    mi.insert(t_myMap::value_type(1,1));
    mi.insert(t_myMap::value_type(2,1));
    mi.insert(t_myMap::value_type(3,1));
    mi.insert(t_myMap::value_type(4,1));
    mi.insert(t_myMap::value_type(5,1));
    mi.insert(t_myMap::value_type(6,1));

    cout << "Init" << endl;
    for(t_myMap::const_iterator i = mi.begin(); i != mi.end(); i++)
        cout << '\t' << i->first << '-' << i->second << endl;

    t_myVec markedForDeath;

    for (t_myMap::const_iterator it = mi.begin(); it != mi.end() ; it++)
        if (it->first > 2 && it->first < 5)
            markedForDeath.push_back(it->first);

    for(size_t i = 0; i < markedForDeath.size(); i++)
        // old erase, returns void...
        mi.erase(markedForDeath[i]);

    cout << "after old style erase of 3 & 4.." << endl;
    for(t_myMap::const_iterator i = mi.begin(); i != mi.end(); i++)
        cout << '\t' << i->first << '-' << i->second << endl;

    for (auto it = mi.begin(); it != mi.end(); ) {
        if (it->first == 5)
            // new erase() that returns iter..
            it = mi.erase(it);
        else
            ++it;
    }

    cout << "after new style erase of 5" << endl;
    // new cend/cbegin and lambda..
    for_each(mi.cbegin(), mi.cend(), [](t_myMap::const_reference it){cout << '\t' << it.first << '-' << it.second << endl;});

    return 0;
}

prints:

main() ENTRY
Init
        1-1
        2-1
        3-1
        4-1
        5-1
        6-1
after old style erase of 3 & 4..
        1-1
        2-1
        5-1
        6-1
after new style erase of 5
        1-1
        2-1
        6-1

Process returned 0 (0x0)   execution time : 0.021 s
Press any key to continue.
Kashyap
  • 15,354
  • 13
  • 64
  • 103
  • 1
    I don't get it. What is the problem with `mi.erase(it++);` ? – lvella Jan 28 '18 at 19:08
  • 1
    @lvella see op. "If I use map.erase it will invalidate the iterators". – Kashyap Jan 29 '18 at 06:42
  • 1
    Your new method will not work if after erasing, the map becomes empty. In that case, the iterator will be invalidated. So, soon after erasing, its better to insert `if(mi.empty()) break;`. – Rahat Zaman Jun 10 '20 at 08:28
5

Pretty sad, eh? The way I usually do it is build up a container of iterators instead of deleting during traversal. Then loop through the container and use map.erase()

std::map<K,V> map;
std::list< std::map<K,V>::iterator > iteratorList;

for(auto i : map ){
    if ( needs_removing(i)){
        iteratorList.push_back(i);
    }
}
for(auto i : iteratorList){
    map.erase(*i)
}
Michael Daum
  • 820
  • 6
  • 12