Comments 72
Ещё можно купить уже решарпер и разметить код аннотациями. Тогда тот сам будет подсказывать, откуда можно ждать null, и ругаться при попытке передать его не туда, куда нужно.
Можно также воспользоваться CodeContracts. Это средство более мощное.
Но не стоит забывать, что и ваш способ и способ с CodeContracts — это дополнительные инвестиции в проект.
Но не стоит забывать, что и ваш способ и способ с CodeContracts — это дополнительные инвестиции в проект.
Решарпер своих денег стоит и окупается за пару месяцев возросшей за счёт него продуктивности.
Я и не говорил про Решарпер. Хотя его цену надо также учитывать. А ещё его эффективность надо доказать. Я его сам использую, купил лицензию. Вы замеряли рост производительности?
Я, в целом, говорил о том, что и в описанном вами способе и мной появляется дополнительный код. Любой дополнительный код требует сопровождения и сам же является источником потенциальных ошибок (например, ослабление контракта кем-то из коллег).
Я, в целом, говорил о том, что и в описанном вами способе и мной появляется дополнительный код. Любой дополнительный код требует сопровождения и сам же является источником потенциальных ошибок (например, ослабление контракта кем-то из коллег).
У вас 2 сообщения подряд сияет посыл «купить решарпер», завязывайте с этим. Это удел кирби.
Не хотите покупать — спиратьте. Мне больно смотреть, как люди делают то, что должна быть занята автоматика.
Если не дошло говорю прямым текстом — хорош рекламировать платные продукты. Тут серьёзные статьи за такое банят и в соотв. хаб направляют. Дошло?
А что покупать я как и любой другой хабражитель как нибудь без сопливых решим, и то что вам на что либо больно смотреть никак на подобные решения не повлияет — такие излияния в личном блоге пишите.
А что покупать я как и любой другой хабражитель как нибудь без сопливых решим, и то что вам на что либо больно смотреть никак на подобные решения не повлияет — такие излияния в личном блоге пишите.
Есть еще замечательный IntelliJ IDEA от того же самого JetBrains (есть и бесплатная версия, но не уверен, что оно там есть), для Java — тоже аннотации, которые спасают от ряда ситуаций, описанных в статье, хотя, конечно, не панацея. Плюс к этому можно обработать исходники в post-compile, добавив assert-подобные конструкции — всяко лучше, чем отхватить NPE.
В Eclipse, кстати, тоже есть такие аннотации; но чтобы была польза, их придется расставлять по всему проекту.
Для дотнета есть штука под названием PostSharp, которая встраивается как постсборочный таск и преобразует атрибуты аннотаций в исполняемый код. Помимо стандарных ассертов можно делать свои атрибуты с обработчиками, вызываемыми на входе и на выходе из функции, таким образом привнося в язык аспектно ориентированное программирование. Причём даже вносит правки в отладочные символы. Стоит, правда, как авианосец, так что в боевых проектах пока не пробовал.
Вы о стоимости PostSharp или о стоимости написания своей инфраструктуры для поддержки АОП?
Я так понял, что PostSharp — это что-то вроде KindOfMagic, только с большими возможностями?
Да, чем-то похоже. Но ради одного INotifyPropertyChanged городить целый post-build таск? У меня для этих целей Reflectiin.Emit-кодогенератор на коленке за пару часов написанный используется сто лет уже.
Код на гитхабе/пастебине имеется?
Сегодня освобожу от зависимости от инфраструктуры кодогенерации и выложу.
github.com/kekekeks/NotifyHelper — выложил тут. А, да. Оно сделано так, что дата биндинги не отваливаются после обфускации — если генератор видит атрибут PropertyName, то не только будет использовать его как имя для события, но сгенерирует в итоговом классе свойство с таким именем, которое в дальнейшем доступно через Reflection.
Представьте, что вы получаете отчёт об ошибках с вашей системы, находящейся в эксплуатации. Трассировка стека указывает на метод GetProductDetails и типом выброшенного исключения является NullReferenceException. Какой из шести возможных объектов с нулевой ссылкой стал причиной ошибки?
Неужели по номеру строки, с которой вылетело исключение, этого никак нельзя понять?
Я бы не стал всерьёз ориентироваться на номер строки. Если номера строк совпадут — хорошо. А если с последней версии номера строк поменялись, то вам, как минимум, придётся лезть в историю через source control средство.
Не вижу проблемы слазить в историю. Мы же всегда достоверно знаем, что именно где сейчас стоит и как получить еще одну точно такую же сборку, не правда ли?
У меня вот с Android'a полный callstack приходит, с номерами строк, методами, текущей версией приложения, которое, собственно, крашнулось и прочей дополнительной информацией.
Неужели на шарпе (desktop || ASP || на_что_оно_еще_делится?) так же нельзя?
Неужели на шарпе (desktop || ASP || на_что_оно_еще_делится?) так же нельзя?
Вы поставляете отладочные сборки в комплекте с отладочными символами?
Ну у нас же тут вроде бы управляемый код, всё это идет в комплекте и я не уверен, что отключаемо. Ошибаюсь?
1) сборка может быть отладочной и релизной, отличие релизной в том, что в результате применения оптимизаций строки могут не всегда корректно определяться.
2) отладочная информация находится в лежащих рядом со сборками pdb(mdb в случае использования компилятора dmcs), которые с приложением обычно не поставляют, впрочем, в случае веб-приложений это не составляет проблем
2) отладочная информация находится в лежащих рядом со сборками pdb(mdb в случае использования компилятора dmcs), которые с приложением обычно не поставляют, впрочем, в случае веб-приложений это не составляет проблем
Кстати, да, вы правы. Совсем об этом забыл)))
Не знаю, как с этим дела обстоят в шарпе, но в плюсовом проекте мы хранили отдельно символы для каждого из билдов, и тогда при крэше определенной сборки можно было просто подключить нужные символы и загнать в отладчик. Сами бинари в пакетах были, разумеется, стрипнутые.
Разве окончательный вариант — не то же самое, что валидация входных данных? Или это другое название одного и того же?
Узнал, что я приверженец защитного программирования :) На новой работе столкнулся с интересной практикой: код должен валиться только в действительно исключительных ситуациях типа невозможности отобразить UI. В подавляющем же большинстве остальных ситуациях, проблемный код должен быть просто проигнорирован, то есть исключения ловятся, но не обрабатываются вообще (sic!) — ни сообщений пользователю, ни записей в лог (на сервере, речь о веб-приложении).
А вот отсутствие записей в логе вам точно аукнется. Типичная ситуация вида «ничего не работает, но ничего не известно». Боретесь как-нибудь с этой ситуацией? Ну, может, стиль кодирования какой-то особенно устойчивый?
Один приверженец защитного программирования пытался ввести на предприятии стандарт кодирования, где нельзя была написать:
По его мнению нужно было в обязательном порядке писать так:
Даже если else программисту не нужно. Во всех if.
if (условие)
{
код
}
По его мнению нужно было в обязательном порядке писать так:
if (условие)
{
код
}
else
{
;
}
Даже если else программисту не нужно. Во всех if.
и помогло? (а вдруг..)
Настолько я не заморачиваюсь, но постоянно пишу что-то вроде
switch ($var) {
case 'val1':
// no break;
case 'val2:
doSmth();
break;
// no default
}
Очень полезная, кстати, практика, и защитное програмирование тут не причем.
А в else лучше писать короткий коммент, объясняющий почему else пустой.
Исключением могут быть случаи коротких if, заканчивающихся return или throw,
Например, при проверке аргументов.
И да, нам помогает. На мой взгляд, это сопоставимо с требованием всегда использовать фигурные скобки после if и else даже если в блоке только одна строка.
А в else лучше писать короткий коммент, объясняющий почему else пустой.
Исключением могут быть случаи коротких if, заканчивающихся return или throw,
Например, при проверке аргументов.
И да, нам помогает. На мой взгляд, это сопоставимо с требованием всегда использовать фигурные скобки после if и else даже если в блоке только одна строка.
Категорически с этим не согласен.
А в else лучше писать короткий коммент, объясняющий почему else пустой.
if (!isset($a)) {
$a = 1; // default value
} else {
; // we have value and use it as is
}
Так что ли? По-моему бредово.
Просмотрел первый попавшийся файл. Из 43 if-ов только в 6 был else, 10 кончались на return, break или continue, а остальные 27 — просто отработка возникшей ситуации, не требующей разбора случая, когда ситуация не возникла. Причём два else (и один if без else) можно убрать, заменив на switch. Так что else останется в 10% случаев. Если каждый из остальных снабдить else, да ещё и с фигурными скобками — разобраться в коде будет заметно сложнее.
В Google Guava есть метод T checkNotNull(T reference), который делает проверку на не null и возвращает сам этот объект. С его помощью такие вот защитные проверки становятся намного компактнее:
Наверняка в C# есть что-то аналогичное
this.userRepository = checkNotNull(userRepository);
Наверняка в C# есть что-то аналогичное
Да, можно сделать такой ThrowIfNull Extension Method
Подобное можно и самому написать. Вопрос в том, что делать, если всё-таки окажется Null.
Странно, что утверждения упоминаются в открывающей цитате, но не в тексте. В шарпе утверждения, конечно, не самые красивые, но есть. Едва ли имеет смысл использовать if и throw, там где можно обойтись Assert. Собственно, они для сохранения инвариантов и нужны.
Однажды столкнувшись с такой проблемой, мы использовали другой подход. Для необходимых типов данных мы завели пустой объект с необходимой информацией. После этого даже визуально можно было понять что объекта нет, или каких либо его свойств.
Т.е., вы использовали паттерн NullObject?
Иногда встречаю в чужом коде (и борюсь с желанием использовать в своем) анти-паттерн, проглатывающий ошибки. Что-то вроде такого:
Писать код в таком стиле подсказывает лень — непонятно, что делать с исключительной ситуацией, и разбираться в причинах ее возникновения тоже лень, поэтому давайте всё обмажем проверками, авось пронесет. В итоге баг неуловимым образом присутствует в программе годами.
void Process(Container c, int x)
{
if(c == null) return;
if(x <= 0) return;
c.DoStuff(x);
}
Писать код в таком стиле подсказывает лень — непонятно, что делать с исключительной ситуацией, и разбираться в причинах ее возникновения тоже лень, поэтому давайте всё обмажем проверками, авось пронесет. В итоге баг неуловимым образом присутствует в программе годами.
А за такое нужно руки с корнем отрывать. Ни исключения, ни сообщения в логе — вообще ничего! И поди пойми, исполнился метод DoStuff() или нет.
Легче станет, если будет
function Process(Container $c, SplInt $x)
{
if (is_null($c)) throw new NullArgumentException();
if (x <=0) throw new NonPositiveArgumentException();
$c->doStuff($x);
}
try {
Process ($c, $x);
} catch (Exception $e) {
}
Нет, разумеется, с чего бы? По-моему это достаточно очевидно, что просто выкинуть исключение (без должной его обработки) — недостаточно.
Я считаю, что должно быть достаточно просто выкинуть исключение — необработанное само попадет в логи/stderr в известных мне средах. Но тут не отсутствие обработки, тут явное игнорирование, причем общее, любого исключения.
>Но тут не отсутствие обработки, тут явное игнорирование, причем общее, любого исключения.
Ну так о том и речь. Кидаешь исключение — либо обрабатывай по-человечески, либо вообще не обрабатывай. А так ваш пример получается еще хуже оригинального: в оригинале хотя бы сразу видно, что кусок кода потенциально проблемный, а в вашем примере на первый взгляд кажется, что все нормально — мол, видите, исключения правильные кидаем!
Ну так о том и речь. Кидаешь исключение — либо обрабатывай по-человечески, либо вообще не обрабатывай. А так ваш пример получается еще хуже оригинального: в оригинале хотя бы сразу видно, что кусок кода потенциально проблемный, а в вашем примере на первый взгляд кажется, что все нормально — мол, видите, исключения правильные кидаем!
Сильный аргумент в пользу такого поведения — популярные браузеры. Встречая, не то что не валидный, но даже не велл-формед html или xml код они тупо игнорируют проблемы. Знаете ли хоть один ещё такой популярный софт, как браузеры, заюивающие на синтаксис? Оси популярнее, но они обычно очень трепетно относятся к неправильному обращению с собой, выбрасывая бдосы, сегфолты и прочие классные штуки.
Во-первых надо отличать вызов метода и конструктор. Дело в том, что конструктор создает объект, который «хранит состояние» между вызовами. Принципиально надо сводить всё к случаям, чтобы программа не могла в какое-то время находиться в несогласованном состоянии. Т.е. если в конструктор передаются ссылки null, а принципиально не может или не должен быть объект с null-cсылками — программа должна не создать объект, а упасть. Т.е. в конструкторе — да, проверки нужны.
В методе не обязательны. Пусть падает.
Но там говнокод в другом.
И как думаете, после каждого вызова метода Get надо такие проверки делать? Может лучше внутри Get поместить эту проверку? В Linq есть методы, которые хорошо называются, например: FirstOrDefault — в названии заключено, что ждать от метода. Это очень плохая практика — взять, назвать метод Get, а потом, ожидать, что из него не только объект, но и null возвращается. Если в каких-то случаях нужен будет метод, который возвратит «пустоту» в случае «не нашел», то лучше для этих случаев писать отдельный метод. Но из метода Get ожидать объект. Договориться как-то с названиями методов. Например, позволять null — метод GetIfExists(). А метод Get (или Single, или как договоритесь), обязательно возвращает.
И тогда и код чистый и DRY и смысла в коде больше, уверенности больше. Защитное программирование в общем-то не причем. Пусть падает и падает как можно быстрее. Это и мотив — написать проверки в конструкторе. В методах и так упадет, без разрыва по времени. Просто тяжелее искать, что упало. Но поддержка нормальных имен методов и по возможности вообще исключение null из всех мест, где по смыслу не может быть — это ключ к чистоте и надежности кода.
В методе не обязательны. Пусть падает.
Но там говнокод в другом.
var product = this.productRepository.Get(productId);
if (product == null)
throw new InvalidOperationException("Product was null.");
И как думаете, после каждого вызова метода Get надо такие проверки делать? Может лучше внутри Get поместить эту проверку? В Linq есть методы, которые хорошо называются, например: FirstOrDefault — в названии заключено, что ждать от метода. Это очень плохая практика — взять, назвать метод Get, а потом, ожидать, что из него не только объект, но и null возвращается. Если в каких-то случаях нужен будет метод, который возвратит «пустоту» в случае «не нашел», то лучше для этих случаев писать отдельный метод. Но из метода Get ожидать объект. Договориться как-то с названиями методов. Например, позволять null — метод GetIfExists(). А метод Get (или Single, или как договоритесь), обязательно возвращает.
И тогда и код чистый и DRY и смысла в коде больше, уверенности больше. Защитное программирование в общем-то не причем. Пусть падает и падает как можно быстрее. Это и мотив — написать проверки в конструкторе. В методах и так упадет, без разрыва по времени. Просто тяжелее искать, что упало. Но поддержка нормальных имен методов и по возможности вообще исключение null из всех мест, где по смыслу не может быть — это ключ к чистоте и надежности кода.
А, извините, невнимателен. В статье в конце о том же. Видимо у меня другая терминология. Не рассматриваю это как защитное программирование, а просто считаю, что null — это враг смысла в коде, который зачем-то всегда подается в коробке с классами и от которого нельзя избавиться. Поэтому по умолчанию у меня его нет. Методы не возвращают, если явно в имени это не указано и в этом явный смысл. Защита — это когда на вход методу подается «что-то не то» и мы защищаемся. Я же думаю — на выходе отдавать надо то, что просят, а если не можешь — кинуть исключение. Проверки только в местах сайд-эффектов — конструкторы или установки полей из методов. В общем, в статье с другой стороны это и описали.
Создавать надо два метода ради того, чтобы в ситуации, когда клиент точно знает, что значение точно вернётся из БД он вызывал Get, а если он не уверен, то GetIfExists, если я вас правильно понял?
И, если я вас действительно правильно понял, то как часто встречается ситуация, когда клиент точно уверен, что что-то вернётся? Третье чувство подсказывает мне, что таких случаев 1%.
И, если я вас действительно правильно понял, то как часто встречается ситуация, когда клиент точно уверен, что что-то вернётся? Третье чувство подсказывает мне, что таких случаев 1%.
Если Ваш проект — всё приложение, а не библиотека на продажу, то создавать надо только то, что используется. Даже если 1% использует. Если Вы как клиент точно уверены, что Вам нужен объект и он должен быть — вызываете Get. Или даже если не уверены, а подозреваете, что так должно быть. Работа приложения покажет. Надо выбирать более ограниченный вариант.
Потом, механически, в данном случае именно так и подозревалось: после вызова метода идет проверка на null. Как минимум, если в паре мест встречается такая проверка — DRY требует вынести в один метод. И можно даже писать такой метод (с выбрасыванием эксепшина) для поиска объектов по базе, которых заведомо может не быть. Создать свой эксепшин — NotFoundException. Тогда метод будет выглядеть так:
Таким образом клиентский код не будет загрязняться проверками. А выброшенное исключение по сути является частью логики ПО, а не ошибкой.
Вообще, null — это вещь плохая и ненужная. Оно по сути означает: ссылка на объект в куче не инициализирована. А то, что это «объект не найден к базе» — за уши притянуто. Null — это тип в типе. По смыслу — тип — это множество возможных значений. А когда метод может возвращать null, то это говорит, что метод может возвратить одно из двух: [Product | null]. Хотелось бы, чтобы если я подразумевал, что что-то не нашел в базе, например, то я бы сам указывал это в коде явно типом: [Product | NotFound]. А так, получается, что кишки устройства дотнет присутствуют везде в коде и везде ведется борьба в коде с возможными ненужными null.
Вообще, мой посыл в комментарии был в том, что код должен выражать смысл задачи. Никакого защитного программирования. Если Вы ожидаете от метода что-то, то ровно это одно метод и должен возвращать, а не неявно подсовывать иногда null. Тогда код остается чистым автоматически. К сожалению, от ссылок null не избавиться языковыми средствами шарпа, поэтому иногда приходится в установках полей писать проверки. Не было бы этого уродства в шарпе, не было бы и там проверок. И отлично, надежно проверял бы корректность программы компилятор, а не появлялись ошибки в рантайме.
А проверять входные параметры, как принято в защитном программировании — не нужно (не обязательно). Если создается объект или в нем меняются поля, то это состояние может сохраняться между вызовами и потом ошибку, кем она внесена, будет тяжело найти. А локальные переменные, в том числе и параметры, не отвечающие требованиям, заставят и так упасть программу сразу же. Проверки захламляют код. Иногда они разве что дают более внятное описание ошибки. Но это не особая проблема. Есть стек. А если бы не было null, так вообще все сообщения об ошибках были более менее читаемыми.
Защитное программирование хорошо в языках, где ошибка может привести к неопределенному поведению, а не падению. В языках, где программно управляете памятью. Вот там, в С++/С, надо опасаться всего.
В статье по сути пришли к этому же, что я написал, как бы убрав почти везде защитное программирование, хотя исходили из него вначале.
Потом, механически, в данном случае именно так и подозревалось: после вызова метода идет проверка на null. Как минимум, если в паре мест встречается такая проверка — DRY требует вынести в один метод. И можно даже писать такой метод (с выбрасыванием эксепшина) для поиска объектов по базе, которых заведомо может не быть. Создать свой эксепшин — NotFoundException. Тогда метод будет выглядеть так:
Product Get(int productId)
{
var product = GetIfExist(productId);
if (product == null)
{
throw new NotFoundException(...);
}
return product;
}
Таким образом клиентский код не будет загрязняться проверками. А выброшенное исключение по сути является частью логики ПО, а не ошибкой.
Вообще, null — это вещь плохая и ненужная. Оно по сути означает: ссылка на объект в куче не инициализирована. А то, что это «объект не найден к базе» — за уши притянуто. Null — это тип в типе. По смыслу — тип — это множество возможных значений. А когда метод может возвращать null, то это говорит, что метод может возвратить одно из двух: [Product | null]. Хотелось бы, чтобы если я подразумевал, что что-то не нашел в базе, например, то я бы сам указывал это в коде явно типом: [Product | NotFound]. А так, получается, что кишки устройства дотнет присутствуют везде в коде и везде ведется борьба в коде с возможными ненужными null.
Вообще, мой посыл в комментарии был в том, что код должен выражать смысл задачи. Никакого защитного программирования. Если Вы ожидаете от метода что-то, то ровно это одно метод и должен возвращать, а не неявно подсовывать иногда null. Тогда код остается чистым автоматически. К сожалению, от ссылок null не избавиться языковыми средствами шарпа, поэтому иногда приходится в установках полей писать проверки. Не было бы этого уродства в шарпе, не было бы и там проверок. И отлично, надежно проверял бы корректность программы компилятор, а не появлялись ошибки в рантайме.
А проверять входные параметры, как принято в защитном программировании — не нужно (не обязательно). Если создается объект или в нем меняются поля, то это состояние может сохраняться между вызовами и потом ошибку, кем она внесена, будет тяжело найти. А локальные переменные, в том числе и параметры, не отвечающие требованиям, заставят и так упасть программу сразу же. Проверки захламляют код. Иногда они разве что дают более внятное описание ошибки. Но это не особая проблема. Есть стек. А если бы не было null, так вообще все сообщения об ошибках были более менее читаемыми.
Защитное программирование хорошо в языках, где ошибка может привести к неопределенному поведению, а не падению. В языках, где программно управляете памятью. Вот там, в С++/С, надо опасаться всего.
В статье по сути пришли к этому же, что я написал, как бы убрав почти везде защитное программирование, хотя исходили из него вначале.
UFO just landed and posted this here
Sign up to leave a comment.
Защитное программирование