如何从对象的指针中清空 std::set？

Question

我在清空我的集合时遇到问题，所以我有 3 个这样的类：
A 类，以及 2 个继承的 B 和 C 类。 在代码中，我将 3 种类型的元素存储在我的集合中，集合是：

set<A*> objects;

所以每当我创建一个 B 元素时，我都会这样做：

A* b = new B(); // calling B C'tor 

// and so on with A and C elements I do the exact same.`

所以问题来了，每当我想擦除一个元素，甚至结束程序（调用析构函数）时，我都不知道应该在析构函数中输入什么，我就是这样：

set<A*>::iterator first = objects.begin();
set<A*>::iterator last = objects.end();
while (first != last) {
    set<A*>::iterator to_delete = first;
    objects.erase(to_delete);
    delete *to_delete;    
    ++first;
}

我也尝试将 delete *to_delete; 放在 objects.erase 上方， 还尝试将其单独放置，并尝试将擦除单独放置而不使用delete，但问题是我使用了new，所以我应该在某处使用delete。 一切都不起作用，程序只是崩溃了，我试着让 D'tor 为空，程序可以工作，但是我有很多内存泄漏，我已经检查过了。

请帮帮我，我坚持这件事。 非常感谢

文件： 除了 Destructor 和 removeRoom 函数（基本上哪里是 delete.. 还有 operatorvirtual ~EscapeRoomWrapper();

#include <string>
#include <iostream>
#include <set>
#include "Company.h"
#include "ScaryRoom.h"
#include "KidsRoom.h"
#include "Exceptions.h"

using std::set;
using std::endl;

using namespace mtm;
using namespace escaperoom;

Company::Company(string name, string phoneNumber) : name(name), phoneNumber(phoneNumber) {

}

Company::~Company() {
    while(!rooms.empty()) {
        set<EscapeRoomWrapper*>::iterator iter = rooms.begin();
        rooms.erase(iter);
        delete *iter;
    }
//  set<EscapeRoomWrapper*>::iterator first = rooms.begin();
//  set<EscapeRoomWrapper*>::iterator last = rooms.end();
//  while (first != last) {
//      set<EscapeRoomWrapper*>::iterator to_delete = first;
//      rooms.erase(to_delete);
//      delete *to_delete;
//
//      ++first;
//      last = rooms.end();
//  }
//  while (rooms.begin() != rooms.end()) {
//      set<EscapeRoomWrapper*>::iterator to_delete = rooms.begin();
//      rooms.erase(to_delete);
//      delete *to_delete;
//  }
}

Company::Company(const Company& company) : name(company.name), phoneNumber(company.phoneNumber), rooms(company.rooms) {

}

Company& Company::operator=(const Company& company) {
    if (this == &company) {
        return *this;
    }
    name = company.name;
    phoneNumber = company.phoneNumber;
    rooms.clear();
    rooms = company.rooms;
    return *this;
}

void Company::createRoom(char* name, const int& escapeTime, const int& level, const int& maxParticipants) {
    try {
        EscapeRoomWrapper* room = new EscapeRoomWrapper(name, escapeTime, level, maxParticipants);
        rooms.insert(room);
    } catch (EscapeRoomMemoryProblemException& e) {
        throw CompanyMemoryProblemException();
    }
}

void Company::createScaryRoom(char* name, const int& escapeTime, const int& level,
                                 const int& maxParticipants, const int& ageLimit, const int& numOfScaryEnigmas) {
    try {
        EscapeRoomWrapper* room = new ScaryRoom(name, escapeTime, level, maxParticipants, ageLimit, numOfScaryEnigmas);
        rooms.insert(room);
    } catch (EscapeRoomMemoryProblemException& e) {
        throw CompanyMemoryProblemException();
    }
}

void Company::createKidsRoom(char* name, const int& escapeTime, const int& level,
        const int& maxParticipants, const int& ageLimit) {
    try {
        EscapeRoomWrapper* room = new KidsRoom(name, escapeTime, level, maxParticipants, ageLimit);
        rooms.insert(room);
    } catch (EscapeRoomMemoryProblemException& e) {
        throw CompanyMemoryProblemException();
    }
}

set<EscapeRoomWrapper*> Company::getAllRooms() const {
    return rooms;
}

void Company::removeRoom(const EscapeRoomWrapper& room) {
    set<EscapeRoomWrapper*>::iterator first = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last = rooms.end();
    while (first != last) {
        if (**first == room) {
            break;
        }
        ++first;
    }
    if (first == last) {
        throw CompanyRoomNotFoundException();
    }
    delete *first;
    rooms.erase(first);
   // delete *first;     // check this
}

