Всегда проверяйте параметры и создавайте исключения - PullRequest
9 голосов
/ 21 октября 2008

Стоит ли всегда проверять параметры и генерировать исключения в .NET, когда параметры не соответствуют вашим ожиданиям? Например. нулевые объекты или пустые строки?

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

Я заканчиваю тем, что выбрасываю множество исключений ArgumentNullException («name»), даже если код, обрабатывающий исключение, не может на самом деле ничего сделать программно, поскольку нет гарантии, что «name» не изменится в будущем.

Я полагаю, эта информация просто полезна при просмотре журнала, содержащего информацию об исключениях?

Лучше ли всегда быть «равнодушным к худшему».

Ответы [ 9 ]

12 голосов
/ 21 октября 2008

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

6 голосов
/ 21 октября 2008

Для public методов, тогда да: обязательно нужно проверить ваши аргументы; для внутренних / частных вызовов Эрик Липперт мог бы классифицировать их как «безумные» ( здесь ); его совет не поймать их ... исправить код!

Чтобы избежать раздувания кода, вы можете рассмотреть AOP, например postsharp . Чтобы проиллюстрировать это, у Джона Скита есть атрибут postsharp, который выполняет проверку нулевого аргумента, здесь . Затем (чтобы процитировать его пример) вы можете просто приписать метод:

[NullArgumentAspect("text")]
public static IEnumerable<char> GetAspectEnhancedEnumerable(string text)
{ /* text is automatically checked for null, and an ArgumentNullException thrown */ }

Другим удобным трюком здесь могут быть методы расширения; Методы расширения имеют любопытную особенность: их можно вызывать в пустых экземплярах ... поэтому вы можете делать следующее (где использование обобщений вместо «объекта» позволяет избежать случайного вызова из-за блокировки типа значения) :

static void ThrowIfNull<T>(this T value, string name) where T : class
{
    if (value == null) throw new ArgumentNullException(name);
}
// ...
stream.ThrowIfNull("stream");

И вы можете делать подобные вещи за пределами диапазона и т. Д.

5 голосов
/ 21 октября 2008

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

3 голосов
/ 21 октября 2008

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

1 голос
/ 21 октября 2008

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

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

Тем не менее, в C / C ++ я нахожу, что конкретный случай проверки на нулевые указатели является лишь незначительной помощью. Если кто-то передает вам указатель, то условие полной действительности не "не должно быть нулевым". Он должен указывать на память, которая может быть прочитана (возможно, также записана) текущим процессом до определенного размера и содержит правильный тип объекта, возможно, в определенном подмножестве всех возможных состояний. Его не нужно освобождать, не уничтожать из-за переполнения буфера в другом месте, возможно, чтобы он не был одновременно изменен другим потоком и т. Д. Вы не собираетесь проверять все это при входе в метод, поэтому вы все равно можете пропустить неверный параметры. Все, что заставляет вас или других программистов думать, что «этот указатель не является нулевым, поэтому он должен быть действительным», поскольку вы протестировали только одну небольшую часть условия допустимости, вводит в заблуждение.

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

Конечно, в некоторых языках вы не проходите мимо указателя, и у вызывающей стороны есть только ограниченные возможности повредить память, поэтому меньше мусора. Но в Java, например, передача неправильного объекта все еще является более распространенной ошибкой программирования, чем передача ошибочного нуля. В любом случае, значения Null обычно довольно легко диагностировать, если оставить их на время выполнения, чтобы определить их и посмотреть на трассировку стека. Таким образом, значение нулевой проверки весьма ограничено даже там. В C ++ и C # вы можете использовать передачу по ссылке, где нулевые значения будут запрещены.

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

1 голос
/ 21 октября 2008

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

Для любых непублично видимых членов (т. Е. internal или private членов или классов) я использую Debug.Assert вместо выполнения проверки. Таким образом, если кто-либо из вызывающих в сборке нарушает контракт, который вы немедленно обнаруживаете во время разработки / тестирования, но у вас нет никаких проблем с производительностью в конечном развернутом решении, поскольку эти операторы удалены в сборках RELEASE.

1 голос
/ 21 октября 2008

Моя философия в этой ситуации - уведомлять и продолжать (когда это применимо). Псевдокод будет:

if value == not_valid then
#if DEBUG
  log failure
  value = a_safe_default_value
#elsif RELASE
  throw
#endif
end

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

0 голосов
/ 20 декабря 2018

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

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

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

0 голосов
/ 21 октября 2008

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...