Разумно ли обнулять каждый разыменованный указатель?

65

На новой работе я получаю пометки в обзорах кода для такого кода:

PowerManager::PowerManager(IMsgSender* msgSender)
  : msgSender_(msgSender) { }

void PowerManager::SignalShutdown()
{
    msgSender_->sendMsg("shutdown()");
}

Мне сказали, что последний метод должен читать:

void PowerManager::SignalShutdown()
{
    if (msgSender_) {
        msgSender_->sendMsg("shutdown()");
    }
}

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

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

Разумно ли для стандарта кодирования требовать, чтобы каждый отдельный указатель, разыменованный в функции, проверялся в NULLпервую очередь - даже на частные элементы данных? (Примечание: чтобы дать некоторый контекст, мы делаем устройство бытовой электроники, а не систему управления воздушным движением или какой-либо другой продукт «отказ-равный-людям-умирать».)

РЕДАКТИРОВАТЬ : В приведенном выше примере msgSender_соавтор не является обязательным. Если это когда-либо NULL, это указывает на ошибку. Единственная причина, по которой он передается в конструктор, заключается в том, что PowerManagerего можно протестировать с помощью имитирующего IMsgSenderподкласса.

РЕЗЮМЕ : Было несколько действительно хороших ответов на этот вопрос, спасибо всем. Я принял один из @aaronps главным образом из-за его краткости. Похоже, что существует общее мнение, что:

  1. Обязательная NULLохрана для каждого разыменованного указателя является излишним, но
  2. Вы можете обойти всю дискуссию, используя вместо этого ссылку (если возможно) или constуказатель, и
  3. assertоператоры являются более понятной альтернативой NULLохранникам для проверки выполнения предварительных условий функции.
evadeflow
источник
14
Не думайте ни на минуту, что ошибки - это дело младших программистов. 95% разработчиков всех уровней испытывают что-то одно время от времени. Остальные 5% лежат сквозь зубы.
Blrfl
56
Если бы я видел, как кто-то пишет код, который проверяет на ноль и молча терпит неудачу, я бы хотел уволить их на месте.
Уинстон Эверт
16
Вещи, молча терпящие неудачу, в значительной степени худшие. По крайней мере, когда он взрывается, вы знаете, где происходит взрыв. Проверять nullи ничего не делать - это всего лишь способ переместить ошибку вниз по ходу выполнения, что значительно усложняет поиск источника.
MirroredFate
1
+1, очень интересный вопрос. В дополнение к моему ответу я бы также указал, что когда вам нужно написать код, целью которого является проведение модульного теста, вы действительно торгуете повышенным риском из-за ложного чувства безопасности (больше строк кода = больше возможностей для ошибок). Редизайн для использования ссылок не только улучшает читабельность кода, но также уменьшает объем кода (и тестов), который вы должны написать, чтобы доказать, что он работает.
Сет
6
@Rig, я уверен, что есть подходящие случаи для тихого сбоя. Но если кто-то считает молчание неудачой в качестве общей практики (если не работает в сфере, где это имеет смысл), я действительно не хочу работать над проектом, в который они помещают код.
Уинстон Эверт

Ответы:

66

Это зависит от «контракта»:

Если PowerManager ДОЛЖЕН иметь действительный IMsgSender, никогда не проверять на ноль, пусть он умрет раньше.

Если, с другой стороны, он МОЖЕТ иметь IMsgSender, то вам нужно проверять каждый раз, когда вы используете, так просто.

Заключительный комментарий по поводу истории младшего программиста, проблема на самом деле в отсутствии процедур тестирования.

