Stuck in inf loop in a multimap iterator?

[Steve Edwards]

Hi,
While iterating through a multimap, I need to replace elements that meet
certain conditions with a new element.

There doesm't seem to be a replace() function for multimaps, so I'm
inserting a new element, and then deleting the old.

typedef multimap<double, MyStructType, greater<double> >;MyMultimap

MyMultimap map = ...

MyMultimap::iterator iter;

for(iter = map.begin(); iter!= map.end())
{
if(...)
{
map.insert(MyMultimap::value_type(newKey, newData));
map.erase(iter++);
}
else
++iter;
}

The for loop never exits.
I'm guessing that the iterator is messed up after the insert, but can't
see how to correct it.

Is there a way around this, or even a more efficient way to replace
elements altogether?

Thanks

Steve

 

[Maxim Yegorushkin]

Hi,
While iterating through a multimap, I need to replace elements that meet
certain conditions with a new element.

There doesm't seem to be a replace() function for multimaps, so I'm
inserting a new element, and then deleting the old.

typedef multimap<double, MyStructType, greater<double> >;MyMultimap

MyMultimap map = ...

MyMultimap::iterator iter;

for(iter = map.begin(); iter!= map.end())
{
if(...)
{
map.insert(MyMultimap::value_type(newKey, newData));
map.erase(iter++);
}
else
++iter;
}

The for loop never exits.
I'm guessing that the iterator is messed up after the insert, but can't
see how to correct it.

Only the iterator to the deleted elemented becomes invalid. Other
iterators are not affected.

It looks like when you insert a new element it gets inserted after
iter. On a next iteration if(your new element) yields true and you
insert another element after it. That's how you probably get your
endless loop.

Is there a way around this, or even a more efficient way to replace
elements altogether?

Build a new map rather modifying an existing one and then swap the
maps. Or try boost::multi_index.

 

[Balaji]

Erase returns the next iterator. So, iter = map.erase(iter) should
work.

 

[Maxim Yegorushkin]

Erase returns the next iterator. So, iter = map.erase(iter) should
work.

There is no difference between:

iter = map.erase(iter);

and

map.erase(iter++);

 

[Balaji]

map.erase(iter++)

Here, we erase what iter points to. This could invalidate iter. We then
try to increment a invalid iter.

iter = map.erase(iter)

Here, erase returns us the right next iterator.

Maxim, am I missing something?

 

[Maxim Yegorushkin]

map.erase(iter++)

Here, we erase what iter points to. This could invalidate iter. We then
try to increment a invalid iter.

Wrong. Note the postfix increment, we increment first, then pass an old
copy to erase().

iter = map.erase(iter)

Here, erase returns us the right next iterator.

Maxim, am I missing something?

Also note that the standard (multi)map::erase does not return an
iterator. This is a non-standard extension.

 

[Dietmar Kuehl]

map.erase(iter++)

Here, we erase what iter points to. This could invalidate iter.

This could invalidate the pointer passed to 'map.erase()' which is,
however, a different one than 'iter'! 'iter++' moves to the next
position and then returns the original iterator. This returned
iterator is passed to 'erase()' and invalidated.

We then try to increment a invalid iter.

Nope, we don't.

iter = map.erase(iter)

Here, erase returns us the right next iterator.

Maxim, am I missing something?

Apparently you missed the use of 'operator++(int)' on the argument
to 'erase()' - or you missed how post increment works for iterators.

 

[Steve Edwards]

There is no difference between:

iter = map.erase(iter);

and

map.erase(iter++);

Thanks, rebuilding the map worked best.

Steve

 

[a]

You could use one of the several variants of the std::replace algorithm.

A