Проверка входного параметра в вызывающей стороне: дублирование кода?

16

Где лучшее место для проверки входных параметров функции: в вызывающей программе или в самой функции?

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

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

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

Меня интересуют не только нулевые условия, но и проверка любых входных переменных (отрицательное значение для sqrtфункции, деление на ноль, неправильная комбинация состояния и почтового индекса или что-либо еще)

Существуют ли правила, как решить, где проверять условия ввода?

Я думаю о некоторых аргументах:

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

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

Srnka
источник

Ответы:

15

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

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

Возьмите следующий метод в качестве примера:

public void DeletePerson(Person p)
{            
    _database.Delete(p);
}

Что подразумевается под контрактом DeletePerson? Программист может только предполагать, что если что-либо Personпередано, оно будет удалено. Однако мы знаем, что это не всегда так. Что если pэто nullзначение? Что если pне существует в базе данных? Что делать, если база данных отключена? Следовательно, DeletePerson похоже, плохо выполняет свой контракт. Иногда он удаляет человека, а иногда выдает исключение NullReferenceException или DatabaseNotConnectedException, а иногда ничего не делает (например, если человек уже удален).

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

Вот несколько способов улучшить контракт:

  • Добавьте проверку и добавьте исключение к контракту. Это делает контракт сильнее , но требует, чтобы вызывающая сторона выполняла проверку. Разница, однако, в том, что теперь они знают свои требования. В этом случае я сообщаю об этом с помощью комментария C # XML, но вы можете вместо этого добавить throws(Java), использовать Assertили использовать инструмент контракта, такой как Code Contracts.

    ///<exception>ArgumentNullException</exception>
    ///<exception>ArgumentException</exception>
    public void DeletePerson(Person p)
    {            
        if(p == null)
            throw new ArgumentNullException("p");
        if(!_database.Contains(p))
            throw new ArgumentException("The Person specified is not in the database.");
    
        _database.Delete(p);
    }
    

    Примечание: аргумент против этого стиля часто заключается в том, что он вызывает чрезмерную предварительную проверку всего вызывающего кода, но по моему опыту это часто не так. Подумайте о сценарии, в котором вы пытаетесь удалить нулевого персонажа. Как это случилось? Откуда взялась нулевая персона? Например, если это пользовательский интерфейс, почему обрабатывается клавиша «Удалить», если нет текущего выбора? Если он уже был удален, разве он не должен был быть удален с дисплея? Очевидно, есть исключения из этого, но по мере роста проекта вы будете часто благодарить подобный код за предотвращение проникновения ошибок в систему.

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

    public void TryDeletePerson(Person p)
    {            
        if(p == null || !_database.Contains(p))
            return;
    
        _database.Delete(p);
    }
    
  • Объединить подходы. Иногда вам нужно немного и того и другого, когда вы хотите, чтобы внешние абоненты строго следовали правилам (чтобы заставить их отвечать за код), но вы хотите, чтобы ваш личный код был гибким.

    ///<exception>ArgumentNullException</exception>
    ///<exception>ArgumentException</exception>
    public void DeletePerson(Person p)
    {            
        if(p == null)
            throw new ArgumentNullException("p");
        if(!_database.Contains(p))
            throw new ArgumentException("The Person specified is not in the database.");
    
        TryDeletePerson(p);
    }
    
    internal void TryDeletePerson(Person p)
    {            
        if(p == null || !_database.Contains(p))
            return;
    
        _database.Delete(p);
    }
    

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

Кевин Маккормик
источник
Спасибо за очень хороший ответ с примером. Мне нравится точка зрения "оборонительных" и "строгих контрактов".
srnka
7

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

Не все функции одинаковы. Не все требования равны. Не все проверки равны.

Например, если ваш Java-проект старается по возможности избегать нулевых указателей (см. , Например, рекомендации по стилю Guava ), проверяете ли вы каждый аргумент функции, чтобы убедиться, что он не нулевой? Это, вероятно, не обязательно, но есть вероятность, что вы все равно делаете это, чтобы упростить поиск ошибок. Но вы можете использовать assert, где ранее вы генерировали исключение NullPointerException.

Что если проект находится на C ++? Соглашение / традиция в C ++ состоит в том, чтобы задокументировать предварительные условия, но только проверять их (если вообще) в отладочных сборках.

