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

Question

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

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

Моя точка зрения заключается в том, что в том месте, где код, x.parent НЕ МОЖЕТ БЫТЬ НУЛЬ. И когда это ноль, у нас есть серьезная проблема, и я хочу знать это! Следовательно, нулевая проверка не должна быть там и позволить исключение нисходящего потока.

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

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

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

Answer 1

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

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

Я вижу две разные вещи для размышления.

Откуда поступают данные?

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

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

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

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

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

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

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

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

Answer 2

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

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

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

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

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

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

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

Исключения

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Answer 3

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

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

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

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

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

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

Answer 4

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

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

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

Answer 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);
}

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