SPyofgame's blog

By SPyofgame, history, 5 years ago, In English

Recently while I am solving a problem, I realised that increase iterator after erasing the last element of the map (the map is empty) then it would run infinitively

So I stop the loop by adding a small line to check whether it is empty or not

/// map-looping using either iterator or reverse iterator
{
    /// some code upper that erase the element but possible to make the map empty
    if (map.empty()) break;
}

Is there an alternative way (some other implementations) to prevent from iterator increament when the map is empty (and reverse_iterator too if possible)

  • Vote: I like it
  • -5
  • Vote: I do not like it

| Write comment?
»
5 years ago, # |
Rev. 2   Vote: I like it 0 Vote: I do not like it

Not sure but this seems to be working. Also, I don't see much of a point in deleting things from a map.

#include <iostream>
#include <map>
using namespace std;

int main(){
   map<int,int> cnt;
   cnt[1]++;
   cnt[2]++;
   cnt[4]++;

   for (auto it = cnt.begin(); it != cnt.end(); it++){
      if (it->first == 4) {
         cnt.erase(it++);
         if (it == cnt.end()) break;
      }
      cout << it->first << ' ';
   }
}
  • »
    »
    5 years ago, # ^ |
    Rev. 2   Vote: I like it 0 Vote: I do not like it

    I think this is not the correct way to do it. Because docu says "Iterator validity Iterators, pointers and references referring to elements removed by the function are invalidated. All other iterators, pointers and references keep their validity."

    So it++ increments an invalidated iterator. More correct would be to use a second variable created prior to the erase() call: auto it2=it+1; cnt.erase(it);

    However, I assume with most implementations both would work without difference.

    Edit: And just noticed that in C++11 and up, erase returns an iterator to the next position. So, correct would be: it=cnt.erase(it);