Лучший способ удалить элемент из std::list, вложенного в std::map

Как говорится в заголовке, я хотел бы знать, как наиболее эффективно удалить элемент из списка внутри карты, который соответствует определенным условиям (имя и дата). Вот функция, которую я предоставил:

void Register::DeleteActivity(const Date &f,const std::string &a) {
    auto it = Registro.find(f);
    if(it != Registro.end()) {
        if(it->second.empty()) {
            std::cout <<"Error"<<std::endl;
        } else {
            for(auto ip = it->second.begin(); ip != it->second.end();) {
                if(ip->getName() == a && ip->getStartdate() == f){
                    ip->printInfo();
                
                    it->second.erase(ip);
                } else {
                    ip++;
                }
            }
        }
    } else {
        std::cout<< "DeleteActivity::day not found"<<std::endl;
    }
}

А вот и весь класс:

class Register {
    private:
        map<Date,std::list<Activity>> Registro;
    public:
        Register(){};
        void addActivity(Date &z, Activity &n);
        void editActivity(const Date &a, const std::string &c, Date k, const std::string newname);
        void DeleteActivity(const Date &f, const std::string &a);

}

person Andrea Mugnaini    schedule 08.07.2020    source источник


Ответы (2)


Есть ошибка: вам нужно ip = it->second.erase(ip);.
erase делает итератор недействительным и возвращает итератор к следующему элементу.

Но вы можете избавиться от цикла и позволить списку сделать всю работу:

else {
    it->second.remove_if([&f, &a](const Activity& act) 
                                 { return act.getName() == a 
                                       && act.getStartdate() == f; });
}
person molbdnilo    schedule 08.07.2020
comment
Да, это решило проблему! - person Andrea Mugnaini; 08.07.2020

ваш код имеет ошибку сегментации, поскольку стирание делает итератор недействительным, но возвращает итератор к следующему элементу. Так должно быть:

ip = it->second.erase(ip);

Преинкремент также более эффективен, чем постинкремент, поскольку исходное значение не нужно сохранять, а итератор копировать:

++ip;

В противном случае, если у вас нет дубликатов, я бы использовал std::set вместо std::list, который также должен быть более производительным (логарифмический вместо линейного).

#include <iostream>
#include <map>
#include <set>


std::map<const int, std::set<int>> map_set({{0, {1, 2, 3, 4}},
                                            {1, {10, 20, 30, 40}},
                                            {2, {100, 200, 300, 400}},
                                            {3, {}}});


void delete_from_map_set(const int map_index, int value) {

    const auto & map_it = map_set.find(map_index);

    if(map_it != map_set.end()) {

        map_it->second.erase(value);
    }
}


void print_map_set() {
    for(auto & map_it : map_set) {
        std::cout << map_it.first << ": [";
        for(auto & list_it : map_it.second) {
            std::cout << list_it << ", ";
        }
        std::cout << "]" << std::endl;
    }
}


int main(int argc, char * argv[]) {

    print_map_set();

    delete_from_map_set(0, 1);

    print_map_set();

    return 0;
}

person PirklW    schedule 08.07.2020