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

Я провел последние 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;
        }
    }
}

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

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


person Generic Error    schedule 06.02.2009    source источник


Ответы (11)


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

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

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

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

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

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

person jalf    schedule 06.02.2009
comment
Мне было сложно выбрать здесь приемлемый ответ. Я выбираю это, потому что он описывает проблемы, которые у меня возникли (вещи, отличные от мира, хранящего ссылки), а также предлагает shared_ptr, который стал частью моего решения. Спасибо. - person Generic Error; 07.02.2009
comment
Другая причина, по которой удаление self - это плохо, заключается в том, что оно препятствует созданию объекта в стеке, а не в куче. Кроме того, подключение к World делает невозможным независимое тестирование. - person walrii; 26.07.2012

Вы сделали 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!
    }
}
person abelenky    schedule 06.02.2009
comment
Я видел это! Пытаться выследить - неинтересно. - person Adam Tegen; 07.02.2009
comment
Ой, это не весело. Спасибо, что указали на это, 2 проблемы, которых мы (напрямую) не имеем в C #. - person Generic Error; 07.02.2009

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

person 1800 INFORMATION    schedule 06.02.2009

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

person Rob deFriesse    schedule 06.02.2009

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

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

person hacken    schedule 06.02.2009

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

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

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

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

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

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

// 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. Возможно, здесь гораздо лучше не делать «удалить это».

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

person Johannes Schaub - litb    schedule 06.02.2009
comment
Мне нравится этот ответ, и в итоге я использовал его элементы, но, к сожалению, могу принять только один. Спасибо! - person Generic Error; 07.02.2009
comment
Я рад, что мы вам помогли :) Думаю, у jalf есть несколько действительно хороших идей в своем ответе. развлекайся :) - person Johannes Schaub - litb; 07.02.2009

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

Однако я также видел, как он разбился. Хотел бы я вспомнить обстоятельства крушения. @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;
};
person Adam Tegen    schedule 06.02.2009
comment
Вероятно, после вызова удаления вы попытались получить доступ к переменной-члену или вызвали виртуальную функцию - person 1800 INFORMATION; 07.02.2009
comment
отличная иллюстрация необходимости виртуальных деструкторов. Модернизация вас. - person abelenky; 07.02.2009

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

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

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

--

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

person JohnMcG    schedule 07.02.2009
comment
Мы не зажигали огонь Он всегда горел С тех пор, как мир вращался Мы не зажигали огонь Нет, мы его не зажигали Но мы пытались с ним бороться - person abelenky; 07.02.2009
comment
Эта отсутствующая публика связана с тем, что мой мозг все еще находится в режиме C # ... я не думал, как удалить огонь, если мир кончится первым ... это звучит довольно серьезно! - person Generic Error; 07.02.2009

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

person Ron Warholic    schedule 06.02.2009
comment
Если вы имеете в виду World :: Remove (Object * o) {delete o}, то это плохо, поскольку метод Remove был вызван из удаляемого объекта. Это означает, что он вернется к методу жалости, которой больше не существует. - person KeithB; 07.02.2009

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

person Yes - that Jake.    schedule 06.02.2009

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

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

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

person Erik Funkenbusch    schedule 07.02.2009