CODEWITHSUNDEEP
c++segmentation-faultunordered-map
rutuja
rutuja

Reputation: 55

std::unordered_map not being updated properly, when modify it in a for loop

Trying to update an unordered map using the following code snippet to have only lowercase letters, but it seems to stop after erasing one key-value pair { [33 '!']: 3 } and exits the loop leaving the rest of the map unvisited and prints the partly updated map.

 for (auto &i : m)
        if (!(i.first >= 'a' && i.first <= 'z'))
            m.erase(i.first);

Following debugging images revealed the above

enter image description here

enter image description here

The complete code is herewith:

#include <iostream>
#include <unordered_map>
#include <algorithm>    
using namespace std;
int main()
{
    string line = "Try! Try! Try! until you succeed";
    //getline(cin, line);
    unordered_map<char, int> m;
    for (int i = 0; line[i]; i++)
    {   
        char lower = (char)tolower(line[i]);
        if (m.find(lower) == m.end())
            m.insert(make_pair(lower, 1));
        else
            m[lower]++;
    }

    for (auto &i : m) //only updates until ! 
        if (!(i.first >= 'a' && i.first <= 'z'))
            m.erase(i.first);

    cout<<"The freq. map so formed is : \n";
    for (auto &i : m)
        cout<<i.first<<"\t"<<i.second<<endl;
    
    return 0;
}
/*
OUTPUT : 
The freq. map so formed is : 
d       1
t       4
r       3
e       2
y       4
l       1
o       1
        5
n       1
u       3
i       1
s       1
c       2
*/

Can't seem to understand why it won't loop through the complete unordered map.

Also, not sure if this helps towards a clear picture but, when the standard map is used instead of unordered map it gives an Address Boundary Error at the same instance where the next character of the map needs to be updated like so:

enter image description here

enter image description here

Upvotes: 2

Views: 567

Answers (2)

Marek R
Marek R

Reputation: 37607

One of the c++ traps is that iterators are invalidated for most containers when they are modified.

std::unordered_map<Key,T,Hash,KeyEqual,Allocator>::erase - cppreference.com

References and iterators to the erased elements are invalidated. Other iterators and references are not invalidated.

So when you have remove item form m current iterators are become invalid.

Now range base for loop uses iterators underneath.

The beast way to fix it us use std::erase_if algorithm:

std::erase_if(m.begin(), m.end(), [](const auto& i) { 
    return !(std::islower(i.first)); 
});

Upvotes: 5

qex
qex

Reputation: 54

You cannot erase the element of a map while iterating this way. When you erase the iterator, it becomes invalidated, so you need to explicitly increment it before you delete the element.

Try this code instead:

 for (auto it = m.begin(); it != m.end();)
     if (!((*it).first >= 'a' && (*it).first <= 'z'))
         it = m.erase(it);
     else
        ++it;

Upvotes: 3

Related Questions