Question

I am maintaining a set of iterators of a multiset container in a separate data structure. After a while I pick one iterator from this data structure and then erase the associated element to that iterator from multiset. I use this some thing like this first :

#include <iostream>
#include <set>

int main ()
{
  std::multiset<int> myints;
  std::cout << "0. size: " << myints.size() << '\n';

  for (int i=0; i<10; i++) myints.insert(i);
  std::cout << "1. size: " << myints.size() << '\n';

  myints.insert (5);
  std::cout << "2. size: " << myints.size() << '\n';

  std::multiset<int>::iterator it = myints.find(5);
  myints.erase (it);
  std::cout << "3. size: " << myints.size() << '\n';
  myints.erase (it);
  std::cout << "4. size: " << myints.size() << '\n';
  return 0;
}

However, it turns out that second myints.erase (it); cause segmentation fault. Therefore, I change to following code and it works. I was wondering if this is good way to go or it is workable undefined situation:

int main ()
{
  std::multiset<int> myints;
  std::cout << "0. size: " << myints.size() << '\n';

  for (int i=0; i<10; i++) myints.insert(i);
  std::cout << "1. size: " << myints.size() << '\n';

  myints.insert (5);
  std::cout << "2. size: " << myints.size() << '\n';

  std::multiset<int>::iterator it = myints.find(5);
  myints.erase (it);
  std::cout << "3. size: " << myints.size() << '\n';

  std::multiset<int>::iterator newit = myints.find(*it);
  myints.erase (newit);
  std::cout << "4. size: " << myints.size() << '\n';

  return 0;
}
Was it helpful?

Solution

erase(it) invalidates the iterator it, i.e. it is useless after the erase and doing anything with it results in undefined behaviour. (You were probably expecting it to 'move to the next element' when the element it's pointing to is erased, but that's not what it does.)

Your second approach doesn't fix this. It may work by chance, but you're still reusing it after you have erased it.


edit: Given your description "I want to erase only one 5 from multiset and maintain the it valid after erase for next erase.", you can do that by creating a copy of the iterator, incrementing the original and then erasing through the copy:

it = myints.find(5);
// better add a check here to make sure there actually is a 5 ...
std::multiset<int>::iterator newit = it;
it++;
myints.erase(newit);

Since you have already incremented it, it remains valid because it does not point to the element that is killed by erase.

However, I honestly cannot imagine a situation in which this might actually be useful, or rather, required.

OTHER TIPS

In your first approach iterator is getting invalidated as you removed an element pointed by that iterator and later on when you try to erase again using the same iterator you will get segmentation fault.
In your second approach since every time you are doing find after erase which gives you correct iterator.

You can fix first case by making following change in your code. Post increment operator will return a new object and will move the iterator to next position. I will also suggest to do an end check before erasing otherwise you can get undefined behavior.

      std::multiset<int>::iterator it = myints.find(5);
      if(it != myints.end())
      myints.erase (it++);
      std::cout << "3. size: " << myints.size() << '\n';
      if(it != myints.end())
      myints.erase (it++);
      std::cout << "4. size: " << myints.size() << '\n';
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top