-1

我在清空我的集合时遇到问题,所以我有 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 为空,程序可以工作,但是我有很多内存泄漏,我已经检查过了。

请帮帮我,我坚持这件事。非常感谢<3

文件: 除了 Destructor 和 removeRoom 函数(基本上哪里是 delete.. 还有 operator<< 无法正常工作,但我相信这是因为那件事,我再次拥有virtual ~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;
}
4

4 回答 4

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();


刚刚根据您的描述在简化版本上进行了测试,以下代码段对我来说非常有效:

#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;
}
于 2017-07-01T10:23:36.430 回答
1

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

    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;

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

于 2017-07-01T10:05:09.023 回答
0

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

在我看来,真正的解决方案是避免使用new. 你会怎么做?大概有3个选项:

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

我猜您实际上并不需要将一家公司分配给另一家公司,同时保留两者。因为您的赋值运算符和复制构造函数语义是错误的:您正在使一家公司的房间可以通过另一家公司进行修改,这应该是const。您可能只是执行移动构造函数

Company::Company(Company&& company);

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

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

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

要获得我的建议的更多灵感,请考虑阅读有关零规则的内容,例如在这篇博文中。

PS - 你确定你需要预定的房间吗?如果没有,请使用std::unordered_set代替std::set.

于 2017-07-01T19:12:01.843 回答
0

如果您只想删除所有内容,那么有一个更简单的解决方案。先删除所有内容,然后调用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而不是原始指针。

于 2017-07-01T10:32:12.597 回答