How to become an author
Search
Write a publication
Pull to refresh

Comments 72

Ещё можно купить уже решарпер и разметить код аннотациями. Тогда тот сам будет подсказывать, откуда можно ждать null, и ругаться при попытке передать его не туда, куда нужно.
Total votes 7: ↑7 and ↓0+7
Можно также воспользоваться CodeContracts. Это средство более мощное.
Но не стоит забывать, что и ваш способ и способ с CodeContracts — это дополнительные инвестиции в проект.
Total votes 6: ↑6 and ↓0+6
Решарпер своих денег стоит и окупается за пару месяцев возросшей за счёт него продуктивности.
Total votes 1: ↑1 and ↓0+1
Я и не говорил про Решарпер. Хотя его цену надо также учитывать. А ещё его эффективность надо доказать. Я его сам использую, купил лицензию. Вы замеряли рост производительности?
Я, в целом, говорил о том, что и в описанном вами способе и мной появляется дополнительный код. Любой дополнительный код требует сопровождения и сам же является источником потенциальных ошибок (например, ослабление контракта кем-то из коллег).
Total votes 1: ↑1 and ↓0+1
У вас 2 сообщения подряд сияет посыл «купить решарпер», завязывайте с этим. Это удел кирби.
Total votes 16: ↑3 and ↓13-10
Не хотите покупать — спиратьте. Мне больно смотреть, как люди делают то, что должна быть занята автоматика.
Total votes 3: ↑2 and ↓1+1
Если не дошло говорю прямым текстом — хорош рекламировать платные продукты. Тут серьёзные статьи за такое банят и в соотв. хаб направляют. Дошло?
А что покупать я как и любой другой хабражитель как нибудь без сопливых решим, и то что вам на что либо больно смотреть никак на подобные решения не повлияет — такие излияния в личном блоге пишите.
Total votes 15: ↑1 and ↓14-13
UFO just landed and posted this here
Есть еще замечательный IntelliJ IDEA от того же самого JetBrains (есть и бесплатная версия, но не уверен, что оно там есть), для Java — тоже аннотации, которые спасают от ряда ситуаций, описанных в статье, хотя, конечно, не панацея. Плюс к этому можно обработать исходники в post-compile, добавив assert-подобные конструкции — всяко лучше, чем отхватить NPE.
This comment wasn’t rated yet0
В Eclipse, кстати, тоже есть такие аннотации; но чтобы была польза, их придется расставлять по всему проекту.
This comment wasn’t rated yet0
Для дотнета есть штука под названием PostSharp, которая встраивается как постсборочный таск и преобразует атрибуты аннотаций в исполняемый код. Помимо стандарных ассертов можно делать свои атрибуты с обработчиками, вызываемыми на входе и на выходе из функции, таким образом привнося в язык аспектно ориентированное программирование. Причём даже вносит правки в отладочные символы. Стоит, правда, как авианосец, так что в боевых проектах пока не пробовал.
This comment wasn’t rated yet0
Вы о стоимости PostSharp или о стоимости написания своей инфраструктуры для поддержки АОП?
This comment wasn’t rated yet0
О стоимости PostSharp. АОП обычно прикручиваю с рантайм-кодогенерацией классов-врапперов. Дёшево, сердито, отладочные символы не портятся, готовые решения есть.
This comment wasn’t rated yet0
Я так понял, что PostSharp — это что-то вроде KindOfMagic, только с большими возможностями?
This comment wasn’t rated yet0
Да, чем-то похоже. Но ради одного INotifyPropertyChanged городить целый post-build таск? У меня для этих целей Reflectiin.Emit-кодогенератор на коленке за пару часов написанный используется сто лет уже.
This comment wasn’t rated yet0
Сегодня освобожу от зависимости от инфраструктуры кодогенерации и выложу.
This comment wasn’t rated yet0
github.com/kekekeks/NotifyHelper — выложил тут. А, да. Оно сделано так, что дата биндинги не отваливаются после обфускации — если генератор видит атрибут PropertyName, то не только будет использовать его как имя для события, но сгенерирует в итоговом классе свойство с таким именем, которое в дальнейшем доступно через Reflection.
Total votes 2: ↑2 and ↓0+2
Представьте, что вы получаете отчёт об ошибках с вашей системы, находящейся в эксплуатации. Трассировка стека указывает на метод GetProductDetails и типом выброшенного исключения является NullReferenceException. Какой из шести возможных объектов с нулевой ссылкой стал причиной ошибки?

