Сколько проверки нуля достаточно?
Каковы некоторые рекомендации, когда нет необходимости проверять на ноль?
Большая часть унаследованного кода, над которым я работал в последнее время, имеет нулевые проверки и тошноту. Нулевые проверки на тривиальные функции, нулевые проверки на вызовы API, которые устанавливают ненулевые возвраты, и т. Д. В некоторых случаях нулевые проверки являются разумными, но во многих местах нулевое значение не является разумным ожиданием.
Я слышал ряд аргументов от "Вы не можете доверять другому коду" до "ВСЕГДА программируйте с защитой" до "Пока язык не гарантирует мне ненулевое значение, я всегда буду проверять". Я, конечно, до определенного момента согласен со многими из этих принципов, но я обнаружил, что чрезмерная проверка на нуль вызывает другие проблемы, которые обычно нарушают эти принципы. Стойкая проверка на ноль действительно того стоит?
Часто я наблюдал, что коды с избыточной проверкой нуля на самом деле имеют худшее качество, а не более высокое качество. Большая часть кода, кажется, настолько сосредоточена на нулевых проверках, что разработчик упустил из виду другие важные качества, такие как читабельность, корректность или обработка исключений. В частности, я вижу много кода, игнорирующего исключение std::bad_alloc, но делающего нулевую проверку на new,
В C++ я понимаю это в некоторой степени из-за непредсказуемого поведения разыменования нулевого указателя; разыменование нуля обрабатывается более изящно в Java, C#, Python и т. д. Я только что видел плохие примеры бдительной проверки нуля или есть что-то в этом?
Этот вопрос задуман как независимый от языка, хотя в основном меня интересуют C++, Java и C#.
Некоторые примеры проверки на нуль, которые я видел, которые кажутся чрезмерными, включают следующее:
Этот пример, кажется, учитывает нестандартные компиляторы, поскольку спецификация C++ говорит, что сбойное новое генерирует исключение. Если вы явно не поддерживаете несовместимые компиляторы, имеет ли это смысл? Имеет ли это какой-то смысл в управляемом языке, таком как Java или C# (или даже C++/CLR)?
try {
MyObject* obj = new MyObject();
if(obj!=NULL) {
//do something
} else {
//??? most code I see has log-it and move on
//or it repeats what's in the exception handler
}
} catch(std::bad_alloc) {
//Do something? normally--this code is wrong as it allocates
//more memory and will likely fail, such as writing to a log file.
}
Другой пример - при работе с внутренним кодом. В частности, если это небольшая команда, которая может определять свои собственные методы разработки, это кажется ненужным. В некоторых проектах или унаследованном коде доверять документации может быть нецелесообразно... но для нового кода, которым вы или ваша команда управляете, действительно ли это необходимо?
Если метод, который вы видите и можете обновить (или можете кричать на разработчика, который несет за это ответственность), имеет контракт, все равно необходимо ли проверять наличие нулевых значений?
//X is non-negative.
//Returns an object or throws exception.
MyObject* create(int x) {
if(x<0) throw;
return new MyObject();
}
try {
MyObject* x = create(unknownVar);
if(x!=null) {
//is this null check really necessary?
}
} catch {
//do something
}
При разработке частной или другой внутренней функции, действительно ли необходимо явно обрабатывать нуль, когда контракт требует только ненулевые значения? Почему нулевая проверка предпочтительнее утверждения?
(очевидно, в вашем общедоступном API проверки на ноль имеют жизненно важное значение, поскольку считается невежливым кричать на ваших пользователей за неправильное использование API)
//Internal use only--non-public, not part of public API
//input must be non-null.
//returns non-negative value, or -1 if failed
int ParseType(String input) {
if(input==null) return -1;
//do something magic
return value;
}
По сравнению с:
//Internal use only--non-public, not part of public API
//input must be non-null.
//returns non-negative value
int ParseType(String input) {
assert(input!=null : "Input must be non-null.");
//do something magic
return value;
}
18 ответов
Прежде всего обратите внимание, что это особый случай проверки контракта: вы пишете код, который ничего не делает, кроме проверки во время выполнения, что задокументированный контракт выполнен. Ошибка означает, что некоторый код где-то неисправен.
Я всегда немного сомневаюсь в реализации особых случаев более общей концепции. Проверка контракта полезна, потому что она улавливает ошибки программирования при первом пересечении границы API. Что такого особенного в пустых значениях, что означает, что они являются единственной частью контракта, которую вы хотите проверить? Еще,
На предмет проверки входных данных:
null является особенным в Java: многие API Java написаны так, что null является единственным недопустимым значением, которое даже можно передать в вызов данного метода. В таких случаях нулевая проверка "полностью проверяет" входные данные, поэтому применяется полный аргумент в пользу проверки контракта.
В C++, с другой стороны, NULL - это только одно из почти 2^32 (2^64 на более новых архитектурах) недопустимых значений, которые может принимать параметр указателя, поскольку почти все адреса не относятся к объектам правильного типа. Вы не можете "полностью проверить" свои входные данные, если у вас нет списка где-либо всех объектов этого типа.
Тогда возникает вопрос, является ли NULL достаточно распространенным неверным вводом, чтобы получить специальную обработку, которая (foo *)(-1) не получается?
В отличие от Java, поля не инициализируются автоматически в NULL, поэтому неинициализированное значение мусора так же правдоподобно, как и NULL. Но иногда объекты C++ имеют члены-указатели, которые явно инициализируются NULL, что означает "у меня его еще нет". Если ваш абонент делает это, то существует значительный класс ошибок программирования, которые можно диагностировать с помощью проверки NULL. Исключение может быть легче для их отладки, чем ошибка страницы в библиотеке, для которой у них нет источника. Так что, если вы не возражаете против раздувания кода, это может быть полезно. Но вы должны думать не о себе, а о вызывающем абоненте - это не защитное кодирование, потому что оно "защищает" только от NULL, а не от (foo *)(-1).
Если NULL не является допустимым вводом, вы можете рассмотреть возможность выбора параметра по ссылке, а не по указателю, но многие стили кодирования не одобряют неконстантные ссылочные параметры. И если вызывающая сторона передает вам * fooptr, где fooptr равен NULL, то это все равно никому не принесло пользы. То, что вы пытаетесь сделать, это втиснуть немного больше документации в сигнатуру функции, в надежде, что ваш вызывающий с большей вероятностью подумает: "Хм, fooptr может быть нулевым здесь?" когда они должны явно разыменовать его, чем если бы они просто передали его вам в качестве указателя. Это заходит так далеко, но насколько это может помочь.
Я не знаю C#, но я понимаю, что это похоже на Java в том, что ссылки гарантированно имеют действительные значения (по крайней мере, в безопасном коде), но в отличие от Java в этом не все типы имеют значение NULL. Поэтому я предполагаю, что нулевые проверки там редко стоят: если вы в безопасном коде, тогда не используйте обнуляемый тип, если только нулевой не является допустимым вводом, и если вы в небезопасном коде, то применяются те же рассуждения, что и в C++.
На предмет проверки выходных данных:
Аналогичная проблема возникает: в Java вы можете "полностью проверить" вывод, зная его тип, и что значение не является нулевым. В C++ вы не можете "полностью проверить" вывод с проверкой NULL - для всего, что вы знаете, функция вернула указатель на объект в своем собственном стеке, который только что был размотан. Но если NULL является распространенным недопустимым возвращением из-за конструкций, обычно используемых автором кода вызываемого абонента, то проверка этого поможет.
Во всех случаях:
Используйте проверки, а не "реальный код", чтобы проверить контракты, где это возможно - после того, как ваше приложение заработало, вам, вероятно, не нужно, чтобы код раздувался для каждого вызываемого, проверяющего все его входные данные, и для каждого вызывающего, проверяющего свои возвращаемые значения.
В случае написания кода, переносимого на нестандартные реализации C++, вместо кода в вопросе, который проверяет наличие нуля, а также перехватывает исключение, у меня, вероятно, будет такая функция:
template<typename T>
static inline void nullcheck(T *ptr) {
#if PLATFORM_TRAITS_NEW_RETURNS_NULL
if (ptr == NULL) throw std::bad_alloc();
#endif
}
Затем в качестве одного из списка действий, которые вы делаете при портировании на новую систему, вы правильно определяете PLATFORM_TRAITS_NEW_RETURNS_NULL (и, возможно, некоторые другие PLATFORM_TRAITS). Очевидно, вы можете написать заголовок, который делает это для всех известных вам компиляторов. Если кто-то возьмет ваш код и скомпилирует его в нестандартной реализации C++, о которой вы ничего не знаете, он будет принципиально сам по более важным причинам, чем этот, поэтому им придется делать это самостоятельно.
Следует помнить, что ваш код, который вы пишете сегодня, хотя он может быть небольшой командой и иметь хорошую документацию, превратится в устаревший код, который придется поддерживать кому-то другому. Я использую следующие правила:
Если я пишу общедоступный API, который будет доступен другим, тогда я буду делать нулевые проверки всех ссылочных параметров.
Если я пишу внутренний компонент для своего приложения, я пишу нулевые проверки, когда мне нужно сделать что-то особенное, когда существует нулевое значение, или когда я хочу сделать это очень ясным. В противном случае я не против получить исключение нулевой ссылки, так как это также довольно ясно, что происходит.
При работе с возвращаемыми данными из фреймворков других людей, я проверяю ноль только тогда, когда возможно и допустимо вернуть ноль. Если их контракт говорит, что он не возвращает нули, я не буду делать проверку.
Если вы пишете код и его контракт, вы несете ответственность за его использование с точки зрения его контракта и обеспечения правильности контракта. Если вы говорите "возвращает ненулевой" x, то вызывающая сторона не должна проверять наличие нулевого значения. Если с этой ссылкой / указателем возникает исключение нулевого указателя, это неверный контракт.
Нулевая проверка должна доходить до крайности только при использовании библиотеки, которой нельзя доверять или которая не имеет надлежащего контракта. Если это код вашей команды разработчиков, подчеркните, что контракты не должны быть нарушены, и отследите человека, который неправильно использует контракт при возникновении ошибок.
Частично это зависит от того, как используется код - например, если это метод, доступный только в рамках проекта или общедоступного API. Проверка ошибок API требует чего-то более сильного, чем утверждение.
Так что, хотя это нормально в рамках проекта, где он поддерживается с помощью модульных тестов и тому подобного:
internal void DoThis(Something thing)
{
Debug.Assert(thing != null, "Arg [thing] cannot be null.");
//...
}
в методе, где вы не можете контролировать, кто его вызывает, что-то вроде этого может быть лучше:
public void DoThis(Something thing)
{
if (thing == null)
{
throw new ArgumentException("Arg [thing] cannot be null.");
}
//...
}
Это зависит от ситуации. Остальная часть моего ответа предполагает C++.
- Я никогда не проверяю возвращаемое значение new, поскольку все реализации, которые я использую, выбрасывают bad_alloc в случае неудачи. Если в каком-либо коде, над которым я работаю, я вижу унаследованный тест на новое возвращающее значение null, я вырезаю его и не пытаюсь заменить его чем-либо.
- Если только небольшие стандарты кодирования не запрещают это, я утверждаю задокументированные предварительные условия. Сломанный код, который нарушает опубликованный контракт, должен немедленно и резко потерпеть неудачу.
- Если ноль возникает из-за сбоя во время выполнения, который не из-за неработающего кода, я бросаю. Ошибка fopen и malloc (хотя я редко, если вообще использую их в C++), попадает в эту категорию.
- Я не пытаюсь восстановиться после сбоя распределения. Bad_alloc попадает в main().
- Если нулевой тест относится к объекту, являющемуся сотрудником моего класса, я переписываю код, чтобы взять его по ссылке.
- Если коллаборационист действительно может не существовать, я использую шаблон проектирования Null Object, чтобы создать заполнитель для сбоя четко определенными способами.
Проверка NULL в целом является злой, поскольку она добавляет небольшой отрицательный токен в тестируемость кода. С NULL-проверками везде, где вы не можете использовать технику "pass null", и это ударит вас при модульном тестировании Лучше иметь модульный тест для метода, чем нулевую проверку.
Проверьте достойную презентацию по этому вопросу и модульное тестирование в целом Миско Хевери на http://www.youtube.com/watch?v=wEhu57pih5w&feature=channel
Более старые версии Microsoft C++ (и, возможно, другие) не создавали исключение для неудачных выделений через новые, но возвращали NULL. Код, который должен был выполняться как в стандартных, так и в более старых версиях, будет иметь избыточную проверку, которую вы указали в первом примере.
Было бы чётче сделать так, чтобы все неудачные распределения проходили по одному и тому же пути кода:
if(obj==NULL)
throw std::bad_alloc();
Я бы сказал, что это немного зависит от вашего языка, но я использую Resharper с C#, и он в основном выходит из-под контроля, говоря мне, что "эта ссылка может быть нулевой", и в этом случае я добавляю проверку, если она говорит мне "это всегда будет истинным " for "if (null!= oMyThing && ....)", тогда я слушаю его, не проверяя на null.
Широко известно, что есть люди, ориентированные на процедуры (фокусируйтесь на том, чтобы делать все правильно), и люди, ориентированные на результат (получают правильный ответ). Большинство из нас лежат где-то посередине. Похоже, вы нашли выброс для процедурно-ориентированных. Эти люди сказали бы: "Все возможно, если вы не понимаете вещи совершенно; так что готовьтесь ко всему". Для них то, что вы видите, сделано правильно. Для них, если вы измените это, они будут волноваться, потому что утки не все выстроены в очередь.
Работая над чужим кодом, я стараюсь убедиться, что знаю две вещи.
1. Что задумал программист
2. Почему они написали код так, как сделали
Может быть, это поможет в работе с программистами типа А.
Таким образом, "сколько достаточно" в конечном итоге становится социальным вопросом, а не техническим вопросом - нет единого согласованного способа его измерения.
(Это сводит меня с ума тоже.)
Я проверяю NULL только тогда, когда знаю, что делать, когда вижу NULL. "Знать, что делать" здесь означает "знать, как избежать сбоя" или "знать, что сказать пользователю, кроме места сбоя". Например, если malloc() возвращает NULL, у меня обычно нет выбора, кроме как прервать программу. С другой стороны, если fopen() возвращает NULL, я могу сообщить пользователю имя файла, который не может быть открыт и может быть ошибочным. И если find() возвращает end(), я обычно знаю, как продолжить без сбоев.
Когда вы можете указать, какой компилятор используется, для системных функций, таких как "новая" проверка на нулевое значение, это ошибка в коде. Это означает, что вы будете дублировать код обработки ошибок. Дублирующий код часто является источником ошибок, потому что часто один изменяется, а другой нет. Если вы не можете указать версию компилятора или компилятора, вы должны быть более осторожны.
Что касается внутренних функций, вы должны указать контракт и убедиться, что контракт применяется через модульные тесты. У нас была проблема в нашем коде некоторое время назад, когда мы либо выдавали исключение, либо возвращали ноль в случае отсутствия объекта в нашей базе данных. Это просто сбивало с толку вызывающего API, поэтому мы прошли через него, сделали его постоянным по всей базе кода и удалили дубликаты проверок.
Важная вещь (IMHO) - не иметь дубликатов логики ошибок, когда никогда не будет вызываться одна ветвь. Если вы никогда не сможете вызвать код, то вы не сможете его протестировать и никогда не узнаете, сломан он или нет.
Лично я считаю, что нулевое тестирование не нужно в большинстве случаев. Если новый сбой или malloc дает сбой, у вас есть большие проблемы, и вероятность восстановления составляет почти ноль в тех случаях, когда вы не пишете проверку памяти! Кроме того, нулевое тестирование часто скрывает ошибки на этапах разработки, поскольку "нулевые" предложения часто просто пусты и ничего не делают.
Независимо от того, стоит ли проверять на ноль или нет, зависит от обстоятельств
Например, в нашем магазине мы проверяем параметры для методов, которые мы создаем для нуля внутри метода. Простая причина в том, что, как оригинальный программист, я хорошо представляю, что именно должен делать метод. Я понимаю контекст, даже если документация и требования являются неполными или менее чем удовлетворительными. Более поздний программист, которому поручено обслуживание, может не понимать контекст и может ошибочно предположить, что передача нуля безвредна. Если я знаю, что нуль будет вредным, и я могу предвидеть, что кто-то может передать ноль, я должен сделать простой шаг, чтобы убедиться, что метод реагирует изящно.
public MyObject MyMethod(object foo)
{
if (foo == null)
{
throw new ArgumentNullException("foo");
}
// do whatever if foo was non-null
}
Я не думаю, что это плохой код. Значительное количество вызовов API Windows / Linux возвращает NULL в случае какого-либо сбоя. Поэтому, конечно, я проверяю на наличие ошибок в порядке, указанном API. Обычно я передаю поток управления модулю ошибок, а не дублирую код обработки ошибок.
Если я получу указатель, который не гарантируется языком, чтобы он не был нулевым, и собираюсь де-ссылаться на него так, чтобы ноль сломал меня, или раздал, поставил свою функцию там, где я сказал, что не буду выдавать значения NULL, я проверьте на NULL.
Это не просто NULL, функция должна проверять предварительные и постусловия, если это возможно.
Неважно, если в контракте функции, которая дала мне указатель, написано, что он никогда не будет давать нули. Мы все делаем ошибки. Есть хорошее правило, что программа должна рано и часто выходить из строя, поэтому вместо передачи ошибки другому модулю и ее сбоя я потерплю неудачу на месте. Упрощает отладку при тестировании. Также в критических системах легче поддерживать систему в здравом уме.
Кроме того, если исключение исключается из основного, стек может не свернуться, что вообще не позволяет работать деструкторам (см. Стандарт C++ для terminate()). Что может быть серьезно. Поэтому оставить флажок bad_alloc не так опасно, как кажется.
Fail with assert vs. fail с ошибкой во время выполнения - это совсем другая тема.
Проверка на NULL после new(), если стандартное поведение new() не было изменено, чтобы возвращать NULL вместо броска, кажется устаревшим.
Есть еще одна проблема, которая заключается в том, что даже если malloc вернул действительный указатель, это еще не означает, что вы выделили память и можете ее использовать. Но это уже другая история.
Код более низкого уровня должен проверять использование кода более высокого уровня. Обычно это означает проверку аргументов, но это может означать проверку возвращаемых значений из upcalls. Upcall аргументы не должны быть проверены.
Целью является немедленное и очевидное выявление ошибок, а также документирование контракта в коде, который не лжет.
Моя первая проблема в том, что это приводит к коду, который усеян нулевыми проверками и тому подобным. Это ухудшает читабельность, и я бы даже сказал, что это ухудшает удобство сопровождения, потому что действительно легко забыть нулевую проверку, если вы пишете кусок кода, где определенная ссылка действительно никогда не должна быть нулевой. И вы просто знаете, что нулевые проверки будут отсутствовать в некоторых местах. Что на самом деле делает отладку сложнее, чем нужно. Если бы исходное исключение не было перехвачено и заменено ошибочным возвращаемым значением, то мы получили бы ценный объект исключения с информативной трассировкой стека. Что дает пропущенная нулевая проверка? NullReferenceException в части кода, которая заставляет вас перейти: wtf? эта ссылка никогда не должна быть нулевой!
Итак, вам нужно начать выяснять, как был вызван код, и почему ссылка может быть нулевой. Это может занять много времени и реально подорвать эффективность ваших усилий по отладке. В конце концов вы поймете реальную проблему, но есть вероятность, что она была спрятана довольно глубоко, и вы потратили гораздо больше времени на ее поиск, чем следовало бы.
Еще одна проблема, связанная с нулевыми проверками повсеместно, заключается в том, что некоторые разработчики не тратят время на то, чтобы правильно подумать о реальной проблеме, когда получают исключение NullReferenceException. Я на самом деле видел довольно много разработчиков, которые просто добавили проверку на ноль над кодом, где возникла исключительная ситуация NullReferenceException. Отлично, исключение больше не встречается! Ура! Мы можем идти домой сейчас! Хм... как насчет "нет, ты не можешь, и ты заслуживаешь локоть в лицо"? Настоящая ошибка может больше не вызывать исключений, но теперь у вас, вероятно, отсутствует или некорректно работает… и нет исключений! Что еще более болезненно и требует еще больше времени для отладки.
Сначала это казалось странным вопросом: null чеки это отличный и ценный инструмент. Проверяя что new возвращается null определенно глупо Я просто собираюсь игнорировать тот факт, что есть языки, которые позволяют это. Я уверен, что есть веские причины, но я действительно не думаю, что смогу справиться с жизнью в этой реальности:) Шутка в сторону, кажется, что вы, по крайней мере, должны указать, что new должен вернуться null когда не хватает памяти.
Во всяком случае, проверка на null где уместно приводит к более чистому коду. Я бы сказал, что никогда не назначать значения параметров функции по умолчанию - это следующий логический шаг. Чтобы пойти еще дальше, возвращая пустые массивы и т. Д., Где это уместно, мы получаем еще более чистый код. Приятно не беспокоиться о получении nullза исключением случаев, когда они имеют логическое значение. Нули как значения ошибок лучше избегать.
Использование утверждений действительно отличная идея. Особенно, если это дает вам возможность отключить их во время выполнения. Плюс, это более явно контрактный стиль:)