Remove only one element from multimap with duplicate keys

Question

I have a multimap with Note objects from which I want to remove only one object. There can be several Note objects with the same key. The problem is that right now there's also objects being removed that aren't within the key range I specify:

long key = note.measureNumber * 1000000 + note.startTime; // = 2000001
multimap<long, Note>::iterator it;
for (it = noteList.lower_bound(key); it != noteList.end() && it->first < (key + 1); it++) {
    if(it->second.frequency == note.frequency){
        noteList.erase(it);
    }
}

When I run this code with key 2000001 the object I'm able to erase the right object, but another object with key 1000017 gets erased as well. Both objects have the same frequency though.

Any idea's what is wrong with my for loop?

EDIT: To be clear, I only want to check for objects with one specific key (in this case 2000001) there's no need for the iterator to look at objects with different keys than that one.

score 2 · Answer 1 · edited May 23 '17 at 11:50

2

Calling erase() with the iterator will invalidate it, so you can't then continue to use it.

See Can I continue to use an iterator after an item has been deleted from std::multimap<>

edited May 23 '17 at 11:50

Community

1
1

answered Aug 21 '12 at 20:04

Rob Walker

46,588
15
99
136

score 1 · Accepted Answer · answered Aug 21 '12 at 20:05

1

Once you erase an iterator it becomes invalid. If you wish to erase from a map while you iterate through it, your code needs to change. Try this:

multimap<long, Note>::iterator it;
for (it = noteList.lower_bound(key); it != noteList.end() && it->first < (key + 1);) {
    if(it->second.frequency == note.frequency){
        noteList.erase(it++);
    }
    else
    {
        ++it;
    }
}

answered Aug 21 '12 at 20:05

Craig H

7,949
16
49
61

Actually there's only one element that has to be erased every time. Any idea how I can just escape the for loop when an element has been erased? – Aug 21 '12 at 20:08
1

Put a 'break' after the erase line – Rob Walker Aug 21 '12 at 20:10
I'll try that. I actually thought that would only escape the if condition – Aug 21 '12 at 20:24

Alex I. · Answer 3 · 2012-08-21T21:37:50.480

0

As already indicated here, erasing an iterator invalidates it. I'd like to point out some inefficiency in code you have: You don't need to iterate till the end of the loop. Consider this:

for (it = noteList.lower_bound(key); it != noteList.upper_bound(key) && it->first == key; it++)
{
    if(it->second.frequency == note.frequency)
    {
       noteList.erase(it++);
    }
    else
    {
        ++it;
    }
}

edited Aug 21 '12 at 21:37

answered Aug 21 '12 at 21:23

Alex I.

1
2

Will that make a difference though? as soon as 'it->first == key;' is met the loop will stop anyway, right? – Aug 21 '12 at 22:01

Remove only one element from multimap with duplicate keys

3 Answers3