Как очистить std :: set из указателей объектов?
У меня проблема с очисткой моего набора, поэтому у меня есть 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 .. Кроме того,<< не работает должным образом, но я считаю, что это из-за этой вещи, опять же у меня есть 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;
}
Решение
Проблема в том, что 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;
в вашей программе. Так что вам не нужно заботиться о правильном освобождении объектов вообще.
Другие решения
Ключевой проблемой вашего подхода является тот факт, что вы модифицируете свой контейнер, перебирая его. Я бы посоветовал изменить его на:
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;
}
Если вы просто хотите удалить все, есть гораздо более простое решение. Сначала удалите все, затем позвоните 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 вместо сырых указателей.
Я считаю, что ваш вопрос является примером XY проблема: Вы хотите очистить свой набор указателей, но вы хотите это только потому, что хотите вручную уничтожить этот набор без утечки памяти. И вам нужно сделать это только потому, что автоматическое уничтожение этого не сделает.
реальный Решение, как я понимаю, состоит в том, чтобы избежать ручного распределения с new, Что бы вы сделали вместо этого? Есть, вероятно, 3 варианта:
unique_ptr«sshared_ptr«sstd::variant«s
Я предполагаю, что вам не нужно назначать одну компанию другой, сохраняя обе; поскольку Ваш оператор присваивания и семантика конструктора копирования неправильно: вы делаете комнаты одной компании изменяемыми через другую компанию, которая должна быть const, Вы, вероятно, просто выполняете переместить конструктор:
Company::Company(Company&& company);
в котором вы «захватите» набор комнаты старой компании, оставив его пустым (и, возможно, оператором перемещения). Если это так — у вас есть только одна ссылка на комнату, и unique_ptr<EscapeRoomWrapper> Сделаю. Вам не нужно ничего удалять вручную, и деструктор по умолчанию для набора будет работать очень хорошо. На самом деле, вы можете просто удалить пользовательский конструктор перемещения и просто использовать значение по умолчанию.
если ты делать нужно, чтобы несколько компаний ссылались на одну и ту же комнату, и нужны эти не имеющие значения конструктор и оператор присваивания — используйте shared_ptr<EscapeRoomWrapper>, Опять же, вам не нужно ничего удалять, и комнаты будут удалены, когда будет уничтожена последняя ссылка на них. Это будет стоить вам немного накладных расходов, но в любом случае это не критичный для производительности код, поэтому это не имеет значения.
Наконец, если количество комнат ограничено, и все они наследуются от класса-оболочки, вы можете подумать о замене виртуального наследования на std::variant из различных классов комнаты, и не используя указатель вообще — просто наличие набора комнаты.
Для дополнительного вдохновения для моих предложений, подумайте о прочтении правила нуля, например, в этот блог.
PS — Вы уверены, что вам нужно заказать номер комнаты? Если нет, используйте std::unordered_set вместо std::set,