Deleting a "value" from a multimap

Question

My requirement is to delete a a "value" from the multimap and not the "key". A key may have multiple values and i want delete a specific value.My requirement is similar to deleting a node from a linked list.

I am doing so by using multimap::erase() method. But after deletion if I try to print the values of the multimap, the values deleted using multimap::erase() are also printed.

below is my code snippet:

void Clientqueues::clearSubscription(string name,string sessionid)
{
    pair<multimap<string,string>::iterator,multimap<string,string>::iterator> i;
    multimap<string, string>::iterator j;
    i = registeredClientInfo.equal_range(name);

    if (j == registeredClientInfo.end())
            return;
    for(j=i.first;j != i.second;++j)
    {
        if((j->second) == sessionid) registeredClientInfo.erase(j->second);
    }

    for(j=i.first;j != i.second;++j)
    {
        cout<<""<<j->second<<endl;///This prints the erased values too;
    }

}

Am i doing something wrong? Any help in this regard greatly appreciated.

Answer 1

Most important, you call erase(j->second), when you meant to call erase(j). You're not erasing the element of the multimap pointed to by j, you're erasing all elements whose keys are equal to the value of the element pointed to by j (which is sessionid). I expect that's nothing.

Also: call equal_range again after the erase loop is complete - the effect of using an erased iterator is undefined, so if you erased the first iterator i.first, then you can't start iterating from there again afterwards.

Note that this also means there's a bug in your loop that does the erase, since in the case that you do call erase, you increment j when it holds an iterator value that's no longer valid. Unfortunately, the correct code is:

for(j=i.first;j != i.second;)
{
    if((j->second) == sessionid) {
        auto next = j;
        ++next;
        registeredClientInfo.erase(j);
        j = next;
    } else {
        ++j;
    }
}

Or if you prefer:

for(j=i.first;j != i.second;)
{
    auto current = j;
    ++j;
    if((current->second) == sessionid) registeredClientInfo.erase(current);
}

Or if the entry is unique for the key/value pair, so that you only have to remove at most one thing, then:

for(j=i.first;j != i.second;++j)
{
    if((j->second) == sessionid) {
        registeredClientInfo.erase(j);
        break;
    }
}

if (j == registeredClientInfo.end()) return; isn't right either, since j is uninitialized when you do it. If the key isn't found, then equal_range returns an empty range (two equal iterator values), so your other loops will do nothing anyway.