# map::erase() What in gods name is happening

## Recommended Posts

RunningInt    133
my map has an integer key to an object. This is reference code:
map<int,CMyClass> mymap;

void CMyClass::doshit()
{
mymap.erase(this.key);
}


So each object stores its key into the map. Then a member function can call map::erase to remove that object from the map. Essentially the idea is that the object can delete itself from the map. It's a bit dodgy, but I thought it would work seeing as "delete this" is similar and is generally ok if done properly. Instead map::erase does some sort of amazing half-effort. It doesn't remove the specified key/value pair from the map. But it does NULL the value as an afterthought. So all parameters of the object stored become zero, but you still encounter that object during an iteration of the map. I really hate having to implement the object to raise a "kill me" flag and the main program has to scan over all objects and murder the ones who want out. It would be much better if the objects could just commit suicide. Does anyone have a good way of doing such a thing? Also if anyone feels like defending map::erase for its above mentioned actions.

##### Share on other sites
Doggan    528
You are using the erase function wrong. There are two uses for it:

1. mymap.erase(element) will remove all elements with the value element. It returns the # of removed elements.

2. mymap.erase(position), where posistion is an iterator. It removes the element that your iterator points to. It returns nothing. (You can also use mymap.erase( begpos, endpos) to delete a range).

As you can see, you can't actually delete all elements with a certain 'key' as you have done. I am guessing you want #2. Try this:

map<int,CMyClass>::iterator pos;for ( pos = mymap.begin(); pos != mymap.end(); ++pos ){    if ( pos->first == this.key )    {        mymap.erase( pos );        break;  // since it's just a map you can break here.    }}

Since you're using a map, this works fine. If you had a multimap it would get a little more complicated since there would be multiple elements with the same value to delete and you have to be careful not to invalidate your iterator.

Hope that helps.

edit: fixed my original source. i just woke up and wasn;t thinking ;p

##### Share on other sites
Enigma    1410
Quote:
 Original post by Doggan1. mymap.erase(element) will remove all elements with the value element. It returns the # of removed elements.

No, it's mymap.erase(key) and it will remove the element whose key is the given value.
From the source of stlport:
size_type erase(const key_type& __x)

OP: Most likely there is a bug somewhere else in the code that is causing the problem. map.erase(key) will remove an element from the map as you would expect.

Enigma

##### Share on other sites
Doggan    528
Quote:
Original post by Enigma
Quote:
 Original post by Doggan1. mymap.erase(element) will remove all elements with the value element. It returns the # of removed elements.

No, it's mymap.erase(key) and it will remove the element whose key is the given value.
From the source of stlport:
size_type erase(const key_type& __x)

OP: Most likely there is a bug somewhere else in the code that is causing the problem. map.erase(key) will remove an element from the map as you would expect.

Enigma

You're right. I was going by The C++ Standard Library book by Josuttis. I guess it's an error.

Either way, my fix should still work.

##### Share on other sites
petewood    819
Quote:
 Original post by RunningIntI really hate having to implement the object to raise a "kill me" flag and the main program has to scan over all objects and murder the ones who want out. It would be much better if the objects could just commit suicide. Does anyone have a good way of doing such a thing?

Use smart pointers, particularly boost's shared_ptr and weak_ptr.

##### Share on other sites
leiavoia    960
Quote:
 I really hate having to implement the object to raise a "kill me" flag and the main program has to scan over all objects and murder the ones who want out. It would be much better if the objects could just commit suicide. Does anyone have a good way of doing such a thing?

Not necessarily. I say that because i've approached that problem from both ends. I prefer the "killme flag" because other objects may be doing calculations on the killed object still, such as collision detection or some form of tracking. I would prefer to do everyone's calculations for that one frame, then wipe out the killme's afterward.

##### Share on other sites
daerid    354
Quote:
Original post by leiavoia
Quote:
 I really hate having to implement the object to raise a "kill me" flag and the main program has to scan over all objects and murder the ones who want out. It would be much better if the objects could just commit suicide. Does anyone have a good way of doing such a thing?

Not necessarily. I say that because i've approached that problem from both ends. I prefer the "killme flag" because other objects may be doing calculations on the killed object still, such as collision detection or some form of tracking. I would prefer to do everyone's calculations for that one frame, then wipe out the killme's afterward.

boost::shared_ptr will do that for you.

##### Share on other sites
MaulingMonkey    1730
Quote:
Original post by leiavoia
Quote:
 I really hate having to implement the object to raise a "kill me" flag and the main program has to scan over all objects and murder the ones who want out. It would be much better if the objects could just commit suicide. Does anyone have a good way of doing such a thing?

Not necessarily. I say that because i've approached that problem from both ends. I prefer the "killme flag" because other objects may be doing calculations on the killed object still, such as collision detection or some form of tracking. I would prefer to do everyone's calculations for that one frame, then wipe out the killme's afterward.

Another benifit of a killme flag: If removing multiple elements from a container such as a list, the erasing code need only make a single pass (checking the killme flag each time) than multiple passes (to find each element individually, then erase it).

For a large list, this reduces the time required to erase all the elements from O(N) to O(1) (working with a list of fixed size - of course a larger list will take longer to iterate through, but I don't know how to express the correct statement otherwise ;_;)

## Create an account

Register a new account