Неужели по номеру строки, с которой вылетело исключение, этого никак нельзя понять?
Total votes 6: ↑3 and ↓30
Я бы не стал всерьёз ориентироваться на номер строки. Если номера строк совпадут — хорошо. А если с последней версии номера строк поменялись, то вам, как минимум, придётся лезть в историю через source control средство.
Total votes 4: ↑2 and ↓20
Не вижу проблемы слазить в историю. Мы же всегда достоверно знаем, что именно где сейчас стоит и как получить еще одну точно такую же сборку, не правда ли?
Total votes 7: ↑5 and ↓2+3
Хеш-суммы сборок, собранных из одних и тех же исходников имеет привычку различаться. Не думаю, что изменения затрагивают msil, но тем не менее
This comment wasn’t rated yet0
Если речь идет о Java, можно добавить шаг сборки проекта в дженкинсе, чтобы внутрь артефакта упаковывался текстовый файл с git SHA1, да и номером сборки до кучи.

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

А с пунктуацией у меня всегда проблемы были — последовательность с десяток-другой символов я ещё могу запомнить, но вот на знаки препинания памяти не хватает.
Total votes 3: ↑2 and ↓1+1
А вот отсутствие записей в логе вам точно аукнется. Типичная ситуация вида «ничего не работает, но ничего не известно». Боретесь как-нибудь с этой ситуацией? Ну, может, стиль кодирования какой-то особенно устойчивый?
Total votes 2: ↑2 and ↓0+2
Ну, собственно, я и пытаюсь продвинуть позицию «фиг с ним с пользователем, но хоть для нас самих давайте в логи писать об ошибках, обработку (или отсутствие её) которых мы не предусмотрели».
This comment wasn’t rated yet0
Один приверженец защитного программирования пытался ввести на предприятии стандарт кодирования, где нельзя была написать:
if (условие)
{
  код
}

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

Даже если else программисту не нужно. Во всех if.
Total votes 3: ↑2 and ↓1+1
и помогло? (а вдруг..)
This comment wasn’t rated yet0
Настолько я не заморачиваюсь, но постоянно пишу что-то вроде
switch ($var) {
    case 'val1':
        // no break;
    case 'val2:
        doSmth();
        break;
    // no default
}
This comment wasn’t rated yet0
Видел советы делать вот так:

switch (...) {
    case ... :
        ....
        break;
    case ... :
        ....
        break;
    default:
        throw new UnsupportedOperationException();
}
Total votes 1: ↑1 and ↓0+1
Не совсем-то. Я о ситуации когда необработка «левых» значений нормально, но нужно как-то в коде показать, что я о них тупо не забыл.
This comment wasn’t rated yet0
Очень полезная, кстати, практика, и защитное програмирование тут не причем.
А в else лучше писать короткий коммент, объясняющий почему else пустой.
Исключением могут быть случаи коротких if, заканчивающихся return или throw,
Например, при проверке аргументов.
И да, нам помогает. На мой взгляд, это сопоставимо с требованием всегда использовать фигурные скобки после if и else даже если в блоке только одна строка.
Total votes 5: ↑2 and ↓3-1
Категорически с этим не согласен.
Total votes 5: ↑5 and ↓0+5
А в else лучше писать короткий коммент, объясняющий почему else пустой.

if (!isset($a)) {
  $a = 1; // default value
} else {
  ; // we have value and use it as is
}

Так что ли? По-моему бредово.
Total votes 2: ↑2 and ↓0+2
Или так:
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
  }
}
Total votes 1: ↑1 and ↓0+1
Просмотрел первый попавшийся файл. Из 43 if-ов только в 6 был else, 10 кончались на return, break или continue, а остальные 27 — просто отработка возникшей ситуации, не требующей разбора случая, когда ситуация не возникла. Причём два else (и один if без else) можно убрать, заменив на switch. Так что else останется в 10% случаев. Если каждый из остальных снабдить else, да ещё и с фигурными скобками — разобраться в коде будет заметно сложнее.
Total votes 1: ↑1 and ↓0+1
В Google Guava есть метод T checkNotNull(T reference), который делает проверку на не null и возвращает сам этот объект. С его помощью такие вот защитные проверки становятся намного компактнее:

this.userRepository = checkNotNull(userRepository);


Наверняка в C# есть что-то аналогичное
This comment wasn’t rated yet0
Еще бы стек раскрутить до вызывающего метода
This comment wasn’t rated yet0
Можно и стек в исключение положить, не проблема.
This comment wasn’t rated yet0
Подобное можно и самому написать. Вопрос в том, что делать, если всё-таки окажется Null.
This comment wasn’t rated yet0
Странно, что утверждения упоминаются в открывающей цитате, но не в тексте. В шарпе утверждения, конечно, не самые красивые, но есть. Едва ли имеет смысл использовать if и throw, там где можно обойтись Assert. Собственно, они для сохранения инвариантов и нужны.
This comment wasn’t rated yet0
Однажды столкнувшись с такой проблемой, мы использовали другой подход. Для необходимых типов данных мы завели пустой объект с необходимой информацией. После этого даже визуально можно было понять что объекта нет, или каких либо его свойств.
Total votes 2: ↑2 and ↓0+2
Возможно, для системы это были полностью рабочие объекты. Очень помогало когда дизайнеры заводят огромное количество объектов а отображения и данных для них еще нет.
This comment wasn’t rated yet0
Иногда встречаю в чужом коде (и борюсь с желанием использовать в своем) анти-паттерн, проглатывающий ошибки. Что-то вроде такого:

void Process(Container c, int x)
{
    if(c == null) return;
    if(x <= 0) return;

    c.DoStuff(x);
}

Писать код в таком стиле подсказывает лень — непонятно, что делать с исключительной ситуацией, и разбираться в причинах ее возникновения тоже лень, поэтому давайте всё обмажем проверками, авось пронесет. В итоге баг неуловимым образом присутствует в программе годами.
Total votes 1: ↑1 and ↓0+1
А за такое нужно руки с корнем отрывать. Ни исключения, ни сообщения в логе — вообще ничего! И поди пойми, исполнился метод DoStuff() или нет.
This comment wasn’t rated yet0
Легче станет, если будет
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) {
}
?
This comment wasn’t rated yet0
Нет, разумеется, с чего бы? По-моему это достаточно очевидно, что просто выкинуть исключение (без должной его обработки) — недостаточно.
This comment wasn’t rated yet0
Я считаю, что должно быть достаточно просто выкинуть исключение — необработанное само попадет в логи/stderr в известных мне средах. Но тут не отсутствие обработки, тут явное игнорирование, причем общее, любого исключения.
This comment wasn’t rated yet0
>Но тут не отсутствие обработки, тут явное игнорирование, причем общее, любого исключения.

Ну так о том и речь. Кидаешь исключение — либо обрабатывай по-человечески, либо вообще не обрабатывай. А так ваш пример получается еще хуже оригинального: в оригинале хотя бы сразу видно, что кусок кода потенциально проблемный, а в вашем примере на первый взгляд кажется, что все нормально — мол, видите, исключения правильные кидаем!
This comment wasn’t rated yet0
Сильный аргумент в пользу такого поведения — популярные браузеры. Встречая, не то что не валидный, но даже не велл-формед html или xml код они тупо игнорируют проблемы. Знаете ли хоть один ещё такой популярный софт, как браузеры, заюивающие на синтаксис? Оси популярнее, но они обычно очень трепетно относятся к неправильному обращению с собой, выбрасывая бдосы, сегфолты и прочие классные штуки.
Total votes 2: ↑2 and ↓0+2
Отсутствие внешних проявлений != отсутствию каких-либо проявлений. Логгирование все равно очень желательно, хотя бы в дев-версии (а из релизной оно при желании с легкостью вырезается простым #ifdef-ом).
This comment wasn’t rated yet0
Я полностью с вами согласен, правда в наших языках нет препроцессоров. Вот как бы мне коллег убедить?
This comment wasn’t rated yet0
Во-первых надо отличать вызов метода и конструктор. Дело в том, что конструктор создает объект, который «хранит состояние» между вызовами. Принципиально надо сводить всё к случаям, чтобы программа не могла в какое-то время находиться в несогласованном состоянии. Т.е. если в конструктор передаются ссылки 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 из всех мест, где по смыслу не может быть — это ключ к чистоте и надежности кода.
Total votes 2: ↑2 and ↓0+2
А, извините, невнимателен. В статье в конце о том же. Видимо у меня другая терминология. Не рассматриваю это как защитное программирование, а просто считаю, что null — это враг смысла в коде, который зачем-то всегда подается в коробке с классами и от которого нельзя избавиться. Поэтому по умолчанию у меня его нет. Методы не возвращают, если явно в имени это не указано и в этом явный смысл. Защита — это когда на вход методу подается «что-то не то» и мы защищаемся. Я же думаю — на выходе отдавать надо то, что просят, а если не можешь — кинуть исключение. Проверки только в местах сайд-эффектов — конструкторы или установки полей из методов. В общем, в статье с другой стороны это и описали.
This comment wasn’t rated yet0
Создавать надо два метода ради того, чтобы в ситуации, когда клиент точно знает, что значение точно вернётся из БД он вызывал Get, а если он не уверен, то GetIfExists, если я вас правильно понял?

И, если я вас действительно правильно понял, то как часто встречается ситуация, когда клиент точно уверен, что что-то вернётся? Третье чувство подсказывает мне, что таких случаев 1%.
This comment wasn’t rated yet0
Если Ваш проект — всё приложение, а не библиотека на продажу, то создавать надо только то, что используется. Даже если 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, так вообще все сообщения об ошибках были более менее читаемыми.
Защитное программирование хорошо в языках, где ошибка может привести к неопределенному поведению, а не падению. В языках, где программно управляете памятью. Вот там, в С++/С, надо опасаться всего.

В статье по сути пришли к этому же, что я написал, как бы убрав почти везде защитное программирование, хотя исходили из него вначале.
Total votes 2: ↑2 and ↓0+2
UFO just landed and posted this here
Sign up to leave a comment.

Articles

Show the best of all time
Unlock dark mode

Your account

Sections

Information

Services

Support
© 2006–2024, Habr