Подтвердить что ты не робот

Является ли это защитное программирование кода или плохой практикой?

У меня есть эта дискуссия с моим коллегой по поводу этого фрагмента кода:

var y = null;
if (x.parent != null)
    y = x.parent.somefield;

Моя точка зрения заключается в том, что в том месте, где находится код, x.parent не должен быть POSSIBLY равным нулю. И когда это null, у нас есть серьезная проблема, и я хочу это знать! Следовательно, нулевая проверка не должна быть там и исключить исключение нисходящего потока.

Мой коллега говорит, что это защитное программирование. И нулевая проверка гарантирует, что код не нарушит приложение.

Мой вопрос в том, является ли это защитное программирование? Или плохая практика?

Примечание: дело не в том, кто прав. Я пытаюсь извлечь уроки из этого примера.

4b9b3361

Ответ 1

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

"x.parent не должен ВОЗМОЖНО быть нулевым" является серьезным предположением. Вы должны быть очень уверены в том, чтобы играть в безопасное место, конечно, есть классический "это никогда не произойдет"....... до тех пор, пока это не произойдет, поэтому я думаю, что интересно рассмотреть возможности.

Я вижу две разные вещи, о которых нужно подумать.

Откуда берутся данные?

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

  • Противоположный случай был бы, если бы данные, введенные пользователем, или прочитанные из файла, или из URL-адреса, что-либо внешнее вообще. Поскольку вы не можете контролировать, с чем работает программа: всеми способами проверяйте ее так же тщательно, как вы можете, прежде чем использовать ее каким-либо образом, поскольку вы подвергаетесь фиктивной/отсутствующей/неполной/неправильной/вредоносной информации, которая может вызвать проблемы путь.

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

Что делать с проверкой?

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

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

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

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

Ответ 2

Похоже, что ваш коллега неправильно понимает "защитное программирование" и/или исключения.

Оборонительное программирование

Защитное программирование - это защита от определенных ошибок.

В этом случае x.parent == null является ошибкой, потому что ваш метод должен использовать x.parent.SomeField. И если parent было нулевым, то значение SomeField было бы явно недопустимым. Любое вычисление с помощью или задачи, выполненной с использованием недопустимого значения, приведет к ошибочным и непредсказуемым результатам.

Итак, вам нужно защититься от этой возможности. Очень хороший способ защиты - выбросить NullPointerException, если вы обнаружите, что x.parent == null. Исключение остановит вас от получения недопустимого значения из SomeField. Это остановит выполнение каких-либо вычислений или выполнение каких-либо задач с недопустимым значением. И он прервет всю текущую работу до тех пор, пока ошибка не будет устранена соответствующим образом.

Обратите внимание, что исключение составляет не ошибка; недопустимое значение в parent, которое является фактической ошибкой. Исключение на самом деле является механизмом защиты. Исключения составляют защитный метод программирования, их не следует избегать.

Поскольку С# уже генерирует исключение, вам фактически не нужно ничего делать. На самом деле получается, что ваши усилия коллеги "во имя защитного программирования" на самом деле отменяют встроенное защитное программирование, предоставляемое языком.

Исключения

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

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

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

Исключения будут, однако, прерывать (прервать) текущую операцию. И это то, что они должны делать. Потому что если вы закодируете метод под названием DoSomething, который вызывает DoStep1; ошибка в DoStep1 означает, что DoSomething не может выполнить свою работу должным образом. Нет смысла звонить DoStep2.

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

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

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

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

  • Запись в файл журнала.
  • Отправьте информацию о стеке вызовов для устранения неполадок.
  • Игнорировать определенные классы исключений. (Например, Abort может означать, что вам даже не нужно указывать пользователю, возможно, потому, что вы ранее отображали сообщение.)
  • и др.

Обработка исключений

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

ПРИМЕЧАНИЕ. Серверы обработчиков исключений имеют две разные цели: сначала они предоставляют вам место для выполнения очистки (или кода отката), в частности, потому что там была ошибка/исключение. Во-вторых, они предоставляют место для устранения ошибки и проглатывания исключения. NB. В первом случае очень важно, чтобы исключение было повторно поднято/выбрано, поскольку оно не было разрешено.

В комментарии о бросании исключения и его обработке вы сказали: "Я хотел сделать это, но мне сказали, что он везде создает код обработки исключений".

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

  • Вы можете разрешить ошибку, и в этом случае вы закончили.
  • Или где вам нужно реализовать код отката.

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

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

Ответ 3

Я бы не назвал это защитным программированием вообще - я бы назвал это "la la la я can not hear you" программирования:) Потому что код, кажется, эффективно игнорирует потенциальное условие ошибки.

Очевидно, мы не имеем никакого представления о том, что будет дальше в вашем коде, но поскольку вы не включили предложение else, я могу только предположить, что ваш код просто выполняется даже в случае, если x.parent на самом деле null.

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

Тогда возникает вопрос - что более приемлемо в контексте проблемы, которую вы пытаетесь решить ( "домен" ), и это зависит от того, что вы собираетесь делать с y позже.

  • Если y является null после того, как этот код не является проблемой (скажем, вы сделаете защитную проверку позже для y!=null), тогда это ОК - хотя лично мне не нравится этот стиль - вы в конечном итоге защищаете каждую отдельную ссылку, потому что никогда не можете быть абсолютно уверены, что вы являетесь нулевой ссылкой от сбоев...

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

Ответ 4

Короче говоря, я бы сказал, что это НЕ защитное программирование. Я согласен с теми, кто считает, что этот код скрывает системную ошибку вместо того, чтобы разоблачать и исправлять. Этот код нарушает принцип "fail fast".

Это верно тогда и только тогда, когда x.parent является обязательным ненулевым свойством (что кажется очевидным из контекста.) Однако, если x.parent является необязательным свойством (т.е. может иметь нулевое значение), тогда этот код может быть в порядке в зависимости от вашей бизнес-логики.

Я всегда рассматриваю использование пустых значений (0, "", пустых объектов) вместо нулей, для которых требуются тонны нерелевантных операторов if.

Ответ 5

После нескольких лет размышлений об этой проблеме я использую следующий "защитный" стиль программирования: 95% моих методов возвращают строку как результат успеха/неудачи.

Я возвращаю string.Empty для успеха и информативную текстовую строку, если не удалось. При возврате текста ошибки я записываю его в журнал одновременно.

public string Divide(int numA, int numB, out double division)
{
    division = 0;
    if (numB == 0)
        return Log.Write(Level.Error, "Divide - numB-[{0}] not valid.", numB);

    division = numA / numB;
    return string.Empty;
}

Затем я использую его:

private string Process(int numA, int numB)
{
    double division = 0;
    string result = string.Empty;
    if (!string.IsNullOrEmpty(result = Divide(numA, numB, out divide))
        return result;

    return SendResult(division);
}

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