В любом случае у вас есть задокументированное предварительное условие для вашей функции: ни один аргумент не может быть нулевым. Вместо этого вы можете расширить область функции, включив в нее значения null с определенным поведением, например, «если какой-либо аргумент имеет значение null, выдается исключение». Конечно, это опять-таки мое наследие C ++, о котором я говорю - в Java это достаточно распространено, чтобы задокументировать предварительные условия таким образом.

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

Единственное реальное место, где вам нужны явные проверки - это границы. Внешний вклад в ваш проект? Утвердить, подтвердить, подтвердить. Серая область - это границы API. Это действительно зависит от того, насколько вы хотите доверять клиентскому коду, какой ущерб наносит неправильный ввод, и какую помощь вы хотите оказать в поиске ошибок. Разумеется, любая граница привилегий должна учитываться как внешняя - системные вызовы, например, выполняются в контексте повышенных привилегий и поэтому должны быть очень осторожны при проверке. Любая такая проверка должна, конечно, быть внутренней по отношению к системному вызову.

Себастьян Редл
источник
Спасибо за Ваш ответ. Можете ли вы дать ссылку на рекомендацию стиля гуавы? Я не могу Google и выяснить, что вы имели в виду под этим. +1 для проверки границ.
srnka
Добавлена ​​ссылка. На самом деле это не полное руководство по стилю, а лишь часть документации по ненулевым утилитам.
Себастьян Редл
6

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

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

Бернард
источник
Спасибо за ответ. Таким образом, вы думаете, что функция должна проверять как действительные, так и недействительные входные параметры в каждом случае. Что-то отличное от утверждения книги Pragmatic Programmer: «проверка входного параметра является обязанностью вызывающего». Это хорошая мысль: «Функция должна знать, что считается действительным ... Вызывающие могут не знать этого» ... Так что вам не нравится использовать предварительные условия?
srnka
1
Вы можете использовать предварительные условия, если хотите (см . Ответ Себастьяна ), но я предпочитаю защищаться и обрабатывать любые возможные варианты ввода.
Бернард
4

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

Более того, если функция обновляется таким образом, что это повлияет на валидацию параметра, вам придется искать каждое вхождение валидации вызывающего абонента, чтобы обновить их. Это не мило :-).

Вы можете обратиться к пункту охраны

Обновить

Смотрите мой ответ для каждого предоставленного вами сценария.

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

    Ответ

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

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

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

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

    Ответ

    Да, это хорошая практика, чтобы избежать разброса проверки параметров по всему коду.

  • проверка входного параметра в вызывающей стороне происходит только одна перед вызовом многих функций с этим параметром. Поэтому проверка параметра в каждой функции не эффективна

    Ответ

    Было бы неплохо, если бы вызывающая была функцией, не так ли?

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

  • правильное решение зависит от конкретного случая

    Ответ

    Цель для поддерживаемого кода. Перемещение проверки вашего параметра обеспечивает один источник правды о том, что функция может принять или нет.

OnesimusUnbound
источник
Спасибо за ответ. Sqrt () был только примером, такое же поведение с входным параметром может использоваться многими другими функциями. «если функция обновляется таким образом, что это повлияет на валидацию параметра, вам придется искать каждое вхождение валидации вызывающей стороны» - я с этим не согласен. Затем мы можем сказать то же самое для возвращаемого значения: если функция обновляется таким образом, что это повлияет на возвращаемое значение, вы должны исправить каждого вызывающего ... Я думаю, что функция должна иметь одну четко определенную задачу для выполнения ... В противном случае в любом случае необходимо изменить номер вызывающего абонента.
srnka
2

Функция должна указывать свои предварительные и последующие условия.
Предварительные условия - это условия, которые должны быть выполнены вызывающим абонентом, прежде чем он сможет правильно использовать функцию и может (и часто делает) включать в себя достоверность входных параметров.
Постусловия - это обещания, которые функция дает своим вызывающим.

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

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

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

Барт ван Инген Шенау
источник
+1 Спасибо за ответ. Хорошее размышление: «Проверка в вызываемом файле приводит к дублированию кода и выполнению большого количества ненужной работы». И в предложении: «В большинстве случаев не требуется никаких явных тестов, потому что внутренняя логика и предварительные условия вызывающего уже гарантируют» - что вы имеете в виду под выражением «внутренняя логика»? Функциональность DBC?
srnka
@srnka: Под «внутренней логикой» я подразумеваю вычисления и решения в функции. По сути, это реализация функции.
Барт ван Инген Шенау
0

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

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

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