CODEWITHSUNDEEP
c++
Eric
Eric

Reputation: 2705

Succinct way of modifying/erasing entries while iterating unordered_map?

I wanted to do something like this:

unordered_map<string, deque<string>> table;

...
...

for (const auto &key_deque_pair : table) {
    string key = key_deque_pair.first;
    deque<string> &value_deque = key_deque_pair.second;
    if (value_deque.back() == "NULL") {
        //erase key
        table.erase(key);
    } else {
        //modify value
        swap(value_deque[0], value_deque.back());
        value_deque.resize(1);
    }
}

Then I learned this can lead to an undefined behavior because modifying/erasing while iterating corrupts the iterator.

I think the following code achieves the desired result without this error, in the //erase key case.

auto it = table.begin();
while(it != table.end()) {
    string key = it->first;
    deque<string> &value_deque = it->second;
    if (value_deque.back() == "NULL") {
        //erase key
        it = table.erase(it);
    } else {
        //modify value
        swap(value_deque[0], value_deque.back());
        value_deque.resize(1);
        ++it;
    }
}

Is the //modify value case also correct, or should I erase the entry using it and then put the modified value with that key?

Also, is there any way to do this using for ... each, or in a succinct form?

Upvotes: 2

Views: 1567

Answers (2)

Tony Delroy
Tony Delroy

Reputation: 106086

Is the //modify value case also correct, or should I erase the entry using it and then put the modified value with that key?

It's correct as is - you can freely modify values (but not keys). You might consider this though...

value_deque[0] = std::move(value_deque.back());

...IMHO, it's a clearer expression of what you want done.

Also, is there any way to do this using for ... each, or in a succinct form?

Only thing I can suggest is that you'd cut a line and limit it to the ostensibly needed scope if you used a for statement (i.e. for (auto it = table.begin(); it != table.end(); ) ...

Upvotes: 1

1201ProgramAlarm
1201ProgramAlarm

Reputation: 32732

This is the correct way to delete from a container while iterating thru it. Your //modify value case is correct. There is no way to safely remove from a container using for...each style.

Upvotes: 4

Related Questions