Должен ли я проверить возвращаемое значение вызова метода, даже если я знаю, что метод не может вернуть неверный ввод?

30

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

ДАННЫЙ

User getUser(Int id)
{
    User temp = new User(id);
    temp.setName("John");

    return temp;
}

Я ДОЛЖЕН ДЕЛАТЬ

void myMethod()
{
    User user = getUser(1234);
    System.out.println(user.getName());
}

ИЛИ

void myMethod()
{
    User user = getUser(1234);

    // Validating
    Preconditions.checkNotNull(user, "User can not be null.");
    Preconditions.checkNotNull(user.getName(), "User's name can not be null.");

    System.out.println(user.getName());
}

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


Мой вывод из всех ответов (смело приходите к своим):

Утверждать когда
  • Метод показал себя плохо в прошлом
  • Метод из ненадежного источника
  • Этот метод используется из других мест и явно не указывает его постусловия
Не утверждайте, когда:
  • Метод живет близко к вашему (подробности см. В выбранном ответе)
  • Метод явно определяет его контракт с чем-то вроде надлежащего документа, безопасности типа, модульного теста или проверки после условия
  • Производительность имеет решающее значение (в этом случае утверждение режима отладки может работать как гибридный подход)
Дидье А.
источник
1
Стремитесь к тому, чтобы охватить 100% кода!
Джаред Берроуз
9
Почему вы сосредоточены только на одной проблеме ( Null)? Вероятно, одинаково проблематично, если имя ""(не ноль, а пустая строка) или "N/A". Либо вы можете доверять результату, либо вы должны быть соответственно параноиком.
MSalters
3
В нашей кодовой базе у нас есть схема именования: getFoo () обещает не возвращать ноль, а getFooIfPresent () может возвращать ноль.
pjc50
Откуда исходит ваша презумпция здравомыслия кода? Вы предполагаете, что значение всегда будет ненулевым, потому что оно проверено на входе, или из-за структуры, по которой значение извлекается? И, что более важно, вы тестируете каждый возможный крайний сценарий? В том числе возможность вредоносного скрипта?
Зиббобз
@MSalters Не проверяйте значение, если оно есть ;)
Изката,

Ответы:

42

Это зависит от того, насколько вероятны изменения getUser и myMethod, и, что более важно, насколько вероятно, что они изменятся независимо друг от друга .

Если вы как-то точно знаете, что getUser никогда и никогда не изменится в будущем, тогда да, это пустая трата времени на его проверку, равно как и на потерю времени на проверку, iзначение которой равно 3 сразу после i = 3;выражения. На самом деле, вы этого не знаете. Но есть некоторые вещи, которые вы знаете:

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

  • Является ли функция getUser частью документированного API с конкретным контрактом, а myMethod - просто клиентом указанного API в другой кодовой базе? Если это так, вы можете прочитать эту документацию, чтобы узнать, должны ли вы проверять возвращаемые значения (или предварительно проверять входные параметры!) Или действительно ли безопасно слепо следовать счастливому пути. Если документация не проясняет это, попросите сопровождающего исправить это.

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

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

Ixrec
источник
2
Кроме того: если бы вам пришлось изменить getUserпозже, чтобы он мог вернуть ноль, даст ли явная проверка информацию, которую вы не можете получить из «NullPointerException», и номер строки?
user253751
Кажется, есть две вещи, которые вы можете проверить: (1) То, что метод работает правильно, например, в нем нет ошибок. (2) Что метод уважает ваш контракт. Я вижу подтверждение того, что № 1 будет излишним. У вас должны быть тесты, выполняющие №1. Вот почему я не проверю i = 3 ;. Так что мой вопрос касается # 2. Мне нравится ваш ответ на этот вопрос, но в то же время это ответ на ваш вопрос. Видите ли вы какие-либо другие проблемы, возникающие из-за явной проверки ваших контрактов, кроме того, что вы тратите немного времени на написание утверждения?
Дидье А.
Единственная другая «проблема» заключается в том, что если ваша интерпретация контракта неверна или становится неправильной, вам придется изменить всю валидацию. Но это относится и ко всему остальному коду.
Ixrec
1
Не могу идти против толпы. Этот ответ, безусловно, является наиболее правильным в освещении темы моего вопроса. Я хотел бы добавить, что, прочитав все ответы, особенно @MikeNakis, я теперь думаю, что вы должны подтвердить свои предварительные условия, по крайней мере, в режиме отладки, когда у вас есть один из двух последних пунктов Ixrec. Если вы столкнулись с первым пунктом, то, вероятно, излишне утверждать свои предварительные условия. Наконец, при наличии явного контракта, такого как надлежащий документ, безопасность типов, модульный тест или проверка после условия, вы не должны утверждать свои предварительные условия, это было бы излишним.
Дидье А.
22

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

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

