Нашли или выдавили из себя код, который нельзя назвать нормальным,
на который без улыбки не взглянешь?
Не торопитесь его удалять или рефакторить, — запостите его на
говнокод.ру, посмеёмся вместе!
void Exf2dMW::closeEvent(QCloseEvent* event)
{
int win_num = _winManager->numberOfModellingWindows();
// if more than one modelling windows, just destroy current one:
if (win_num > 1) {
event->accept();
delete this;
}
// if only one window and handle closing model properly, then destroy it:
else {
if( handleCloseModel() ) {
event->accept();
delete this;
delete _winManager;
}
else
event->ignore();
}
}
Еще кусочек говнокода коллеги, я не думал что так можно писать..
если операторы работы с кучей и сама куча правильно реализована, то она не будет удалять не свои аллоки:
unsigned char buf[] = { 0x01, 0x02, 0x03, 0x04 };
delete buf; printf("0x%08X - 0x%08X\n", buf, *((unsigned int*)buf));
delete this бывало встречал в коммерческих проектах... по большому счету криминальное здесь только delete локальной переменной... хотя в принципе тоже должно работать, если this был не последним выделенным аллоком на странице, и если между delete this и delete локальной переменной не встрянет другой поток, который перетрет ее значение...
Можно. Без видения полной картины утверждать наверняка нельзя, но скорее всего это не ошибка и не говнокод. Может быть даже где-то в другом месте документирована такая стратегия.
Можно предположить, что есть менеджер окон, есть несколько окон, на каждое навешен обработчик события закрытия, размещённый в динамической памяти. Если окон несколько -- event->accept() закрывает сответствующее окно, обработчик больше ни с чем не связан и не нужен, удаляется. Если осталось последнее окно, handleCloseModel() запрашивает у пользователя подтверждение и либо закрывает это окно и удаляет уже не нужный менеджер, либо игнорирует событие. Всё нормально.
ну мы видели полную картину и оно крашилось 🙂 closeEvent() - это стандартный евент-хэндлер в Qt, и в документации уж точно не предусмотрено удалять объект во время обработки события. Для этого в Qt есть функции типа deleteLater(). Ну а менеджер окон тоже относится к клиентскому коду. Вы описали правильное поведение, но оно не должно достигаться удалением объекта самим себя. Для этого нужно посылать сигнал менеджеру окон и он сам удалит последнее окно, а затем и себя.
Ага, closeEvent() -- это переопределённый метод виджета, а не стороннего обработчика. Проверьте, установлен ли флаг Qt::WA_DeleteOnClose, если установлен, то delete удаляет уже удалённый объект, если нет, то вроде ничего плохого произойти не должно.
стоит ли подобное использовать - каждому свое
но структуру системы проще понять и поддерживать, если сущности создаются и удаляются там, где они используются, а не так что создадим здесь, тут что-то там вызовем (явно не деструктор) и, опа, оно уже удалено:(
а ещё "delete this;" приводит к проблемам при множественно наследовании:
class A { TypeA fieldA; ... };
class B { public: TypeB fieldB; void DeleteB() { delete this; } ... };
class C : A, public B { ... };
...
C *c = new C();
C->DeleteB();
всё равно что сделать так:
void Fn_DeleteB(B *b) { delete b; }
Fn_DeleteB(new C());
метод DeleteB() получит в качестве this не указатель на (*c), а указатель на с->fieldB, т.е. не на начало области освобождаемой памяти.
Даже если наследоваться никто не будет, не думаю что так стоит писать (
Бестселлер «Эскхатологические методы в программировании».
А вообще, интересная проблема о нахождении внутри несуществующего объекта — на самом стыке программирования и философии!
То, что вы тут написали не имеет никакого отношения ни к `delete this`, ни к множественному наследованию.
В языке С++ выполнять полиморфное удаление объекта (т.е удаление через указатель на базовый подобъект) разрешается только в том случае, если этот базовый подобъект имеет виртуальный деструктор. Вы в своем коде нарушили это правило, поэтому Ваш код порождает неопределенное поведение (а уж какой там указатель прийдет в `delete` - на `fieldB` или еще куда - это бесполезное гадание на кофейной гуще).
Ваш код ничем принципиально не оличается от обычного
struct B {};
struct D : B {};
...
B *b = new D;
delete b; // <- ОБЛОМИСЬ: неопределенное поведение
и, еще раз, ни к `delete this`, ни к множественному наследованию никакого прямого отношения не имеет.
Добавьте в класс `B` виртуальный деструктор, чтобы соблюсти требования языка С++, и сразу все станет работать нормально, даже с множественным наследованием 🙂
`delete this` - я ни какой конкретной инфы не видел почему так делать нельзя/что так делать можно. при условии что к объекту больше обращений не происходит, проблем возникать не должно. не забываем про мнимый `this->` перед обращением к методам и членам класа. и я видел места где этим народ пользуется.
с другой стороны, я лицезрел подобную фишку в одной системе - удаление объекта получателя сообщения внутри обработчика этого же сообщения. не так откровенно, но тем неменее. и я видел как это весьма спектакулярно и произвольно валило приложение весьма разнообразными способами: фреймворк пытался что-то там еще с этим объектом делать в эпилоге кода диспатча сообщений. имеет смысл на мэйл листах вашего фреймворка поспрашивать.
`delete this` - устоявшаяся широко распространенная идиома. Ничего нелегального с точи зрения языка в ней нет. И никакой говнокодовости в ней нет. Надо просто уметь ею пользоваться и думать, что делаешь. Злоупотреблять, разумеется, ею не стоит, как и всем остальным в С++.
Аргументы типа "объект может быть на стеке" - бред сивой кобылы. Если я, автор объекта, сказал, что объект не может быть "на стеке", значит от там быть не может. А если кто-то умудрится его там создать - вот тот пусть потом и расхлебывает возникшие проблемы. (Добро пожаловать в С++!)
Разумеется, если объекты предназначены исключительно для динамческого создания, то не составит труда исключить их статическое создание или локальное создание. Способов достичь этого много. (Static constructor idiom - лишь один пример).
Постить вырванный из контекста код с `delete this` и утверждать что из-за этого он является говнокодом - дилетантство самого низкого пошиба.
Потенциал для говнокодовости в этом примере, конечно, есть. В частности, имете место обращение к некоему `_winManager` после `delete this`. Если `_winManager` - поле уже удаленного объекта , то мы действительно имеем дело с говнокодом. Но из примера не видно, откуда берется этот `_winManager`, поэтому судить невозможно.
`delete this` - устоявшаяся широко распространенная идиома.
Я бы не сказал что устоявшаяся и широко распространенная. Как тут уже подметили, создание и удаление объекта должно быть подконтрольно одной сущности, а так как очевидно, что объект не может создать себя сам, неправы тут Вы. И не надо кидаться выражениями, типа "дилетанство самого низкого пошиба" или "бред сивой кобылы".
Надо почитать про это, но в любом случае, метод называется destroy - я не сильно понимаю необходимость этого метода, так как использование виртуального деструктора и оператора delete дает тот же самый эффект.
Да, перегрузка операторов new и delete, конструктор и деструктор позволяют решить большинство задач. Но иногда нужно уничтожать объект небезусловно, и тогда приходится использовать другой интерфейс.
Например, объект с внутренним счётчиком ссылок. Метод grab увеличивает счётчик на 1, метод ungrab — уменьшает, и, если он достиг 0, удаляет объект. Это эффективнее smart_ptr.
Или же create при некоторых условиях берёт уже созданный объект из готового статического пула, а destroy, если объект был из пула, возвращает его туда, не разрушая.
Сайт, сборник статей (и не только). Автор пишет "о языках программирования". Статьи больше "поржать", хотя есть интересные моменты... Вот уж не знаю, что автор курит для снятия стресса...
Где Вы такое находите...
"Здравствуйте, меня зовут Николай Валуев. Немного о себе. Сейчас я профессиональный боксёр. Также у меня есть рот. И ещё я туда ем."
Может пригласить автора на ГК?
unsigned char buf[] = { 0x01, 0x02, 0x03, 0x04 };
delete buf; printf("0x%08X - 0x%08X\n", buf, *((unsigned int*)buf));
delete this бывало встречал в коммерческих проектах... по большому счету криминальное здесь только delete локальной переменной... хотя в принципе тоже должно работать, если this был не последним выделенным аллоком на странице, и если между delete this и delete локальной переменной не встрянет другой поток, который перетрет ее значение...
Можно предположить, что есть менеджер окон, есть несколько окон, на каждое навешен обработчик события закрытия, размещённый в динамической памяти. Если окон несколько -- event->accept() закрывает сответствующее окно, обработчик больше ни с чем не связан и не нужен, удаляется. Если осталось последнее окно, handleCloseModel() запрашивает у пользователя подтверждение и либо закрывает это окно и удаляет уже не нужный менеджер, либо игнорирует событие. Всё нормально.
но структуру системы проще понять и поддерживать, если сущности создаются и удаляются там, где они используются, а не так что создадим здесь, тут что-то там вызовем (явно не деструктор) и, опа, оно уже удалено:(
о майне факен готт.......
class A { TypeA fieldA; ... };
class B { public: TypeB fieldB; void DeleteB() { delete this; } ... };
class C : A, public B { ... };
...
C *c = new C();
C->DeleteB();
всё равно что сделать так:
void Fn_DeleteB(B *b) { delete b; }
Fn_DeleteB(new C());
метод DeleteB() получит в качестве this не указатель на (*c), а указатель на с->fieldB, т.е. не на начало области освобождаемой памяти.
Даже если наследоваться никто не будет, не думаю что так стоит писать (
не думаю, что корректно удалять себя, т.к. получится, что мы находимся внутри несуществующего объекта
А вообще, интересная проблема о нахождении внутри несуществующего объекта — на самом стыке программирования и философии!
В языке С++ выполнять полиморфное удаление объекта (т.е удаление через указатель на базовый подобъект) разрешается только в том случае, если этот базовый подобъект имеет виртуальный деструктор. Вы в своем коде нарушили это правило, поэтому Ваш код порождает неопределенное поведение (а уж какой там указатель прийдет в `delete` - на `fieldB` или еще куда - это бесполезное гадание на кофейной гуще).
Ваш код ничем принципиально не оличается от обычного
и, еще раз, ни к `delete this`, ни к множественному наследованию никакого прямого отношения не имеет.
Добавьте в класс `B` виртуальный деструктор, чтобы соблюсти требования языка С++, и сразу все станет работать нормально, даже с множественным наследованием 🙂
с другой стороны, я лицезрел подобную фишку в одной системе - удаление объекта получателя сообщения внутри обработчика этого же сообщения. не так откровенно, но тем неменее. и я видел как это весьма спектакулярно и произвольно валило приложение весьма разнообразными способами: фреймворк пытался что-то там еще с этим объектом делать в эпилоге кода диспатча сообщений. имеет смысл на мэйл листах вашего фреймворка поспрашивать.
Аргументы типа "объект может быть на стеке" - бред сивой кобылы. Если я, автор объекта, сказал, что объект не может быть "на стеке", значит от там быть не может. А если кто-то умудрится его там создать - вот тот пусть потом и расхлебывает возникшие проблемы. (Добро пожаловать в С++!)
Разумеется, если объекты предназначены исключительно для динамческого создания, то не составит труда исключить их статическое создание или локальное создание. Способов достичь этого много. (Static constructor idiom - лишь один пример).
Постить вырванный из контекста код с `delete this` и утверждать что из-за этого он является говнокодом - дилетантство самого низкого пошиба.
Потенциал для говнокодовости в этом примере, конечно, есть. В частности, имете место обращение к некоему `_winManager` после `delete this`. Если `_winManager` - поле уже удаленного объекта , то мы действительно имеем дело с говнокодом. Но из примера не видно, откуда берется этот `_winManager`, поэтому судить невозможно.
Я бы не сказал что устоявшаяся и широко распространенная. Как тут уже подметили, создание и удаление объекта должно быть подконтрольно одной сущности, а так как очевидно, что объект не может создать себя сам, неправы тут Вы. И не надо кидаться выражениями, типа "дилетанство самого низкого пошиба" или "бред сивой кобылы".
Это действительно устоявшаяся широко распространенная идиома. Но применимая в редких конкретных ситуациях, в комплексе с другими средствами.
Например, объект с внутренним счётчиком ссылок. Метод grab увеличивает счётчик на 1, метод ungrab — уменьшает, и, если он достиг 0, удаляет объект. Это эффективнее smart_ptr.
Или же create при некоторых условиях берёт уже созданный объект из готового статического пула, а destroy, если объект был из пула, возвращает его туда, не разрушая.
P.S. в любом случае к подопытному коду это не относится 🙂
>Большинству современных языков программирования не хватет морально-этической и чувственной составляющей...
"Здравствуйте, меня зовут Николай Валуев. Немного о себе. Сейчас я профессиональный боксёр. Также у меня есть рот. И ещё я туда ем."
Может пригласить автора на ГК?