Должны ли объекты удалять себя в C++?

https://stackoverflow.com/questions/522637

22-08-2019
|

Вопрос

Последние 4 года я изучал C#, поэтому меня интересуют текущие лучшие практики и распространенные шаблоны проектирования на C++.Рассмотрим следующий частичный пример:

class World
{
public:
    void Add(Object *object);
    void Remove(Object *object);
    void Update();
}

class Fire : Object
{
public:
    virtual void Update()
    {
        if(age > burnTime)
        {
            world.Remove(this);
            delete this;
        }
    }
}

Здесь у нас есть мир, отвечающий за управление набором объектов и регулярное их обновление.Огонь — это объект, который может быть добавлен в мир при самых разных обстоятельствах, но обычно с помощью другого объекта, уже существующего в мире.Огонь — единственный объект, который знает, когда он сгорел, поэтому сейчас я удаляю его.Объект, вызвавший пожар, скорее всего, больше не существует и не имеет значения.

Разумно ли это или есть лучший дизайн, который поможет очистить эти объекты?

Решение

Проблема в том, что вы на самом деле создаете неявную связь между объектом и классом World.

Если я попытаюсь вызвать Update() вне класса World, что произойдет?В конечном итоге объект может быть удален, и я не знаю, почему.Кажется, обязанности сильно перепутаны.Это вызовет проблемы в тот момент, когда вы будете использовать класс Fire в новой ситуации, о которой вы не подумали, когда писали этот код.Что произойдет, если объект необходимо удалить более чем из одного места?Возможно, его стоит удалить и из мира, и с текущей карты, и из инвентаря игрока?Ваша функция обновления удалит его из мира, а затем удалит объект, и в следующий раз, когда карта или инвентарь попытаются получить доступ к объекту, произойдут плохие вещи.

В общем, я бы сказал, что для функции Update() очень неинтуитивно удалять обновляемый объект.Я бы также сказал, что удаление объекта не интуитивно понятно.Объект, скорее всего, должен иметь какой-то способ инициировать событие, сообщающее, что он закончил горение, и теперь любой желающий может действовать в соответствии с этим.Например, удалив его из мира.Чтобы удалить его, подумайте о праве собственности.

Кому принадлежит объект?Мир?Это означает, что только мир может решить, когда объект умрет.Это нормально, пока ссылка мира на объект переживет другие ссылки на него.Вы думаете, что объект владеет самим собой?Что это вообще значит?Объект следует удалить, когда объект больше не существует?Это не имеет смысла.

Но если нет четко определенного единого владельца, реализуйте совместное владение, например, с помощью интеллектуального указателя, реализующего подсчет ссылок, например boost::shared_ptr

Но наличие функции-члена самого объекта, которая жестко запрограммирована для удаления объекта из один конкретный список, существует он там или нет, и существует он также в каком-либо другом списке, а также удалять сам объект независимо от того, какие ссылки на него существуют, — плохая идея.

Другие советы

Вы сделали Update виртуальная функция, предполагающая, что производные классы могут переопределять реализацию Update.Это таит в себе два больших риска.

1.) Переопределенная реализация может не забыть выполнить World.Remove, но может забыть delete this.Память утекла.

2.) Переопределенная реализация вызывает базовый класс Update, что делает delete this, но затем продолжает работу, но с недопустимым указателем this.

Рассмотрим этот пример:

class WizardsFire: public Fire
{
public:
    virtual void Update()
    {
        Fire::Update();  // May delete the this-object!
        RegenerateMana(this.ManaCost); // this is now invaild!  GPF!
    }
}

В C++ нет ничего плохого в том, что объекты удаляют сами себя, большинство реализаций подсчета ссылок используют нечто подобное.Однако это считается немного эзотерической техникой, и вам нужно будет очень внимательно следить за вопросами владения ею.

Удаление завершится неудачей и может привести к сбою программы, если объект был нет выделено и построено новое.Эта конструкция не позволит вам создать экземпляр класса в стеке или статически.В вашем случае вы, похоже, используете какую-то схему распределения.Если вы будете придерживаться этого, это может быть безопасно.Хотя я бы, конечно, этого не сделал.

Я предпочитаю более строгую модель владения.Мир должен владеть объектами в нем и нести ответственность за их очистку.Либо попросите world delete сделать это, либо update (или другая функция) вернет значение, указывающее, что объект следует удалить.

Я также предполагаю, что в этом типе шаблона вам понадобится подсчитывать ссылки на ваши объекты, чтобы избежать висящих указателей.

Это определенно работает для объектов, созданных new и когда вызывающий абонент Update надлежащим образом проинформирован о таком поведении.Но я бы избегал этого.В вашем случае право собственности явно принадлежит миру, поэтому я бы заставил мир удалить его.Объект не создает себя, я думаю, он также не должен удалять себя.Может быть очень удивительно, если вы вызываете функцию «Обновить» для своего объекта, но затем внезапно этот объект больше не существует, и Мир ничего не делает из себя (кроме удаления его из своего списка - но в другом кадре!Код, вызывающий Update объекта, этого не заметит).

Некоторые идеи по этому поводу

