Narasimha Rao K
Narasimha Rao K

Reputation: 1

Map/iterator incremental error

The following code throwing debug assertion map/iterator incremental error ..

void ClassA::Remove()
{
    std::map<int, CVClassB*>::iterator it(m_p.begin());
    while ( it != m_p.end() )
    {
        if (it->first >= 0)
        {
            m_p.erase(it);
            it++;
        }   
    }
}

Can you please let me know what is the error

Upvotes: 0

Views: 228

Answers (3)

Jellybaby
Jellybaby

Reputation: 986

Your invalidating the iterator by removing inside the loop but in any case all that does is clear the map. Just call m_p.clear() and it will do exactly what you are trying to do. Although not sure what your trying to do is what you intended to do but that's another issue.

If you want to delete the objects pointed to then delete them then clear the map.

for(item : m_p)
   delete item->second;
m_p.clear();

//done

Upvotes: -3

BoBTFish
BoBTFish

Reputation: 19767

std::map::erase invalidates the iterator on which it operates. So it is not safe to increment it afterwards. But erase() does return the next iterator for you:

it = m_p.erase(it);

Also, you only increment it inside the if, so unless all the keys are >=0, you will get stuck in an infinite loop. You probably wanted something like:

// delete all keys >= 0
if (it->first>=0) {
    it = m_p.erase(it); // erase and increment
}
else {
    ++it; // just increment
}

Also, as Vlad's answer alludes to, who manages the lifetime of the CVClassB*? Do you need to delete it? Why use a pointer at all, you can probably store the value in the map directly. (Or use a smart pointer).

Upvotes: 4

Vlad from Moscow
Vlad from Moscow

Reputation: 310990

Write the loop like

while ( it != m_p.end() )
{
    if (it->first >= 0)
    {
        it = m_p.erase(it);
    }
    else
    {
        ++it;
    }
}

Also it seems you should delete the object pointed to by the erased iterator. For example

        delete *it;
        it = m_p.erase(it);

Upvotes: 1

Related Questions