void Company::addEnigma(const EscapeRoomWrapper& room, const Enigma& enigma) {
    set<EscapeRoomWrapper*>::iterator first = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last = rooms.end();
    while (first != last) {
        if (**first == room) {
            break;
        }
        ++first;
    }
    if (first == last) {
        throw CompanyRoomNotFoundException();
    }
    (**first).addEnigma(enigma);
}

void Company::removeEnigma(const EscapeRoomWrapper& room, const Enigma& enigma) {
    set<EscapeRoomWrapper*>::iterator first = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last = rooms.end();
    while (first != last) {
        if (**first == room) {
            break;
        }
        ++first;
    }
    if (first == last) {
        throw CompanyRoomNotFoundException();
    }
    try {
        (**first).removeEnigma(enigma);
    } catch (EscapeRoomNoEnigmasException& e) {
        throw CompanyRoomHasNoEnigmasException();
    } catch (EscapeRoomEnigmaNotFoundException& e) {
        throw CompanyRoomEnigmaNotFoundException();
    }
}

void Company::addItem(const EscapeRoomWrapper& room, const Enigma& enigma, const string& element) {
    set<EscapeRoomWrapper*>::iterator first_room = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last_room = rooms.end();
    while (first_room != last_room) {
        if (**first_room == room) {
            break;
        }
        ++first_room;
    }
    if (first_room == last_room) {
        throw CompanyRoomNotFoundException();
    }
    vector<Enigma>::iterator first = (**first_room).getAllEnigmas().begin();
    vector<Enigma>::iterator last = (**first_room).getAllEnigmas().end();
    while (first != last) {
        if (*first == enigma) {
            break;
        }
        ++first;
    }
    if (first == last) {
        throw CompanyRoomEnigmaNotFoundException();
    }
    (*first).addElement(element);
}

void Company::removeItem(const EscapeRoomWrapper& room, const Enigma& enigma, const string& element) {
    set<EscapeRoomWrapper*>::iterator first_room = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last_room = rooms.end();
    while (first_room != last_room) {
        if (**first_room == room) {
            break;
        }
        ++first_room;
    }
    if (first_room == last_room) {
        throw CompanyRoomNotFoundException();
    }
    vector<Enigma>::iterator first = (**first_room).getAllEnigmas().begin();
    vector<Enigma>::iterator last = (**first_room).getAllEnigmas().end();
    while (first != last) {
        if (*first == enigma) {
            break;
        }
        ++first;
    }
    if (first == last) {
        throw CompanyRoomEnigmaNotFoundException();
    }
    try {
        (*first).removeElement(element);
    } catch (EnigmaNoElementsException& e) {
        throw CompanyRoomEnigmaHasNoElementsException();
    } catch (EnigmaElementNotFoundException& e) {
        throw CompanyRoomEnigmaElementNotFoundException();
    }
}

set<EscapeRoomWrapper*> Company::getAllRoomsByType(RoomType type) const {
    set<EscapeRoomWrapper*> filtered_set;
    set<EscapeRoomWrapper*>::iterator first_room = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last_room = rooms.end();
    EscapeRoomWrapper* ptr = NULL;
    while (first_room != last_room) {
        if (type == BASE_ROOM) {
            if (dynamic_cast<ScaryRoom*>(*first_room) == ptr
                    && dynamic_cast<KidsRoom*>(*first_room) == ptr) {
                filtered_set.insert(*first_room);
            }
        }
        if (type == SCARY_ROOM) {
            if (dynamic_cast<ScaryRoom*>(*first_room) != ptr) {
                filtered_set.insert(*first_room);
            }
        }
        if (type == KIDS_ROOM) {
            if (dynamic_cast<KidsRoom*>(*first_room) != ptr) {
                filtered_set.insert(*first_room);
            }
        }
        ++first_room;
    }
    return filtered_set;
}

EscapeRoomWrapper* Company::getRoomByName(const string& name) const {
    set<EscapeRoomWrapper*>::iterator first_room = rooms.begin();
    set<EscapeRoomWrapper*>::iterator last_room = rooms.end();
    while (first_room != last_room) {
        if ((**first_room).getName() == name) {
            break;
        }
        ++first_room;
    }
    if (first_room == last_room) {
        throw CompanyRoomNotFoundException();
    }
    return *first_room;
}

std::ostream& mtm::escaperoom::operator<<(std::ostream& output, const Company& company) {
    output << company.name << " : " << company.phoneNumber << endl;
    set<EscapeRoomWrapper*>::iterator first_room = company.getAllRooms().begin();
    set<EscapeRoomWrapper*>::iterator last_room = company.getAllRooms().end();
    while (first_room != last_room) {
        output << **first_room << endl;
        ++first_room;
    }
    return output;
}

Answer 1

