Better way to delete element from map

Question

Hi

If we directly delete element from map, it may result into issue (Undefined behavior) if element is not present. Correct? This is Undefined behavior only , right?

We have two options to check before removal. 1. count should be > 0 or iterator should be valid using find. Which one is the better one? Is there any other available option?

Refer code below:
map<int,int> mp;
mp[1] = 1;
mp[2] = 2;

if (mp.count(3))
mp.erase(mp.find(3));

auto itr = mp.find(3);
if (itr != mp.end())
mp.erase(itr);

Answer 1

Ketan Lalcheta

maybe you mean

map<int, int> mp;
mp[1] = 1;
mp[2] = 2;

mp.erase(3); // no error

mp erase(mp.find(3)); //error

But why use find? Or the need for checking? It's just extra operation...

C++20 have contains() which is less disruptive than find()
You can also use count() which returns 0 if there is no such key.

But again, is there really a need for the check?

https://en.cppreference.com/w/cpp/container/map/contains

Answer 2

I'd recommend the following way:

if (mp.find(3) != mp.end()) {
mp.erase(3);
}

It's a partial combination of the 2 options.

It first checks if the key exists using find() before attempting to erase it, preventing undefined behavior that could crash your program. By using find(), you also avoid unnecessary searching through the entire map, making it a more efficient solution.

a more concise way would be as follow:

if (mp.erase(3)) {
}

it might be slightly more efficient, but i find it less readable

Edit: thinking about it a bit more, i think your 2nd option is the best general version as it's easier to add on to and maintain. it's probably very subjective and case dependant at this point which code is best. the solutions above are just other options

auto itr = mp.find(3);
if (itr != mp.end()) {
mp.erase(itr);
}

Edit2: whenever i struggle to decide which option, i err on the safe side and choose the one that allows for most future complexity/general best practice ie the last solution

Answer 3

Meaning we should directly use erase with value and it will not throw error in worst case also and remove the element as well.

Answer 4

Ketan Lalcheta
Are you sure of the undefined behavior you are referring to?

It should just return 0... so basically ignored .

https://stackoverflow.com/questions/695754/c-stl-maperase-a-non-existing-key

https://en.cppreference.com/w/cpp/container/map/erase

https://sololearn.com/compiler-playground/c3IHc22gVPS4/?ref=app

Answer 5

Thanks PietPadda

Why I also prefer second option is search is only once.

When we have iterator ready as return value of find, we just need to call erase on iterator.

Answer 6

My bad Bob_Li

map<int,int> mp;
mp[1] = 1;
mp[2] = 2;

mp.erase(mp.find(3));

Above works meaning erase with key absent is not a problem and it will be ignored

However, mp.erase(mp.find(3)) be a problem