Pull to refresh

Comments 72

Ещё можно купить уже решарпер и разметить код аннотациями. Тогда тот сам будет подсказывать, откуда можно ждать null, и ругаться при попытке передать его не туда, куда нужно.
Можно также воспользоваться CodeContracts. Это средство более мощное.
Но не стоит забывать, что и ваш способ и способ с CodeContracts — это дополнительные инвестиции в проект.
Решарпер своих денег стоит и окупается за пару месяцев возросшей за счёт него продуктивности.
Я и не говорил про Решарпер. Хотя его цену надо также учитывать. А ещё его эффективность надо доказать. Я его сам использую, купил лицензию. Вы замеряли рост производительности?
Я, в целом, говорил о том, что и в описанном вами способе и мной появляется дополнительный код. Любой дополнительный код требует сопровождения и сам же является источником потенциальных ошибок (например, ослабление контракта кем-то из коллег).
У вас 2 сообщения подряд сияет посыл «купить решарпер», завязывайте с этим. Это удел кирби.
Не хотите покупать — спиратьте. Мне больно смотреть, как люди делают то, что должна быть занята автоматика.
Если не дошло говорю прямым текстом — хорош рекламировать платные продукты. Тут серьёзные статьи за такое банят и в соотв. хаб направляют. Дошло?
А что покупать я как и любой другой хабражитель как нибудь без сопливых решим, и то что вам на что либо больно смотреть никак на подобные решения не повлияет — такие излияния в личном блоге пишите.
Простите, а Вы кто такой, собственно, чтобы говорить за хабражителей? Я вот тоже всеми руками и ногами за решарпер, например. И действительно, мнение «сопливых», как Вы выразились, меня не волнует.
Есть еще замечательный IntelliJ IDEA от того же самого JetBrains (есть и бесплатная версия, но не уверен, что оно там есть), для Java — тоже аннотации, которые спасают от ряда ситуаций, описанных в статье, хотя, конечно, не панацея. Плюс к этому можно обработать исходники в post-compile, добавив assert-подобные конструкции — всяко лучше, чем отхватить NPE.
В Eclipse, кстати, тоже есть такие аннотации; но чтобы была польза, их придется расставлять по всему проекту.
Для дотнета есть штука под названием PostSharp, которая встраивается как постсборочный таск и преобразует атрибуты аннотаций в исполняемый код. Помимо стандарных ассертов можно делать свои атрибуты с обработчиками, вызываемыми на входе и на выходе из функции, таким образом привнося в язык аспектно ориентированное программирование. Причём даже вносит правки в отладочные символы. Стоит, правда, как авианосец, так что в боевых проектах пока не пробовал.
Вы о стоимости PostSharp или о стоимости написания своей инфраструктуры для поддержки АОП?
О стоимости PostSharp. АОП обычно прикручиваю с рантайм-кодогенерацией классов-врапперов. Дёшево, сердито, отладочные символы не портятся, готовые решения есть.
Я так понял, что PostSharp — это что-то вроде KindOfMagic, только с большими возможностями?
Да, чем-то похоже. Но ради одного INotifyPropertyChanged городить целый post-build таск? У меня для этих целей Reflectiin.Emit-кодогенератор на коленке за пару часов написанный используется сто лет уже.
Код на гитхабе/пастебине имеется?
Сегодня освобожу от зависимости от инфраструктуры кодогенерации и выложу.
github.com/kekekeks/NotifyHelper — выложил тут. А, да. Оно сделано так, что дата биндинги не отваливаются после обфускации — если генератор видит атрибут PropertyName, то не только будет использовать его как имя для события, но сгенерирует в итоговом классе свойство с таким именем, которое в дальнейшем доступно через Reflection.
Представьте, что вы получаете отчёт об ошибках с вашей системы, находящейся в эксплуатации. Трассировка стека указывает на метод GetProductDetails и типом выброшенного исключения является NullReferenceException. Какой из шести возможных объектов с нулевой ссылкой стал причиной ошибки?

