0

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;
}
ARH
  • 1,355
  • 3
  • 18
  • 32
  • Both approaches have mistakes - can you explain more clearly what you're actually trying to do? – us2012 Feb 13 '13 at 04:21
  • I build a sorted list using multiset container. Hence, I need to erase some elements from the middle of the sorted list with out knowing their keys. For example sorted list of 1 2 3 4. After a while, I need to delete one element from sorted list (like middle element) without knowing its key. Therefore, I maintain a set of iterators, pointing to each element of sorted list in a separate data structure, later on , I will pick one iterator and erase associated elements from sorted list. – ARH Feb 13 '13 at 04:26
  • Yeah, but which ones do you need to remove? What's the condition for an element to be removed? – us2012 Feb 13 '13 at 04:26
  • If you don't know the key how do you delete element? and how do you know what you are deleting? – billz Feb 13 '13 at 04:30
  • After any insert or erase operation, any iterator sitting in an external data structure might be invalidated. You can't do that. – jmucchiello Feb 13 '13 at 04:53

2 Answers2

3

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.

us2012
  • 16,083
  • 3
  • 46
  • 62
  • Is there any way to maintain it live (pointing to the next of elements with the same key) after erase(it) ? – ARH Feb 13 '13 at 04:23
  • There may not be a next element with 'the same key' (I guess you just mean an equivalent next element - this is a multiset, not a multimap.). What are you *actually* trying to do? Erase all fives from the multiset? – us2012 Feb 13 '13 at 04:24
  • I want to erase only one 5 from multiset and maintain the it valid after erase for next erase. – ARH Feb 13 '13 at 04:36
  • @ARH Valid for what? You just deleted the element it refers to. – jmucchiello Feb 13 '13 at 04:54
  • @jmucchiello Given the first comment on my answer, I assume he means "pointing to the element straight after the erased one", although I question the practicality of this for a sorted structure like a `multiset`. – us2012 Feb 13 '13 at 04:56
  • @us2012 Yes, but given the vagueness of these requirements I don't think he's thought through what he really wants. And my question is trying get him to think about it. – jmucchiello Feb 13 '13 at 05:00
  • Can't you simply write myints.erase(it++); instead of using a temporary variable (`newit`)? – Kira Nov 11 '15 at 18:39
0

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';
user258367
  • 3,247
  • 2
  • 18
  • 17