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

Question

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

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

Public Shared Function GenerateHash(ByVal FilePath As IO.FileInfo) As String
        If (FilePath Is Nothing) Then
            Throw New ArgumentNullException("FilePath")
        End If

        Dim _sha As New Security.Cryptography.MD5CryptoServiceProvider
        Dim _Hash = Convert.ToBase64String(_sha.ComputeHash(New IO.FileStream(FilePath.FullName, IO.FileMode.Open, IO.FileAccess.Read)))
        Return _Hash
    End Function

Как видите, я просто принимаю IO.Fileinfo в качестве аргумента, в начале функции я проверяю, чтобы убедиться, что это не ничто.

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

Спасибо.

Answer 1

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

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

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

Answer 2

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

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

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

Answer 3

Да, рекомендуется проверять все аргументы в начале метода и генерировать соответствующие исключения, такие как ArgumentException, ArgumentNullException или ArgumentOutOfRangeException.

Если метод является закрытым, так что только вы, программист, можете передать недопустимые аргументы, тогда вы можете утверждать, что каждый аргумент является допустимым (Debug.Assert) вместо throw.

Answer 4

Я добавлю пару уточнений ( выделено жирным шрифтом ) к превосходному дизайну по совету по контракту, предложенному Брайаном ранее ...

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

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

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

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

Answer 5

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

Классический дизайн по контракту: если ввод правильный, вывод будет правильный. Если ввод неправильный, есть ошибка. (Если ввод правильный, но вывод неправильный, есть ошибка. Это дай мне.)

Answer 6

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

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

То же самое относится и к любым другим предположениям.

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

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

Answer 7

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

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

Answer 8

Если значение NULL является недопустимым, выведите исключение. В одиночку, как вы сделали в своем примере, так что сообщение полезно.

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

Answer 9

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

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

Если ваши API доступны только доверенным абонентам, например, «внутренним» в C #, то вам не нужно писать весь этот дополнительный код. Это никому не пригодится.

Answer 10

В большинстве случаев достаточно просто выбросить исключение, если вы уверены, что исключение не будет проигнорировано.

Однако, если вы можете что-то добавить к этому, не повредит обернуть исключение более точным и отбросить его. Расшифровка «NullPointerException» займет немного больше времени, чем «IllegalArgumentException (« FilePath ДОЛЖЕН быть предоставлен »)» (или что угодно).

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

Мне бы очень хотелось увидеть модификатор "nullable" или "nonull" для переменных и аргументов, чтобы компилятор мог проверить вас.