Добавьте список ссылок на объекты в World.Каждый объект в этом списке ожидает удаления.Это распространенный метод, который используется в wxWidgets для окон верхнего уровня, которые были закрыты, но все еще могут получать сообщения.Во время простоя, когда все сообщения обработаны, обрабатывается список ожидающих сообщений и объекты удаляются.Я считаю, что Qt следует аналогичной методике.
Сообщите миру, что объект хочет быть удален.Мир будет должным образом проинформирован и позаботится обо всем, что необходимо сделать.Что-то вроде deleteObject(Object&) может быть.
Добавить shouldBeDeleted для каждого объекта, которая возвращает true, если объект желает удалить его владелец.

Я бы предпочел вариант 3.Мир назвал бы Update.И после этого он смотрит, следует ли удалить объект и может ли он это сделать, или, если он того пожелает, он запоминает этот факт, добавляя этот объект в список ожидающих удаления вручную.

Это заноза в заднице, когда вы не можете быть уверены, когда и когда вы не сможете получить доступ к функциям и данным объекта.Например, в wxWidgets есть класс wxThread, который может работать в двух режимах.Один из этих режимов (называемый «отделяемым») заключается в том, что если его основная функция возвращается (и ресурсы потока должны быть освобождены), он удаляет себя (чтобы освободить память, занятую объектом wxThread), вместо того, чтобы ждать владельца потока. объект для вызова функции ожидания или соединения.Однако это вызывает сильную головную боль.Вы никогда не сможете вызвать какие-либо функции для него, потому что он может быть прекращен при любых обстоятельствах, и вы не можете создать его без нового.Многие люди говорили мне, что им очень не нравится такое поведение.

Самоудаление объекта с подсчетом ссылок пахнет, имхо.Давайте сравним:

// bitmap owns the data. Bitmap keeps a pointer to BitmapData, which
// is shared by multiple Bitmap instances. 
class Bitmap {
    ~Bitmap() {
        if(bmp->dec_refcount() == 0) {
            // count drops to zero => remove 
            // ref-counted object. 
            delete bmp;
        }
    }
    BitmapData *bmp;
};

class BitmapData {
    int dec_refcount();
    int inc_refcount();

};

Сравните это с самоудалением пересчитанных объектов:

class Bitmap {
    ~Bitmap() {
        bmp->dec_refcount();
    }
    BitmapData *bmp;
};

class BitmapData {
    int dec_refcount() {
        int newCount = --count;
        if(newCount == 0) {
            delete this;
        }
        return newCount;
    }
    int inc_refcount();
};

Я думаю, что первый вариант намного лучше, и я считаю, что хорошо спроектированные объекты с подсчетом ссылок делают это. нет сделайте «удалить это», потому что это увеличивает связь:Класс, использующий данные подсчета ссылок, должен знать и помнить о том, что данные удаляются сами собой как побочный эффект уменьшения счетчика ссылок.Обратите внимание, что «bmp» становится висячим указателем в деструкторе ~Bitmap.Возможно, здесь гораздо лучше не делать этого «удалить это».

Ответ на аналогичный вопрос «Какой смысл удалять это»

Это довольно распространенная реализация подсчета ссылок, и я успешно использовал ее раньше.

Однако я также видел, как он терпел крах.Мне хотелось бы вспомнить обстоятельства крушения. @abelenky это одно место, где я видел, как он разбился.

Возможно, это было то место, где вы создали дополнительный подкласс Fire, но не удалось создать виртуальный деструктор (см. ниже).Если у вас нет виртуального деструктора, Update() функция вызовет ~Fire() вместо соответствующего деструктора ~Flame().

class Fire : Object
{
public:
    virtual void Update()
    {
        if(age > burnTime)
        {
            world.Remove(this);
            delete this; //Make sure this is a virtual destructor.
        }
    }
};

class Flame: public Fire
{
private: 
    int somevariable;
};

Другие упомянули проблемы с «удалить это». Короче говоря, поскольку «мир» управляет созданием огня, он также должен управлять своим удалением.

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

Для желаемой семантики у меня был бы флаг «активный» или «живой» в вашем классе «Fire» (или Object, если применимо).Тогда мир время от времени проверял бы свою инвентаризацию объектов и избавлялся от тех, которые больше не активны.

Еще одно замечание: ваш написанный код имеет частное наследование Fire от Object, поскольку это значение по умолчанию, хотя оно гораздо менее распространено, чем публичное наследование.Вероятно, вам следует явно указать тип наследования, и я подозреваю, что вам действительно нужно публичное наследование.

Обычно не рекомендуется выполнять «удалить это», если только это не необходимо или не используется очень простым способом.В вашем случае похоже, что World может просто удалить объект при его удалении, если только нет других зависимостей, которые мы не видим (в этом случае ваш вызов удаления вызовет ошибки, поскольку они не уведомляются).Сохранение владения за пределами вашего объекта позволяет вашему объекту быть лучше инкапсулированным и более стабильным:объект не станет недействительным в какой-то момент просто потому, что он решил удалить себя.

Я не считаю, что в удалении объекта есть что-то неправильное, но другим возможным методом было бы возложить на мир ответственность за удаление объектов как часть ::Remove (при условии, что все удаленные объекты также были удалены).

удалять это очень рискованно.В этом нет ничего «неправильного».Это законно.Но есть так много вещей, которые могут пойти не так, когда вы его используете, поэтому его следует использовать с осторожностью.

Рассмотрим случай, когда у кого-то есть открытые указатели или ссылки на объект, а затем он удаляется.

Я думаю, что в большинстве случаев срок жизни объекта должен управляться тем же объектом, который его создает.Это просто облегчает понимание того, где происходит разрушение.

Лицензировано под: CC-BY-SA с атрибуция

Не связан с StackOverflow