Мой девиз:

Вопрос, который вы всегда должны себе задавать, это не «я должен это утверждать?» но "есть что-то, что я забыл утверждать?"

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

Но есть исключения даже из этого правила. Как ни странно, если взять пример Иксрека, существует тип ситуации, когда совершенно желательно следовать int i = 3;утверждению, iкоторое действительно находится в определенном диапазоне. Это ситуация, в которой iможет измениться кто-то, кто пробует различные сценарии типа «что-если», а следующий код полагается на iналичие значения в определенном диапазоне, и это не сразу становится очевидным, если быстро взглянуть на следующий код, что приемлемый диапазон Таким образом, int i = 3; assert i >= 0 && i < 5;на первый взгляд это может показаться бессмысленным, но если вы подумаете об этом, это говорит о том, что вы можете играть с ним i, а также о диапазоне, в котором вы должны оставаться. Утверждение является лучшим типом документации, потому чтоэто обеспечивается машиной , поэтому это идеальная гарантия, и он подвергается рефакторингу вместе с кодом , поэтому он всегда остается актуальным.

Ваш getUser(int id)пример не очень отдаленный концептуальный родственник assign-constant-and-assert-in-rangeпримера. Вы видите, по определению, ошибки случаются, когда вещи демонстрируют поведение, отличное от ожидаемого нами поведения. Правда, мы не можем утверждать все, поэтому иногда будет некоторая отладка. Но в отрасли хорошо известно, что чем быстрее мы обнаружим ошибку, тем меньше она будет стоить. Вопросы, которые вы должны задать, касаются не только того, насколько вы уверены, что getUser()никогда не вернете нулевое значение (и никогда не вернете пользователя с нулевым именем), но и того, какие плохие вещи будут происходить с остальным кодом, если это произойдет в факт один день возвращения что - то неожиданное, и как легко, быстро , глядя на остальной части кода , чтобы знать точно , что ожидалось getUser().

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

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

Майк Накис
источник
2
Да. Утверждай это. Я пришел сюда, чтобы дать более или менее такой ответ. ++
RubberDuck
2
@ SteveJessop Я бы исправил это ... && sureness < 1).
Майк Накис
2
@MikeNakis: это всегда кошмар при написании модульных тестов, возвращаемое значение, которое разрешено интерфейсом, но на самом деле невозможно сгенерировать из каких-либо входных данных ;-) В любом случае, когда вы на 101% уверены, что вы правы, вы определенно ошибаетесь. !
Стив Джессоп
2
+1 за указание на вещи, которые я совершенно забыл упомянуть в своем ответе.
Ixrec
1
@DavidZ Это верно, мой вопрос предполагал проверку во время выполнения. Но сказать, что вы не доверяете методу в отладке и доверяете ему в prod, возможно, хорошая точка зрения на эту тему. Таким образом, утверждение в стиле C может быть хорошим способом справиться с этой ситуацией.
Дидье А.
8

Определенное нет.

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

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

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