aaronps
источник
4
+1 за очень лаконичный ответ, который в значительной степени подводит итог тому, что я чувствую: «это зависит ...» У вас также хватило смелости сказать «никогда не проверять на ноль» ( если ноль недействителен). Размещение assert()в каждой функции-члене, которая разыменовывает указатель, является компромиссом, который, вероятно, успокоит многих людей, но даже для меня это похоже на «умозрительную паранойю». Список вещей за пределами моей способности контролировать бесконечен, и как только я позволю себе начать беспокоиться о них, это станет действительно скользким спуском. Научившись доверять моим тестам, мои коллеги и мой отладчик помогли мне лучше спать по ночам.
evadeflow
2
Когда вы читаете код, эти утверждения могут быть очень полезны для понимания того, как код должен работать. Я никогда не воспринимал кодовую базу, которая заставляла меня говорить: «Хотелось бы, чтобы в ней не было всех этих утверждений повсюду», но я много видел, где я говорил обратное.
Кристоф Провост
1
Просто и понятно, это правильный ответ на вопрос.
Мэтью Азкимов
2
Это самый близкий правильный ответ, но я не могу согласиться с «никогда не проверять на ноль», всегда проверяйте недопустимые параметры в том месте, где они будут назначены переменной-члену.
Джеймс
Спасибо за комментарии. Лучше позволить ему аварийно завершить работу, если кто-то не прочитает спецификацию и инициализирует ее нулевым значением, когда там должно быть что-то допустимое.
Ааронпс
77

Я чувствую код должен читать:

PowerManager::PowerManager(IMsgSender* msgSender)
  : msgSender_(msgSender)
{
    assert(msgSender);
}

void PowerManager::SignalShutdown()
{
    assert(msgSender_);
    msgSender_->sendMsg("shutdown()");
}

Это на самом деле лучше, чем защита NULL, потому что очень ясно дает понять, что функция никогда не должна вызываться, если msgSender_NULL. Это также гарантирует, что вы заметите, если это произойдет.

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

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

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

