using map erase - Deleting from a map which contains pointer to another map

Question

when i try to remove an element: i go to secondMap which contains in second field a pointer to the first map, now when i try to erase it it gives me problems:

multimap<SortKey,T> firstMap;
multimap<SearchKey,pair<const SortKey,T>*> secondMap;



   template <class T,class SortKey, class SearchKey> T GarageDataBase<T,SortKey,SearchKey>::Remove(SearchKey toRemove) 
{
 multimap<SearchKey,pair<const SortKey,T>*>::iterator it;
 it=secondMap.find(toRemove);
 multimap<SortKey,T>::const_iterator itr;
 itr=(it->second);//pointer to a pair in the first map
 firstMap.erase(itr);

}

i get:

error C2664: 'std::_Tree_iterator<_Mytree> std::_Tree<_Traits>::erase(std::_Tree_const_iterator<_Mytree>)' : cannot convert parameter 1 from 'std::_Tree_const_iterator<_Mytree>' to 'std::_Tree_const_iterator<_Mytree>'

error C2678: binary '=' : no operator found which takes a left-hand operand of type 'const std::pair<_Ty1,_Ty2>' (or there is no acceptable conversion) 

any idea?

Answer 1

1. erase() is not going to work on a const iterator.
2. you cannot use an iterator from one collection to erase an element in another.

3. You should check find() does not return end() to avoid badness on result access

   multimap<SearchKey,pair<const SortKey,T>*>::iterator it; it=secondMap.find(toRemove);

    if (it != secondMap.end())
    { 
     multimap<SortKey,T>::iterator itr;
     itr=firstMap.find(it->second->first);//find matching element in the first map
   
   
     if (itr != firstMap.end())
     { 
      firstMap.erase(itr);
     }
    }
   

EDIT: The design you have here is going to result in a string of increasingly difficult questions and then hard-to-fix bugs. I would reconsider storing pointers like this. If the values in firstMap are identical who cares which particular value gets deleted? My guess is that you need to know this so you can manage the pointers properly.

Better approaches could be:

• use a boost::shared_ptr as your container value so that refcounting is not your problem - the final erase() just does delete for you don't use pointers at all.
• unless your objects are expensive to copy, don't use pointers, just use object instances as your container value. Then resource management is much less of an issue.

Answer 2

A pointer and an iterator are not the same thing. You cannot assign a pair<X,Y>* to a map<X,Y>::const_iterator.

In some cases iterators are simply typedef'ed pointers (this is usually the case with std::vector, for example), but it's not something that your code should rely on, and either way it is generally not true with std::map implementations, because the iterators need additional information stored in them in order to traverse the tree structure that the map is normally implemented as.

Answer 3

Use the correct type as the value of the second map, not a pointer. Using typedef will be easier to see what you are really doing:

typedef std::multimap<SortKey,T> first_mmap;
typedef std::multimap<SearchKey, typename first_mmap::iterator > second_mmap;

first_mmap firstMap;
second_mmap secondMap;

You cannot erase a const iterator, so remove the const:

second_mmap::iterator it = secondMap.find(toRemove);
first_mmap::iterator itr = it->second;

You should treat the case when you don´t find the key in the secondMap:

if (it == secondMap.end()) ...; // handle the error