How do I delete elements from an std::set that is stored inside a std::map?

Question

I have the following data structure stored in a class.

class MyClass {
 private:
  std::map<std::string, std::set<std::string>> myMap;
 public:
  void remove(std::string id); //trying to remove items from sets inside myMap
}

There is then a method to attempt to delete items from the sets. I tried the following 2 ways but neither worked. Approach 1, use for range.

for (auto pair : myMap) {
 auto set = pair.second;
 if (set.count(id)) {
  set.erase(id);
 }
}

Approach 2, use iterator.

auto it = myMap.begin();
while (it != myMap.end()) {
 auto set = it->second;
 if (set.count(id)) {
  set.erase(id);
 }
 it++;
}

What's the right way of removing elements from the sets inside a map in C++? Note that my code used to work when I had myMap defined as std::map<std::string, std::set<std::string>*> (pointers).

You are missing `&` in many places in your code, without this you operate on copy of `set`. `auto& pair : myMap`, `auto& set = pair.second;`. — rafix07, Feb 22 '20 at 15:45
Why do you expect that any changes to some variable called `set` will have any impact, whatsoever, on another object that's stored in a map? — Sam Varshavchik, Feb 22 '20 at 15:45
TL;DR: `auto` is not deduced to be reference until you ask explicitly to make that type a reference. Plain `auto` will always create a copy. — Yksisarvinen, Feb 22 '20 at 15:50
It appears you are simulating a [`std::multimap`](https://en.cppreference.com/w/cpp/container/multimap). — Eljay, Feb 22 '20 at 16:04

NutCracker · Accepted Answer · 2020-02-23T20:35:58.733

Suppose we have a std::map like this:

std::map<std::string, std::set<std::string>> myMap;
myMap["a"].insert("aaa");
myMap["a"].insert("abb");
myMap["a"].insert("acc");

myMap["b"].insert("aaa");
myMap["b"].insert("abb");
myMap["b"].insert("acc");

Then you can delete items from std::set by doing the following:

for (auto& i : myMap) {
    i.second.erase("aaa");
}

Demo

Why approach from the question is not working?

Because, by doing the following for(auto pair : myMap) {...} and auto set = pair.second; you are actually working on the copy of the data from myMap. So, instead you need to use references to the actual data like for(auto& pair : myMap) {...} and auto& set = pair.second;.

Furthermore, std::set::erase removes the data from the std::set if it exists so there is no need to manually check for the existence of the id.

Aykhan Hagverdili · Answer 2 · 2020-02-22T15:59:40.880

You're copying the object you meant to mutate. It seems like you have a Java background. In C++ if you want reference semantics (as opposed to value semantics) you should use references (auto & instead of auto in this case). Here's how you do it:

#include <algorithm>
#include <map>
#include <set>
#include <string>

class MyClass {
 private:
  std::map<std::string, std::set<std::string>> myMap;

 public:
  void remove(std::string const& id) {
    std::for_each(myMap.begin(), myMap.end(),
                  [&id](auto& p) { p.second.erase(id); });
  }
};

How do I delete elements from an std::set that is stored inside a std::map?

2 Answers2