Павел
источник
Я согласен с общим мнением, но есть обстоятельства, когда определенно имеет смысл проверять возвращаемые значения. Например, если вы звоните во внешнюю веб-службу, иногда требуется, по крайней мере, проверить формат ответа (xsd и т. Д.)
AK_
Похоже, вы защищаете стиль «Дизайн по контракту» вместо стиля «Защитное программирование» Я думаю, что это отличный ответ. Если бы у метода был строгий контракт, я мог бы доверять ему. В моем случае это не так, но я мог бы изменить это, чтобы сделать это. Сказать, что я не мог хотя? Вы бы поспорили, что я должен доверять реализации? Даже если он явно не заявляет в своем документе, что он не возвращает null или фактически имеет проверку после условия?
Дидье А.
По моему мнению, вы должны использовать свое лучшее суждение и взвесить различные аспекты, такие как то, насколько вы доверяете автору делать правильные вещи, насколько вероятно изменение интерфейса функции и насколько жесткими являются ваши временные ограничения. При этом в большинстве случаев автор функции имеет достаточно четкое представление о том, каким должен быть контракт функции, даже если он не документирует его. Из-за этого я говорю, что сначала вы должны доверять авторам, чтобы они поступали правильно и защищались, только когда знаете, что эта функция написана плохо.
Пол
Я обнаружил, что первое доверие другим к правильному поступку помогает мне быстрее выполнять работу. Тем не менее, виды приложений, которые я пишу, легко исправить, когда проблема действительно возникает. Кто-то, работающий с более критичным для безопасности программным обеспечением, может подумать, что я слишком снисходительный и предпочитаю более защитный подход.
Пол
5

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

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

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

MatthewC
источник
Null не является допустимым возвращаемым значением getUser. Изучив getUser, я увидел, что реализация никогда не вернет ноль. Мой вопрос заключается в том, должен ли я доверять своей проверке и не иметь проверки в моем методе, или я должен сомневаться в своей проверке, и все же иметь проверку. Также возможно, что метод изменится в будущем, нарушая мои ожидания не возвращать ноль.
Дидье А.
А что, если getUser или user.getName изменятся, чтобы генерировать исключения? Вы должны иметь чек, но не как часть метода, использующего пользователя. Оберните метод getUser - и даже класс User - для обеспечения выполнения ваших контрактов, если вы беспокоитесь о будущих изменениях в коде getUser. Также создайте модульные тесты, чтобы проверить свои предположения о поведении класса.
MatthewC
@didibus Перефразируя ваш ответ ... Смайлик смайлика не является допустимым возвращаемым значением getUser. Изучив getUser, я увидел, что реализация никогда не вернет смайлик . Мой вопрос заключается в том, должен ли я доверять своей проверке и не иметь проверки в моем методе, или я должен сомневаться в своей проверке, и все же иметь проверку. Также возможно, что метод изменится в будущем, превзойдя мои ожидания не возвращать смайлик смайлика . Вызывающий не должен подтверждать, что вызываемая функция выполняет свой контракт.
Винс О'Салливан
@ VinceO'Sullivan Кажется, что "контракт" находится в центре проблемы. И что мой вопрос касается неявных контрактов, а не явных. Метод, который возвращает int, я не буду утверждать, что я не получаю строку. Но если я хочу только позитивные интты, я должен? Это только неявно, что метод не возвращает отрицательное значение int, потому что я его проверил. Из подписи или документации не ясно, что это не так.
Дидье А.
1
Неважно, является ли контракт неявным или явным. Задача вызывающего кода не заключается в проверке того, что вызываемый метод возвращает действительные данные, когда ему переданы правильные данные. В вашем примере вы знаете, что отрицательные начальные значения не являются неправильными ответами, но знаете ли вы, что положительные значения int верны? Сколько тестов вам действительно нужно, чтобы доказать, что метод работает? Единственный правильный путь - предположить, что метод правильный.
Винс О'Салливан
5

Я бы сказал, что предпосылка вашего вопроса не верна: зачем использовать нулевую ссылку для начала?

Модель объекта нулевого способ вернуть объекты , которые по существу ничего не делать: вместо возврата NULL для вашего пользователя, возвращает объект , который не представляет «нет пользователя.»

Этот пользователь будет неактивен, будет иметь пустое имя и другие ненулевые значения, указывающие «здесь ничего нет». Основным преимуществом является то, что вам не нужно мусорить, ваша программа будет иметь нулевые проверки, ведение журнала и т. Д. Вы просто используете свои объекты.

Почему алгоритм должен заботиться о нулевом значении? Шаги должны быть одинаковыми. Так же просто и намного понятнее иметь нулевой объект, который возвращает ноль, пустую строку и т. Д.

