multimap iterator error

[CeTis]

Ok so I've searched for quite some time trying to find an answer to this. I've found multiple things that are similar and tried all of them and haven't been able to resolve this issue.

I have a multimap that's storing a (string) name and an object pointer.

I'm attempting to delete the pointer and then erase the storage from the multimap.

It looks like this:

void CObjectManager::Remove(CEntityManager * obj)
{
	std::multimap<std::string, CEntityManager*>::iterator it;

	for (it = _mEntity.begin(); it != _mEntity.end(); it++)
	{
		if (it->second == obj)
		{
			delete it->second;
			_mEntity.erase(it);
		}
	}
}

I'm getting the error map/set iterator not incrementable.

The class calling it is a derived class from an asset manager class.

It looks like this:

CGame::GetManager().Remove(this);

Any ideas anyone?

[clanmjc]

void CObjectManager::Remove(CEntityManager * obj)
{
	std::multimap<std::string, CEntityManager*>::iterator it = _mEntity.begin();
        
	while ( it != _mEntity.end())
	{
		if (it->second == obj)
		{
                        //Make a copy of the current iterator, then increment the real one.
                        iterator tmp(it);  //Insert the correct iterator type here, I did this to save space.
                        ++it;
			delete tmp->second;
			_mEntity.erase(tmp);
		}
                 else
                 {
                     ++it;
                 }
	}
}

[CeTis]

I apologize if I'm doing this wrong but I added this the copy iterator where you said like so:

//Make a copy of the current iterator, then increment the real one.
std::multimap<std::string, CEntityManager*>::iterator tmp(it); 

but i'm still getting the same error

[CeTis]

anyone have any ideas on this? I'm throwing darts right now trying to figure it out...I've probably read maybe 20-30 threads involving similar issues and none of their recommendations seem to fix it.

[naraku9333]

Maybe the call to erase invalidates the iterator, since you are using a map can you delete by the key? Also, shouldn't the delete be in the objects dtor?

[clanmjc]

Did you change it to a while loop or did you keep the for loop?

[CeTis]

I had your exact code with the tmp iterator assigned. But I've changed it since that wasn't working (See below)

This is what it looks like at the moment:

void CObjectManager::Remove(std::string name, CEntityManager* obj)
{
	std::multimap<std::string, CEntityManager*>::iterator it = _mEntity.begin();

	while(it != _mEntity.end())
	{
		if (it->second == obj)
		{
			_mEntity.erase(it++);
		}
		else
			++it;
	}
}

Maybe it's the way im creating the objects?

This is for a game I'm working on, I'm trying to create multiple projectiles and store them into the map:

void CPlayer::CreateProjectile(float elapsedTime, float delay)
{
	if (elapsedTime >= (_timer + delay))
	{
		_timer += delay;
		CProjectile* projectile = new CProjectile();
		projectile->SetMoveSpeed(CGame::GetInput().GetMouseX(), CGame::GetInput().GetMouseY());
		CGame::GetManager().Add("Projectile", projectile);
		projectile->SetPosition(GetSprite().GetPosition().x,        GetSprite().GetPosition().y);
	}
}

Then when the objects run out of the window boarders, I remove them.

[naraku9333]

Try

void CObjectManager::Remove(std::string name, CEntityManager* obj)
{
	std::multimap<std::string, CEntityManager*>::iterator it = _mEntity.begin();

	while(it != _mEntity.end())
	{
		if (it->second == obj)
		{
			_mEntity.erase(it);
                        it = _mEntity.begin();
		}
		else
			++it;
	}
}

[CeTis]

I've tried this before and it didn't work.

Error is : map/set iterator is not incrementable.

[CeTis]

I noticed that both the derived class and the base class DTOR are being called. Is that supposed to happen or is that an error on my part? Could that be causing the issue?

Sorry for the newbie questions, I can't remember this exact part of the thousands of pages of c++ books I've read up to this point.

[Lowest0ne]

Try it++?

This thread encouraged me to try out a multi-map, so forgive me for some newb stuff. The multi-map is basically a binary tree which has its contents sorted via the key. Therefor, the fast searching time of your map is wasted when you search via the value.

If you can ensure that your keys are always unique, then erasing a node in the map is as simple as:
myMap.erase(myMap.find(keyValue);

If your key values are not unique, then that code will only erase the first pair that it finds and doesn't work the way you'd want. You need to find the range of said key, and then search through that range looking for your value (if you want to follow my lead).

I've been playing with a file from a game, and thought I'd play with multi-maps with this file:

btw, the incrementing works fine on my computer.

int main(void)
{
  // Techs is a class holding (string, int, string)
  multimap<int, Tech> techs;
  Tech *hold;
  Tech temp;

  ifstream input("input.txt");
  
  while (temp.read(input).good())  // Reads the line of the file
  {      
    techs.insert(pair<int, Tech>(temp.cost, temp));

    // This is to simulate your "CEntityManager* obj"
    if (temp.name == "FUSION")
      hold = new Tech(temp.name, temp.cost, temp.era);
  }
  input.close();
  

  // The multi-map key's are the cost of the tech
  // These two lines create a range with these key values
  pair<multimap<int,Tech>::iterator,multimap<int,Tech>::iterator> range;
  range = techs.equal_range(8000);  // "FUSION" happens to have a cost of 8000


  // Find the "CEntityManager* obj"
  multimap<int, Tech>::iterator iter;
  for (iter = range.first; iter != range.second; iter++)
  {
    if (iter->second == *hold) break;
  }
  // Erase it
  techs.erase(iter);


  //Looks good to me
  for (iter = techs.begin() ; iter != techs.end() ; iter++)
  {
    iter->second.print();
  }


  delete hold;
  cin.get();
  return 0;
}

So I would imagine you could use your "name" variable to find the range, and then look through that range with your "*obj"

[CeTis]

First off. THANK YOU! This was becoming the bane of my day.

I don't think I quite understood the advantages to using the map yet, so that makes perfect sense to me in that it's for quick index identification and removal etc.

I was able to fix the issue by using your example, adding "break" on the for loop and then erasing the index.

Thank you once again, I will try to find a better way to use the map effectively now instead of smashing my keyboard :)

[Lowest0ne]

I guess the way I did it is somewhat dangerous. Make sure that iterator != range.second before trying to erase it from your map.