Кристоф Провост
источник
10
Большая разница между вызовом assert и примером кода OP заключается в том, что assert включается только в сборках отладки (#define NDEBUG), когда продукт попадает в руки клиентов, и отладка отключена, и выполняется комбинация путей кода, которая никогда не встречается, выполняется, чтобы оставить нулевой указатель, хотя рассматриваемая функция выполняется, assert не предоставляет вам никакой защиты.
atk
17
Предположительно, вы бы проверили сборку перед тем, как отправлять ее клиенту, но да, это возможный риск. Решения, однако, просты: либо просто поставьте с включенными утверждениями (это версия, которую вы все равно протестировали), либо замените его собственным макросом, который утверждает в режиме отладки, и просто регистрирует, регистрирует + обратные пути, завершает работу ... в режиме выпуска.
Кристоф Провост
7
@James - в 90% случаев вы также не восстановитесь после неудачной проверки NULL. Но в таком случае код молча завершится ошибкой, и ошибка может появиться намного позже - например, вы подумали, что сохранили в файл, но на самом деле проверка на ноль иногда не удается, оставляя вас не мудрее. Вы не можете оправиться от любой ситуации, которая не должна произойти. Вы можете убедиться, что они не произойдут, но я не думаю, что у вас есть бюджет, если вы не напишите код для спутника НАСА или атомной электростанции. Вы должны иметь способ сказать: «Я понятия не имею, что происходит - программа не должна находиться в этом состоянии».
Мацей Пехотка
6
@ Джеймс Я не согласен с броском. Это делает его похожим на то, что действительно может произойти. Утверждения для вещей, которые должны быть невозможными. Для реальных ошибок в коде другими словами. Исключения составляют непредвиденные, но возможные события. Исключения составляют вещи, которые вы могли бы себе представить и восстановить. Логические ошибки в коде, которые вы не можете исправить, и вы не должны пытаться.
Кристоф Провост
2
@James: По моему опыту работы со встроенными системами программное и аппаратное обеспечение всегда разрабатывалось таким образом, что крайне трудно сделать устройство полностью непригодным для использования. В большинстве случаев существует аппаратный сторожевой таймер, который принудительно завершает полный сброс устройства, если программное обеспечение прекращает работу.
Барт ван Инген Шенау
30

Если msgSender никогда не должен быть null, вы должны поставить nullпроверку только в конструкторе. Это также верно для любых других входных данных в классе - поставить проверку целостности в точке входа в «модуль» - класс, функцию и т. Д.

Мое эмпирическое правило состоит в том, чтобы выполнять проверки целостности между границами модуля - класс в данном случае. Кроме того, класс должен быть достаточно маленьким, чтобы можно было быстро мысленно проверить целостность времени жизни членов класса, гарантируя, что такие ошибки, как неправильное удаление / нулевые назначения, будут предотвращены. Нулевая проверка, которая выполняется в коде в вашем посте, предполагает, что любое недопустимое использование фактически присваивает указателю нулевое значение, что не всегда так. Поскольку «недопустимое использование» по своей сути подразумевает, что любые предположения о обычном коде не применяются, мы не можем быть уверены, что отловим все типы ошибок указателей - например, недопустимое удаление, приращение и т. Д.

Кроме того - если вы уверены, что аргумент никогда не может быть нулевым, рассмотрите возможность использования ссылок, в зависимости от вашего использования класса. В противном случае рассмотрите возможность использования std::unique_ptrили std::shared_ptrвместо необработанного указателя.

Максимум
источник
11

Нет, нет смысла проверять разыменование каждого указателя на наличие указателя NULL.

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

Если бы я столкнулся с ситуацией, подобной вашей, я бы задал два вопроса:

  • Почему ошибка этого младшего программиста не была поймана до релиза? Например, в обзоре кода или на этапе тестирования? Вывод неисправного устройства бытовой электроники на рынок может быть столь же дорогостоящим (если принять во внимание долю рынка и доброжелательность), как выпуск неисправного устройства, используемого в критически важном для безопасности приложении, поэтому я ожидаю, что компания будет серьезно относиться к тестированию и другие мероприятия по обеспечению качества.
  • Если проверка на ноль не удалась, какую обработку ошибок вы ожидаете от меня, или я мог бы просто написать assert(msgSender_)вместо проверки на ноль? Если вы просто установите нулевую отметку, вы, возможно, предотвратили сбой, но вы могли создать худшую ситуацию, потому что программное обеспечение продолжает исходить из того, что операция была выполнена, хотя в действительности эта операция была пропущена. Это может привести к нестабильной работе других частей программного обеспечения.
Барт ван Инген Шенау
источник
9

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

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

PowerManager::PowerManager(const IMsgSender& msgSender)
  : msgSender_(msgSender) {}

void PowerManager::SignalShutdown() {
    msgSender_->sendMsg("shutdown()");
}

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

Вы также можете написать то же самое, используя умный указатель (через boost или c ++ 11), чтобы явно заставить IMsgSenderжить дольше, чем PowerManager:

PowerManager::PowerManager(std::shared_ptr<IMsgSender> msgSender) 
  : msgSender_(msgSender) {}

void PowerManager::SignalShutdown() {
    // Here, we own a smart pointer to IMsgSender, so even if the caller
    // destroys the original pointer, we still have a valid copy
    msgSender_->sendMsg("shutdown()");
}

Этот метод предпочтителен, если возможно, что IMsgSenderвремя жизни не может быть гарантированно длиннее, чем у PowerManager(т. Е. x = new IMsgSender(); p = new PowerManager(*x);).

† Что касается указателей: безудержная проверка нуля затрудняет чтение кода и не улучшает стабильность (это улучшает внешний вид стабильности, что намного хуже).

Где-то кто-то получил адрес для памяти для хранения IMsgSender. Именно эта функция отвечает за обеспечение успешного распределения (проверка возвращаемых значений библиотеки или правильная обработка std::bad_allocисключений), чтобы не пропускать недопустимые указатели.

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

†† Так как вы новичок в этой работе, я ожидаю, что вы взламываете существующий код. Под недостатком дизайна я подразумеваю, что недостаток заключается в коде, с которым вы работаете. Таким образом, люди, которые помечают ваш код, потому что он не проверяет нулевые указатели, действительно помечают себя для написания кода, который требует указателей :)