问题是当从集合中移除某些东西并且存储在last 中的值无效时，objects.end() 会发生变化。
您可以按如下方式修复您的代码：

    while (std::begin(objects) != std::end(objects)) {
        set<A*>::iterator to_delete = objects.begin();
        objects.erase(to_delete);
        delete *to_delete;
    }

---

一般来说，您根本不应该在集合中使用原始指针。而是使用类似的东西

std::set<std::unique_ptr<A>> objects;

在您的程序中。所以你根本不需要关心对象的正确释放。

Answer 2

您的方法的关键问题是您在迭代容器时修改了容器。我建议将其重构为：

while (!objects.empty()) {
     set<A*>::iterator it = objects.begin();
     objects.erase(it);
     delete *it;
}

或者你可以用 C++11 和 lamdas 做这样的事情：

std::for_each(objects.begin(), objects.end(), [](A* obj){ delete obj; });
objects.clear();

---

刚刚根据您的描述在简化版本上进行了测试，以下 sn-p 对我来说效果很好：

#include <iostream>
#include <set>

using namespace std;


class A {
};

class B : public A {
};

int main(int argc, const char *argv[])
{
    set<A*> objects;

    for (int i = 0; i < 10; i++) {
        objects.insert(new B());
    }

    for(set<A*>::iterator it = objects.begin(); it != objects.end(); ++it) {
        delete *it;
    }
    objects.clear();
    cout << "Hello World" << endl;
    return 0;
}

我怀疑我们在这里遗漏了一些细节。

---

更新

好的，虽然由于大部分细节仍然​​缺失，因此很难看到您在此处尝试执行的操作的全貌，但我发现了复制构造函数的一个潜在问题。在您更新的代码中，您正在对 Company 对象进行浅拷贝，但我认为您的意图是：

Company& Company::operator=(const Company& company) {
    if (this == &company) {
        return *this;
    }
    name = company.name;
    phoneNumber = company.phoneNumber;
    // Also clear might be not enough since you also need to release heap memory

    //rooms.clear();

    while (!rooms.empty()) {
       set<A*>::iterator it = rooms.begin();
       rooms.erase(it);
       delete *it;
    }

    // Deep copy of set of rooms in company object
    for (set<Room*>::iterator it = company.rooms.begin(); it != company.rooms.end(); ++i ) {
       rooms.insert(new Room(*it));
    }
    return *this;
}

Answer 3

如果您只想删除所有内容，那么有一个更简单的解决方案。先删除所有内容，然后致电clear()。您可以使用std::for_each 将删除部分变为单行。这是一个例子：

#include <algorithm>
// ...

std::for_each(begin(objects), end(objects), [](auto ptr) { delete ptr; });
objects.clear();

请注意，如果 objects 之后立即超出范围，则 clear() 甚至没有必要。

如果要删除单个元素，那么先delete指针再擦除：

delete *iter; // decltype(iter) = std::set<A*>::iterator
objects.erase(iter);

还可以考虑使用std::unique_ptr 而不是原始指针。

Answer 4

我相信你的问题是XY problem 的一个例子：你想清空你的指针集，但你只是想要这样做，因为你想手动破坏该集而不泄漏内存。你只需要这样做，因为自动销毁不会为你解决这个问题。

在我看来，真正的解决方案是避免使用new 进行手动分配。你会怎么做？大概有3个选项：

1. unique_ptr的
2. shared_ptr的
3. std::variant的

我猜您实际上不需要将一家公司分配给另一家公司，同时保留两者；因为您的赋值运算符和复制构造函数语义是错误的：您正在通过另一家公司（应该是const）来修改一家公司的房间。你可能只是执行move constructor：

Company::Company(Company&& company);

您将在其中“抢占”旧公司的房间集，将其留空（可能还有移动分配操作员）。如果是这种情况 - 您只拥有一个房间的引用，unique_ptr<EscapeRoomWrapper> 可以。您不必手动删除任何内容，并且该集合的默认析构函数可以正常工作。事实上，您也许可以完全删除自定义移动构造函数，而只使用默认构造函数。

如果您确实需要多个公司来引用同一个房间，并且确实需要那些非右值构造函数和赋值运算符 - 使用shared_ptr<EscapeRoomWrapper>。同样，您不需要删除任何内容，并且当最后一次对它们的引用被破坏时，房间将被删除。这将花费您一些开销，但无论如何这不是性能关键代码，所以没关系。

最后，如果房间的种类有限，并且它们都继承自包装类 - 您可以考虑用不同房间类的 std::variant 替换虚拟继承，并且根本不使用指针 - 只需一套房间。

要获得我的建议的更多灵感，请考虑阅读零规则，例如在this blog post。

PS - 您确定需要订购房间吗？如果没有，请使用std::unordered_set 而不是std::set。