Неужели по номеру строки, с которой вылетело исключение, этого никак нельзя понять?
Я бы не стал всерьёз ориентироваться на номер строки. Если номера строк совпадут — хорошо. А если с последней версии номера строк поменялись, то вам, как минимум, придётся лезть в историю через source control средство.
Не вижу проблемы слазить в историю. Мы же всегда достоверно знаем, что именно где сейчас стоит и как получить еще одну точно такую же сборку, не правда ли?
Хеш-суммы сборок, собранных из одних и тех же исходников имеет привычку различаться. Не думаю, что изменения затрагивают msil, но тем не менее
Если речь идет о Java, можно добавить шаг сборки проекта в дженкинсе, чтобы внутрь артефакта упаковывался текстовый файл с git SHA1, да и номером сборки до кучи.

UPD: Прошу прощения, уже увидел, что о C#. Мне кажется, для его сборочницы можно тоже что-то подобное сообразить.
У меня вот с Android'a полный callstack приходит, с номерами строк, методами, текущей версией приложения, которое, собственно, крашнулось и прочей дополнительной информацией.
Неужели на шарпе (desktop || ASP || на_что_оно_еще_делится?) так же нельзя?
Вы поставляете отладочные сборки в комплекте с отладочными символами?
Ну у нас же тут вроде бы управляемый код, всё это идет в комплекте и я не уверен, что отключаемо. Ошибаюсь?
1) сборка может быть отладочной и релизной, отличие релизной в том, что в результате применения оптимизаций строки могут не всегда корректно определяться.
2) отладочная информация находится в лежащих рядом со сборками pdb(mdb в случае использования компилятора dmcs), которые с приложением обычно не поставляют, впрочем, в случае веб-приложений это не составляет проблем
Кстати, да, вы правы. Совсем об этом забыл)))
Не знаю, как с этим дела обстоят в шарпе, но в плюсовом проекте мы хранили отдельно символы для каждого из билдов, и тогда при крэше определенной сборки можно было просто подключить нужные символы и загнать в отладчик. Сами бинари в пакетах были, разумеется, стрипнутые.
Разве окончательный вариант — не то же самое, что валидация входных данных? Или это другое название одного и того же?
Узнал, что я приверженец защитного программирования :) На новой работе столкнулся с интересной практикой: код должен валиться только в действительно исключительных ситуациях типа невозможности отобразить UI. В подавляющем же большинстве остальных ситуациях, проблемный код должен быть просто проигнорирован, то есть исключения ловятся, но не обрабатываются вообще (sic!) — ни сообщений пользователю, ни записей в лог (на сервере, речь о веб-приложении).
Э-э-э… Я как-то затрудняюсь даже предположить, зачем вы мне этой ссылкой ответили?
Чтобы показать, как используется это слово. А еще в конце утвердительных предложений ставится точка.
Я именно так как по вашей ссылке написано его и использовал.

А с пунктуацией у меня всегда проблемы были — последовательность с десяток-другой символов я ещё могу запомнить, но вот на знаки препинания памяти не хватает.
А вот отсутствие записей в логе вам точно аукнется. Типичная ситуация вида «ничего не работает, но ничего не известно». Боретесь как-нибудь с этой ситуацией? Ну, может, стиль кодирования какой-то особенно устойчивый?
Ну, собственно, я и пытаюсь продвинуть позицию «фиг с ним с пользователем, но хоть для нас самих давайте в логи писать об ошибках, обработку (или отсутствие её) которых мы не предусмотрели».
Один приверженец защитного программирования пытался ввести на предприятии стандарт кодирования, где нельзя была написать:
if (условие)
{
  код
}

По его мнению нужно было в обязательном порядке писать так:
if (условие)
{
  код
}
else
{
  ;
}

Даже если else программисту не нужно. Во всех if.
и помогло? (а вдруг..)
Настолько я не заморачиваюсь, но постоянно пишу что-то вроде
switch ($var) {
    case 'val1':
        // no break;
    case 'val2:
        doSmth();
        break;
    // no default
}
Видел советы делать вот так:

switch (...) {
    case ... :
        ....
        break;
    case ... :
        ....
        break;
    default:
        throw new UnsupportedOperationException();
}
Не совсем-то. Я о ситуации когда необработка «левых» значений нормально, но нужно как-то в коде показать, что я о них тупо не забыл.
Очень полезная, кстати, практика, и защитное програмирование тут не причем.
А в 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
}