Сет
источник
1
На самом деле нет ничего плохого в использовании сырых указателей. Std :: shared_ptr не является серебряной пулей, и использование его в списке аргументов является лекарством от неаккуратной инженерии, хотя я понимаю, что считается «легким» использовать его, когда это возможно. Используйте Java, если вы чувствуете себя так.
Джеймс
2
Я не говорил, что сырые указатели плохие! У них определенно есть свое место. Тем не менее, это C + + , ссылки (и общие указатели, в настоящее время) являются частью языка по причине. Используйте их, они приведут вас к успеху.
Сет
Мне нравится идея умного указателя, но это не вариант в этом конкретном концерте. +1 за рекомендацию использования ссылок (как у @Max и нескольких других). Иногда, однако, невозможно внедрить ссылку из-за проблем зависимости типа «курица и яйцо», т. Е. Если объект, который в противном случае был бы кандидатом на инъекцию, должен иметь указатель (или ссылку) на своего держателя, введенный в него . Иногда это может указывать на тесно связанную конструкцию; но это часто приемлемо, как в отношениях между объектом и его итератором, где никогда не имеет смысла использовать последний без первого.
evadeflow
8

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

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

Каждый раз, когда вызывается функция, и это не опциональное поведение, она должна давать сбои громко, а не тихо.

Отредактируйте: думая о своей истории младшего программиста, звучит так, как будто случился так, что частному члену было присвоено значение null, когда это никогда не должно было случиться. У них была проблема с неуместным написанием, и они пытаются исправить это путем проверки при чтении. Это задом наперед. К тому времени, когда вы его идентифицируете, ошибка уже произошла. Реализуемое решение для проверки кода / компилятора для этого - не защитные условия, а геттеры и сеттеры или постоянные члены.

jmoreno
источник
7

Как отметили другие, это зависит от того, msgSenderможет ли быть законным или нет NULL. Следующее предполагает, что оно никогда не должно быть NULL.

void PowerManager::SignalShutdown()
{
    if (!msgSender_)
    {
       throw SignalException("Shut down failed because message sender is not set.");
    }

    msgSender_->sendMsg("shutdown()");
}

Предложенное «исправление» другими членами вашей команды нарушает принцип « Мертвые программы говорят, что нет лжи» . Ошибки действительно трудно найти как есть. Метод, который незаметно изменяет свое поведение в зависимости от более ранней проблемы, не только затрудняет поиск первой ошибки, но также добавляет и вторую собственную ошибку.

Младший сеял хаос, не проверяя на ноль. Что делать, если этот фрагмент кода наносит ущерб, продолжая работать в неопределенном состоянии (устройство включено, но программа «думает», что оно выключено)? Возможно, другая часть программы сделает что-то безопасное, только когда устройство выключено.

Любой из этих подходов позволит избежать тихих сбоев:

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

  2. Брось исключение, если оно пустое.

совать
источник
5

Я согласен с ловушкой нуль в конструкторе. Далее, если член объявлен в заголовке как:

IMsgSender* const msgSender_;

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

Гримм Опинер
источник
Это отличный совет, спасибо! На самом деле так хорошо, что мне жаль, что я не могу проголосовать выше. Это не прямой ответ на поставленный вопрос, но это отличный способ исключить любую возможность «случайного» превращения указателя NULL.
evadeflow
@evadeflow Я подумал: «Я согласен со всеми» ответил на основной вопрос. Ура, хотя! ;-)
Grimm The Opiner
С моей стороны было бы немного невежливо изменить принятый ответ, учитывая то, как был сформулирован вопрос. Но я действительно считаю этот совет выдающимся, и мне жаль, что я сам об этом не подумал. С constуказателем нет необходимости выходить assert()за пределы конструктора, поэтому этот ответ кажется даже немного лучше, чем у @Kristof Provost (который также был выдающимся, но также рассматривал вопрос несколько косвенно). Я надеюсь, что другие проголосуют за это, так как на самом деле не имеет смысла assertв каждом методе, когда вы можете просто сделать указатель const.
evadeflow
@evadeflow Люди только посещают Questins для представителя, теперь на него отвечают, никто не будет смотреть на это снова. ;-) Я только зашел, потому что это был вопрос об указателях, и я слежу за кровавой бригадой "Используйте умные указатели для всего всегда".
Гримм Опинер
3

Это совершенно опасно!

Я работал под руководством старшего разработчика в кодовой базе C с самыми ненадежными «стандартами», которые настаивали на том же, чтобы вслепую проверять все указатели на ноль. В конечном итоге разработчик будет делать такие вещи:

// Pre: vertex should never be null.
void transform_vertex(Vertex* vertex, ...)
{
    // Inserted by my "wise" co-worker.
    if (!vertex)
        return;
    ...
}

