Какова соответствующая сумма проверки ошибок?
public void PublicMethod(FooBar fooBar)
{
if (fooBar == null)
throw new ArgumentNullException("fooBar", "fooBar cannot be null");
// log the call [added: Thanks S.Lott]
_logger.Log("PublicMethod called with fooBar class " + fooBar.Classification);
int action = DetermineAction();
PrivateMethod(fooBar, action);
}
private void PrivateMethod(FooBar fooBar, int action)
{
if (fooBar == null)
throw new ArgumentNullException("fooBar", "fooBar cannot be null"); // Is this line superfluous?
/*
Do something
*/
}
Можно ли пропустить такую проверку ошибок в частных методах, если входные данные уже проверены в открытом интерфейсе? Обычно есть какое-то эмпирическое правило, которое можно пройти...
Редактировать:
Возможно, ArgumentNullException не очень хороший пример, потому что можно привести аргумент, что вы должны проверять оба уровня, но возвращать разные сообщения об ошибках.
9 ответов
Я бы сказал нет.
Хотя, безусловно, верно, что вы в этом случае знаете, что он уже проверен на обнуляемость, через два месяца самый молодой стажер приедет и напишет PublicMethod2, который также вызывает PrivateMethod, но вот, он забыл проверить наличие нуля.
Поскольку публичный метод на самом деле не использует foobar, я не уверен, почему он проверяет. Текущий частный метод заботится, но это частная обязанность заботиться. Действительно, весь смысл частного метода состоит в том, чтобы делегировать ему все обязанности.
Метод проверяет ввод, который он фактически использует; он не проверяет вещи, через которые проходит.
Если у другого подкласса есть тот же открытый метод, но другая реализация частного метода - который может допускать пустые значения - что теперь? У вас есть открытый метод, который теперь имеет неправильные ограничения для нового подкласса.
Вы хотите сделать как можно меньше в публичном методе, чтобы различные частные реализации могли делать правильные вещи. Не "перепроверять" или "просто в случае" проверки. Делегировать ответственность.
Я бы ошибочно проверил все, что вы можете, вы никогда не знаете, когда может произойти что-то, о чем вы не задумывались. (и это лучше, чем потом сожалеть)
При использовании дизайна по контракту ( http://en.wikipedia.org/wiki/Design_by_contract), как правило, клиент (публичный метод) несет ответственность за корректный вызов, то есть передачу допустимых параметров. В этом конкретном сценарии это зависит от того, принадлежит ли null набор допустимых входных значений, поэтому есть 3 варианта:
1) Нулевое допустимое значение: создание исключений или ошибок означало бы разрыв контракта, сервер (закрытый метод) должен обработать нулевое значение и не должен жаловаться.
2) Нулевое значение является недопустимым значением и передается кодом, находящимся в вашем контроле: решать серверу (частный метод), как реагировать. Очевидно, что генерирование исключения является более изящным способом обработки ситуации, но оно требует затрат на обработку этого исключения где-то еще в стеке. Исключения - не лучший способ справиться с нарушением контракта, вызванным ошибками в программировании. Вы действительно должны создавать исключения не тогда, когда контракт уже нарушен, а когда он не может быть выполнен из-за проблем с окружающей средой, которые не могут контролироваться в программном обеспечении. Грубые ошибки лучше обрабатывать, вставляя утверждение в начало приватного метода, чтобы проверить, что параметр не является нулевым. Это снизит сложность вашего кода, не будет необходимости обрабатывать исключение вверх по стеку и позволит достичь цели выделения нарушенных контрактов во время тестирования.
3) Затем идет защитное программирование ( http://en.wikipedia.org/wiki/Defensive_programming). При работе с параметрами, передаваемыми внешним кодом вне вашего контроля, непосредственный уровень вашего кода должен запускать параноидальный уровень проверок и возвращать ошибки в соответствии с контрактом на связь с внешним миром. Затем, углубляясь в слои кода, не представленные извне, все же имеет больше смысла придерживаться программирования по контракту.
В тех случаях, когда PrivateMethod будет вызываться часто с вводом, который уже был проверен, и только редко с вводом пользователя, тогда я бы использовал концепцию PublicMethod/PrivateMethod без проверки ошибок на PrivateMethod (и с PublicMethod, не делая ничего другого, кроме проверки параметров и вызов PrivateMethod)
Я бы также назвал закрытый метод чем-то вроде PublicMethod_impl (для "реализации"), так что ясно, что это метод внутреннего использования / без проверки.
Я утверждаю, что этот дизайн приводит к более надежному приложению, поскольку заставляет задуматься о том, что и когда проверялось. Слишком часто люди, которые всегда проверяют параметры, попадают в ловушку "Я проверил что-то, поэтому я проверил все".
Как пример этого, бывший сотрудник (программирование на C), прежде чем использовать указатель, всегда проверял бы, было ли оно нулевым. Как правило, указатели в его коде были инициализированы как запуск и никогда не менялись, поэтому шансы на его нулевое значение были довольно низкими. Кроме того, указатель имеет одно правильное значение и 65535 возможных неправильных значений, и он проверял только одно из этих неправильных значений.
Это зависит, если нулевое значение указывает на ошибку для метода. Помните, что методы также могут называться сообщениями для объекта; они также влияют на состояние объекта. Параметры могут специализировать вид отправляемого сообщения.
- Если publicMethod() не использует параметр и изменяет состояние экземпляра, в то время как privateMethod() использует этот параметр, не считайте его ошибкой в publicMethod, а делайте это в privateMethod().
- Если publicMethod() не меняет состояние, считайте это ошибкой.
Вы могли видеть последний случай как обеспечение интерфейса для внутреннего функционирования объекта.
Вы также можете пометить "приватный" метод как приватный или защищенный.
Я бы посчитал ответ "да, сделай проверку снова", потому что:
- Закрытый член может быть снова использован в будущем по другому пути через код, поэтому защититесь от этой ситуации.
- Если вы выполняете модульные тесты на частные методы
Мое представление могло бы измениться, если бы у меня был статический анализатор, который мог бы поднять это и не помечать потенциальное использование нулевой ссылки в приватном методе.
По крайней мере, оставьте комментарий, что PrivateMethod должен иметь ненулевой FooBar, и что PublicMethod проверяет это.