Так что ли? По-моему бредово.
Или так:
int maxval=int.MinValue;
foreach(int val in array){
  if(val>maxval){
    maxval=val;  // it is new maximum
  }else{
    // it is not maximum, just skip it
  }
}
Просмотрел первый попавшийся файл. Из 43 if-ов только в 6 был else, 10 кончались на return, break или continue, а остальные 27 — просто отработка возникшей ситуации, не требующей разбора случая, когда ситуация не возникла. Причём два else (и один if без else) можно убрать, заменив на switch. Так что else останется в 10% случаев. Если каждый из остальных снабдить else, да ещё и с фигурными скобками — разобраться в коде будет заметно сложнее.
В Google Guava есть метод T checkNotNull(T reference), который делает проверку на не null и возвращает сам этот объект. С его помощью такие вот защитные проверки становятся намного компактнее:

this.userRepository = checkNotNull(userRepository);


Наверняка в C# есть что-то аналогичное
Еще бы стек раскрутить до вызывающего метода
Можно и стек в исключение положить, не проблема.
Подобное можно и самому написать. Вопрос в том, что делать, если всё-таки окажется Null.
Странно, что утверждения упоминаются в открывающей цитате, но не в тексте. В шарпе утверждения, конечно, не самые красивые, но есть. Едва ли имеет смысл использовать if и throw, там где можно обойтись Assert. Собственно, они для сохранения инвариантов и нужны.
Однажды столкнувшись с такой проблемой, мы использовали другой подход. Для необходимых типов данных мы завели пустой объект с необходимой информацией. После этого даже визуально можно было понять что объекта нет, или каких либо его свойств.
Возможно, для системы это были полностью рабочие объекты. Очень помогало когда дизайнеры заводят огромное количество объектов а отображения и данных для них еще нет.
Иногда встречаю в чужом коде (и борюсь с желанием использовать в своем) анти-паттерн, проглатывающий ошибки. Что-то вроде такого:

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 код они тупо игнорируют проблемы. Знаете ли хоть один ещё такой популярный софт, как браузеры, заюивающие на синтаксис? Оси популярнее, но они обычно очень трепетно относятся к неправильному обращению с собой, выбрасывая бдосы, сегфолты и прочие классные штуки.
Отсутствие внешних проявлений != отсутствию каких-либо проявлений. Логгирование все равно очень желательно, хотя бы в дев-версии (а из релизной оно при желании с легкостью вырезается простым #ifdef-ом).
Я полностью с вами согласен, правда в наших языках нет препроцессоров. Вот как бы мне коллег убедить?
Во-первых надо отличать вызов метода и конструктор. Дело в том, что конструктор создает объект, который «хранит состояние» между вызовами. Принципиально надо сводить всё к случаям, чтобы программа не могла в какое-то время находиться в несогласованном состоянии. Т.е. если в конструктор передаются ссылки null, а принципиально не может или не должен быть объект с null-cсылками — программа должна не создать объект, а упасть. Т.е. в конструкторе — да, проверки нужны.

В методе не обязательны. Пусть падает.
Но там говнокод в другом.
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% использует. Если Вы как клиент точно уверены, что Вам нужен объект и он должен быть — вызываете Get. Или даже если не уверены, а подозреваете, что так должно быть. Работа приложения покажет. Надо выбирать более ограниченный вариант.

Потом, механически, в данном случае именно так и подозревалось: после вызова метода идет проверка на 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, так вообще все сообщения об ошибках были более менее читаемыми.
Защитное программирование хорошо в языках, где ошибка может привести к неопределенному поведению, а не падению. В языках, где программно управляете памятью. Вот там, в С++/С, надо опасаться всего.

В статье по сути пришли к этому же, что я написал, как бы убрав почти везде защитное программирование, хотя исходили из него вначале.
Эта тема прекрасно описана в книге «Совершенный код». Да и вообще, почему некоторые программисты не хотят её читать? :)
Only those users with full accounts are able to leave comments. Log in, please.