Однажды я пытался удалить такую ​​проверку предусловия один раз в такой функции и заменил ее на, assertчтобы посмотреть, что произойдет.

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

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

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

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

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

Разумно ли для стандарта кодирования требовать, чтобы каждый отдельный указатель, разыменованный в функции, сначала проверялся на NULL - даже на частные члены данных?

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

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


источник
1
"assert" - это путь, но помните, что в любой современной системе каждый доступ к памяти через указатель имеет встроенный в аппаратное средство нулевой указатель assert. Так что в «assert (p! = NULL); return * p;» даже утверждение в основном бессмысленно.
gnasher729
@ gnasher729 Ах, это очень хороший момент, но я склонен рассматривать assertкак механизм документирования, чтобы установить предварительные условия, а не просто способ принудительного прерывания. Но мне также нравится природа ошибки подтверждения, которая показывает вам, какая именно строка кода вызвала ошибку и не удалось выполнить условие подтверждения («документирование сбоя»). Это может пригодиться, если мы в конечном итоге используем отладочную сборку вне отладчика и застаемся врасплох.
@ gnasher729 Или я, возможно, неправильно понял часть этого. Эти современные системы показывают, какая строка исходного кода, в котором утверждение не удалось? Я вообще думаю , они потеряли информацию в этой точке и может просто показать нарушение выдаёт ошибку сегментации / доступа, но я своего рода далеко от «современных» - до сих пор упорно на Windows 7 для большинства моего развития.
Например, в MacOS X и iOS, если ваше приложение дает сбой из-за доступа к нулевому указателю во время разработки, отладчик сообщит вам точную строку кода, где это происходит. Есть еще один странный аспект: компилятор попытается предупредить вас, если вы сделаете что-то подозрительное. Если вы передадите указатель на функцию и получите к ней доступ, компилятор не выдаст вам предупреждение о том, что указатель может быть НЕДЕЙСТВИТЕЛЕН, потому что это происходит так часто, что вы утонете в предупреждениях. Однако, если вы делаете сравнение if (p == NULL) ... тогда компилятор предполагает, что есть большая вероятность того, что p равно NULL,
gnasher729
потому что почему бы вы не проверили это иначе, поэтому с тех пор он выдаст предупреждение, если вы используете его без тестирования. Если вы используете свой собственный assert-подобный макрос, вы должны быть немного умны, чтобы написать его таким образом, чтобы "my_assert (p! = NULL," Это нулевой указатель, глупый! "); * P = 1;" не дает вам предупреждение.
gnasher729
2

Objective-C , например, обрабатывает каждый вызов метода nilобъекта как неиспользуемый, который оценивается как нулевое значение. У этого проектного решения в Objective-C есть некоторые преимущества по причинам, предложенным в вашем вопросе. Теоретическая концепция нулевой защиты каждого вызова метода имеет некоторые достоинства, если она хорошо опубликована и последовательно применяется.

Тем не менее, код живет в экосистеме, а не в вакууме. Нулевое охраняемое поведение было бы неидиоматичным и удивительным в C ++, и поэтому его следует считать вредным. Таким образом, никто не пишет код C ++ таким образом, так что не делайте этого! В качестве контр-примера, однако, обратите внимание, что вызов free()или deleteдля a NULLв C и C ++ гарантированно будет неработоспособным.


В вашем примере, вероятно, стоило бы поместить утверждение в конструктор, msgSenderкоторое не является нулевым. Если конструктор называется метод на msgSenderсразу же, то не такое утверждение не будет необходимости, поскольку она разбила бы прямо там в любом случае. Однако, поскольку он просто хранится msgSender для будущего использования, не будет очевидным, если посмотреть на трассировку стека того, SignalShutdown()как получилось значение NULL, поэтому утверждение в конструкторе значительно упростит отладку.

Более того, конструктор должен принимать const IMsgSender&ссылку, чего не может быть NULL.

200_success
источник
1

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

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

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

Вам также следует использовать надежный инструмент для проверки правильности кода, такой как укрытие или укрепление. И вы должны учитывать все предупреждения компилятора.

