"Delete p; p = NULL (nullptr);" антипаттерн?

Поиск чего-то на SO, я наткнулся на этот вопрос и один из комментариев к самому проголосовавшему ответу (пятый комментарий к этому большинству голосов) предполагает, что delete p; p = NULL; является антипаттерном. Должен признаться, что я иногда использую его часто, иногда его обманывая\большую часть времени с помощью чека if (NULL != p). Сам человек, кажется, предлагает его (см. пример функции destroy()), поэтому я действительно смущен тем, почему это может быть такой страшную вещь, которую можно считать антипаттером. Я использую его по следующим причинам:

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

Обратите внимание, что я не задаю вопрос в контексте указателя "this", и предположим, что мы не живем в идеальном мире С++ и этот старый код существует, и он должен поддерживаться, поэтому, пожалуйста, не предлагайте никаких умных указателей:).

Ответы

Ответ 1

Да, я бы не рекомендовал это делать.

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

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

Кроме того, он может скрыть другие ошибки, например, если ваше приложение пытается удалить указатель много раз, установив указатель на нуль, эффект заключается в том, что второе удаление будет преобразовано в no-op, и в то время как приложение не будет терпеть крах, ошибка в логике все еще существует (подумайте, что более поздний отредактирует доступ к указателю прямо перед delete, который не прерывается... как это может произойти?

Ответ 2

Я рекомендую сделать это.

  • Очевидно, что он действителен в контексте, где NULL является допустимым значением указателя. Это, конечно, означает, что если оно используется в других местах, оно должно быть проверено.
  • Даже если указатель может, технически, не быть NULL, он действительно помогает в реальных сценариях, когда клиенты отправляют вам аварийный свал. Если он NULL и он не должен быть (и он не был пойман в ловушку при тестировании с помощью assert(), который вы должны сделать), тогда легко увидеть, что это проблема - вы столкнетесь с чем-то вроде mov eax, [edx + 4] или что-то, вы увидите, что edx равно 0, и вы знаете, в чем проблема. Если, с другой стороны, вы не NULL указатель, но он удален, тогда могут произойти всевозможные вещи. Это может сработать, оно может сработать немедленно, оно может упасть позже, это может показаться странным - на данный момент все, что происходит, является мягким - недетерминированным.
  • Оборонительное программирование - это король. Это включает в себя установку указателя на NULL по-другому, даже если вы считаете, что вам этого не нужно, и выполнение этой дополнительной проверки NULL в нескольких местах, даже если вы технически знаете, что это не должно произойти.
  • Даже если у вас есть логическая ошибка указателя, проходящего через удаление дважды, лучше не сбрасывать и не обрабатывать его безопасно, чем сбой. Это может означать, что вы делаете эту дополнительную проверку, и вы, вероятно, захотите зарегистрировать ее или, может быть, даже закончить программу изящно, но это не просто сбой. Клиентам это не нравится.

Лично я использую встроенную функцию, которая берет указатель в качестве ссылки и устанавливает его в NULL.

Ответ 3

Нет, это не анти-шаблон.

Установка указателя на NULL - отличный способ показать, что указатель больше не указывает на что-либо действительное. Фактически, именно это значение означает значение NULL.

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

Ответ 4

Это зависит от того, как используется указатель. Если весь код, который может видеть указатель, должен "знать", что он уже недействителен - особенно в деструкторе, где указатель уже выходит из области видимости, нет необходимости устанавливать его в null.

С другой стороны, указатель может представлять объект, который иногда существует, иногда нет, и у вас есть код типа if (p) { p->doStuff(); }, чтобы воздействовать на объект, когда он существует. В этом случае, очевидно, вы должны установить значение null после удаления объекта.

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

Ответ 5

По-моему, анти-шаблон не delete p; p = NULL;, он assert(this != NULL).

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

Ответ 6

Хотя я думаю, что @Mark Ransom - это своего рода правильный трек, я бы предложил там еще более фундаментальную проблему, чем просто assert(this!=NULL).

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

Даже если стандартные контейнеры не соответствуют вашим потребностям, вы все равно можете превратить ваш указатель в достаточно маленький, достаточно простой пакет, что такие методы не имеют значения - вы ограничили доступ к указанному указателю к такому небольшому количеству кода, который вы можете заглянуть в него, и убедитесь, что он когда-либо использовался, когда он действителен. Как сказал Хоар, есть два способа сделать что-то: нужно держать код таким простым, очевидно, нет недостатков; другой - сделать код настолько сложным, что нет очевидных недостатков. Этот метод кажется мне актуальным, когда вы уже имеете дело с последним случаем.

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

Ответ 7

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

Нужно исправить этот оператор, поскольку в С++ он неверен.

Другие функции разрушаемого объекта могут быть вызваны, когда объект уничтожается (потому что так или иначе требуется процесс уничтожения). Обычно он считается уродливым, но не всегда хорошая работа вокруг.

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

Тем не менее, хорошая идея в С++ заключается в использовании интеллектуальных объектов (возможно, о чем вы говорите). Более или менее класс, который содержит ссылку на объект и который гарантирует, что указанный объект будет выпущен при ударе деструктора, а не добавит много объектов, чтобы уничтожить все сразу в одном объекте (хотя результат тот же, он более чистый.)