Вот пример проверки кода на ноль:

User u = getUser();
String displayName = "";
if (u != null) {
  displayName = u.getName();
}
return displayName;

Вот пример кода, использующего нулевой объект:

return getUser().getName();

Что яснее? Я считаю, что второй.


Хотя вопрос помечен как , стоит отметить, что шаблон нулевого объекта действительно полезен в C ++ при использовании ссылок. Ссылки на Java больше похожи на указатели C ++ и допускают null. C ++ ссылки не могут храниться nullptr. Если кто-то хотел бы иметь что-то аналогичное нулевой ссылке в C ++, шаблон нулевого объекта - единственный способ, которым я знаю, для достижения этой цели.

Сообщество
источник
Нуль - просто пример того, что я утверждаю. Вполне возможно, мне нужно непустое имя. Мой вопрос все еще применим, вы бы это подтвердили? Или это может быть что-то, что возвращает int, и у меня не может быть отрицательного int, или оно должно находиться в заданном диапазоне. Не думайте, что это пустая проблема.
Дидье А.
1
Почему метод возвращает неправильное значение? Единственное место, где это нужно проверить, находится на границе вашего приложения: интерфейсы с пользователем и с другими системами. В противном случае, поэтому у нас есть исключения.
Представьте, что у меня есть: int i = getCount; поплавок х = 100 / я; Если я знаю, что getCount не возвращает 0, потому что я либо написал метод, либо проверил его, должен ли я пропустить проверку на 0?
Дидье А.
@didibus: вы можете пропустить проверку, если getCount() документы гарантируют, что она не вернет 0 и не будет делать это в будущем, за исключением случаев, когда кто-то решит внести решающее изменение и попытается обновить все, что вызывает его, чтобы справиться с ним. новый интерфейс. Просмотр того, что в данный момент делает код, не поможет, но если вы собираетесь проверять код, то проверяемый код - это модульные тесты getCount(). Если они проверят, чтобы оно не возвращало 0, то вы знаете, что любое будущее изменение с возвратом 0 будет считаться критическим изменением, потому что тесты не пройдут.
Стив Джессоп
Мне не ясно, что нулевые объекты лучше, чем нулевые. Нулевая утка может крякать, но это не утка - на самом деле, семантически, это антитеза утки. После того, как вы введете нулевой объект, любой код, использующий этот класс, может проверить, является ли какой-либо из его объектов нулевым объектом - например, у вас есть нулевое что-то в наборе, сколько у вас вещей? Это выглядит для меня как способ отложить неудачу и запутать ошибки. Связанная статья специально предупреждает против использования нулевых объектов «просто чтобы избежать нулевых проверок и сделать код более читабельным».
Сденхэм
1

Как правило, я бы сказал, что это зависит главным образом от следующих трех аспектов:

  1. Надежность: может ли вызывающий метод справиться со nullзначением? Если nullзначение может привести RuntimeExceptionк проверке, я бы порекомендовал проверить - если сложность не низкая, а вызываемые / вызывающие методы создаются одним и тем же автором и nullне ожидаются (например, оба privateв одном пакете).

  2. ответственность за код: если разработчик A отвечает за getUser()метод, а разработчик B использует его (например, как часть библиотеки), я настоятельно рекомендую проверить его значение. Просто потому, что разработчик B может не знать об изменении, которое может привести к потенциальному nullвозвращаемому значению.

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

Кроме того, я бы порекомендовал документировать потенциальные nullвозвращаемые значения в комментарии JavaDoc. Благодаря выделению описаний JavaDoc в большинстве IDE, это может быть полезным предупреждением для тех, кто их использует getUser().

/** Returns ....
  * @param: id id of the user
  * @return: a User instance related to the id;
  *          null, if no user with identifier id exists
 **/
User getUser(Int id)
{
    ...
}
Андре
источник
-1

Вы определенно не должны.

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

Йеллен
источник
1
Я знаю, но только потому, что я проверял или писал реализацию другого метода. Контракт метода явно не говорит, что не может. Так что это может, например, измениться в будущем. Или это может иметь ошибку, которая приводит к возвращению нулевого значения, даже если он пытается этого не делать.
Дидье А.