Изменить: как уже упоминали другие, молчаливый сбой, как в нескольких примерах кода, как правило, тоже неправильно. Если ваша функция не может восстановить значение, равное нулю, она должна вернуть ошибку (или сгенерировать исключение) вызывающей стороне. Вызывающая сторона несет ответственность за исправление порядка вызовов, восстановление или возврат ошибки (или выдачу исключения) вызывающей стороне и т. Д. В конце концов, либо функция способна изящно восстанавливаться и двигаться дальше, изящно восстанавливаться и давать сбой (например, сбой транзакции в базе данных из-за внутренней ошибки для одного пользователя, но не из-за выхода из нее), либо функция определяет, что состояние приложения повреждено и неустранимый и приложение выходит.

атк
источник
+1 за смелость поддержать (казалось бы) непопулярную позицию. Я был бы разочарован, если бы никто не защитил моих коллег по этому вопросу (они очень умные люди, которые выпускают качественный продукт в течение многих лет). Мне также нравится идея о том, что приложения должны "аварийно завершать работу в тестовой среде ... и постепенно выходить из строя на производстве". Я не убежден в том, что обязательные NULLпроверки «за рулем» являются способом сделать это, но я ценю услышать мнение кого-то за пределами моего рабочего места, который в основном говорит: «Эй. Не кажется слишком необоснованным».
evadeflow
Я раздражаюсь, когда вижу, что люди проверяют NULL после malloc (). Это говорит мне, что они не понимают, как современные ОС управляют памятью.
Кристоф Провост
2
Я предполагаю, что @KristofProvost говорит об ОС, которые используют overcommit, для которого malloc всегда успешен (но ОС может позже уничтожить ваш процесс, если на самом деле не будет достаточно памяти). Однако это не является хорошей причиной для пропуска пустых проверок в malloc: overcommit не универсален для разных платформ, и даже если он включен, могут быть другие причины сбоя malloc (например, в Linux ограничение адресного пространства процесса установлено с помощью ulimit ).
Джон Варфоломей
1
Что сказал Джон Я действительно говорил о чрезмерном подчинении. Overcommit включен по умолчанию в Linux (это то, что оплачивает мои счета). Я не знаю, но подозреваю, что так же и в Windows. В большинстве случаев вы все равно закончите сбоем, если только вы не будете готовы написать много кода, который никогда и никогда не будет тестироваться для обработки ошибок, которые почти наверняка никогда не произойдут ...
Кристоф Провост,
2
Позвольте мне добавить, что я также никогда не видел код (за пределами ядра Linux, где нехватка памяти реально возможна), который действительно правильно обрабатывает состояние нехватки памяти. Весь код, который я видел в try, просто все равно вылетит позже. Все, что было сделано, - это потратить время, усложнить понимание кода и скрыть реальную проблему. Нулевые разыменования легко отлаживать (если у вас есть файл ядра).
Кристоф Провост
1

Использование указателя вместо ссылки скажет мне, что msgSenderэто только вариант, и здесь проверка на ноль будет правильной. Фрагмент кода слишком медленный, чтобы решить это. Может быть, есть другие элементы, PowerManagerкоторые являются ценными (или тестируемыми) ...

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

волк
источник
Как я уже упоминал в другом комментарии, бывают случаи, когда вставка ссылки невозможна. Я
часто сталкиваюсь с примером,
@evadeflow: ОК, признаюсь, это зависит от размера класса и видимости членов-указателей (на которые нельзя ссылаться), проверю ли я перед разыменованием. В тех случаях, когда класс маленький и простой, а указатель (и) являются частными, я не ...
Wolf
0

Это зависит от того, что вы хотите случиться.

Вы написали, что работаете над «устройством бытовой электроники». Если каким - то образом, ошибка , вводится путем установки msgSender_на NULL, вы хотите

  • устройство, чтобы продолжать, пропуская, SignalShutdownно продолжая остальную часть его работы, или
  • сбой устройства, заставляющий пользователя перезапустить его?

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

Конечно, если вы выберете вариант 1, assert(как рекомендовано другими) жизненно важно, чтобы уменьшить вероятность того, что такая ошибка будет незаметно появляться во время разработки. ifНулевой охранник только там для смягчения отказа в использовании продукции.

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

Heinzi
источник