На днях мне довелось делать довольно крупные изменения в одном C# проекте — удаление сторонней сборки. Самое "замечательное", что львиную долю времени я потратил на изменение мест, где использовались helper'ы из этой сборки (так сказать бонус к основной функциональности).
Helper'ы такого вида:
Guard.ArgumentNotNull(myobject, "myobject");Такие классы встречаются очень часто, они могут оформляться в common libraries и кочевать из проекта в проект. Но нужны ли они? Распространенное мнение, что подобные guard'ы уменьшают количество строк кода и увеличивают ясность, но, как по мне, это очень субъективное мнение. Вот пример:
class Manager(IEventProvider eventProvider, IDataSource dataSource)
{
Guard.ArgumentNotNull(eventProvider, "eventProvider");
Guard.ArgumentNotNull(dataSource, "dataSource");
this.eventProvider = eventProvider;
this.dataSource = dataSource;
}class Manager(IEventProvider eventProvider, IDataSource dataSource)
{
if (eventProvider == null)
throw new ArgumentNullException("eventProvider");
if (dataSource == null)
throw new ArgumentNullException("dataSource");
this.eventProvider = eventProvider;
this.dataSource = dataSource;
}Да количество строк кода больше, но пострадала ли ясность? Нет.
Еще одно мнение, что при необходимости выполнить проверки для большого количества зависимостей, конструкция if (...) throw превращает код в кашу. Согласен, но это симптомы проблем в дизайне, а не конструкции if (...) throw.
Интересное рассуждение на эту тему есть в блоге Марка Симана.
А что вы думаете?
P.S. Есть еще code contracts, которые в самом простом варианте могут использоваться для подобного рода проверок.
| 10% (27) |
Использую свой Guard |
| 6% (15) |
Использую Guard из сторонней библиотеки |
| 6% (16) |
Использую Code Contracts |
| 60% (155) |
Использую if (...) throw |
| 18% (47) |
Не проверяю входные параметры |
Проголосовало 260 человек. Воздержалось 72 человека.
Только зарегистрированные пользователи могут участвовать в опросе. Войдите, пожалуйста.
Комментарии (92)
EngineerSpock
11.04.2017 10:17крупные изменения в одном C# проекте — удаление сторонней сборки.
Вы считаете разницу между гардом через класс и явной проверкой на null таким уж техническим долгом, который реально надо лечить?
Я не считаю.
Lardite
11.04.2017 14:10+1Конечно сборку удаляли не из-за guard'ов, они были в ней как дополнение к основной функциональности, которая стала не нужна.
imwode
11.04.2017 23:00А что было не написать свой класс Guard с нужными методами?
Я не программист, но если бы у меня стоял выбор между заменой методов класса на явные условия по всему проекту и реализацией своего класса — я бы выбрал второе. Может я чего не понимаю, но ведь делается реально за 15 минут. Написал класс, запустил сборку. Добавил методы, на отсутствие которых ругается, сделал, чтобы они всегда False возвращали, запустил, собрал, допилил методы, пошел дальше.kvasvik
14.04.2017 13:58-1А потом бы Вы уволились, на Ваше место взяли нового программиста и он 2 часа лазил по коду пытаясь понять глубинный смысл методов всегда возвращающих false. Или того хуже, если он раньше работал с этой либой, то мог решить что здесь она уже установлена и использовать её как будто она рабочая, не зная что там заглушка. Так и накапливается технический долг, превращая нормальный код в г*но.
imwode
14.04.2017 16:34Эээээ, а «допилил методы» — пропустил, когда читал? Я и не предполагал, что надо заглушек налепить, а потом бросить. Заглушки — это первый шаг, чтобы скоуп зафиксировать. Установил оперативно, что не хватает трех, например, методов, потом реализовал их, и пошел дальше, не перелопачивая весь проект.
MonkAlex
11.04.2017 10:18+2Гвард позволяет реагировать на некорректные параметры единообразно, и это важно, на мой взгляд.
Если в вашем стайлгайде прописана стандартная реакция на null аргументы, на ourofrange, на все частые кейсы — то этого вполне достаточно.
Ну и, как обычно, любая стандартизация (в случае в гвардом) позволяет легко добавять туда логгирование, игнорирование (ну а вдруг?), какую-нибудь хитрую обработку и т.д. входных параметров.
impwx
11.04.2017 14:04Стандартизация — это хорошо. Я тоже делал подобные методы. Но в долгосрочной перспективе развивать код становится сложнее. Допустим, есть универсальная проверка, но потом начинают возникать особые случаи — сначала требуется один конкретный аргумент логировать, потом еще один, но другим способом, или в другое место, и так далее. В итоге порог вхождения растёт, и в какой-то момент понимаешь, что этими методами стало сложнее пользоваться, чем обычными условиями.
MonkAlex
11.04.2017 14:39+1Стало больно — отрефакторили. Проблема стандартная, как и решение.
В целом скорее соглашусь, редко подходит одно решение во все места, да ещё и без костылей.
Sane
11.04.2017 10:30+6C# 7 позволяет это делать в одну строчку, btw.
this.eventProvider = eventProvider ?? throw new ArgumentNullException(nameof(eventProvider));
chumakov-ilya
11.04.2017 11:52+2… а получение зависимостей через DI и вовсе делает избыточными null-check в конструкторе.
IL_Agent
11.04.2017 12:35+1По-хорошему, код класса не должен зависеть от того, как он будет использоваться.
chumakov-ilya
11.04.2017 13:09+2Согласен, но если DI в проекте прописан на уровне соглашений, подобные проверки в конструкторе переводятся из разряда "обязательно" в разряд "возможно".
Vadem
11.04.2017 12:51+3Я бы не был так категоричен.
Во первых, получение зависимостей через DI не означает обязательное использование IoC-контейнера.
У ручной инициализации есть свои преимущества.
Во вторых, даже при использовании IoC-контейнера, я бы всё равно не стал отказываться от проверок параметров конструктора. По-хорошему класс ничего не должен знать о том как он инициализируется. Вы всегда можете захотеть инициализировать его вручную.
murzilka
11.04.2017 20:56Разве DI сделает что-то большее, чем просто перенесёт проверки в другое место?
ggrnd0
11.04.2017 10:32Использую методы расширения
public static void CheckArgumentNotNull<T>(this T arg, string paramName) where T : class { if (arg == null) { throw new ArgumentNullException(paramName: paramName); } } public static void CheckArgumentNotNull<T>(this T arg, string paramName, string message) where T : class { if (arg == null) { throw new ArgumentNullException(paramName: paramName, message: message); } } public static void CheckArgumentNotNull<T>(this T arg, string paramName, Func<string> message) where T : class { if (arg == null) { throw new ArgumentNullException(paramName: paramName, message: message()); } } public static async Task CheckArgumentNotNull<T>(this T arg, string paramName, Func<Task<string>> message) where T : class { if (arg == null) { throw new ArgumentNullException(paramName: paramName, message: await message()); } } public static void CheckNotNull<T>(this T arg, string message) where T : class { if (arg == null) { throw new NullReferenceException(message: message); } } public static void CheckNotNull<T>(this T arg, Func<string> message) where T : class { if (arg == null) { throw new NullReferenceException(message: message()); } } public static async Task CheckNotNull<T>(this T arg, Func<Task<string>> message) where T : class { if (arg == null) { throw new NullReferenceException(message: await message()); } }
werwolfby
11.04.2017 17:49+4Простите, но я за такое у нас "убиваю"
Потом на любой переменной после "." выскакиевает огромное количество не нужных мне в этот момент методов.
Это раз.
А два — уже 100 раз писали про экстеншены которые проверяют на нулл. Вы глядя на семантику:
myArgument.CheckForNull()
Сразу ожидаете что если
myArgumentравен нулл, то вы получитеNullReferenceException.
Глядя на этот код не понятно что это экстеншен метод. И именно поэтому вы нигде не найдете такого поведения в стандартной библиотеке.
Так что я настоятельно рекомендую так не делать.
AgentFire
13.04.2017 12:53Ну,
CheckForNullможно переименовать вThrowIfNull, это не проблема, да и названия-то у методов одни и те же, следовательно, IntelliSense перегружен точно не будет.
alexeykuzmin0
11.04.2017 11:25Не очень знаком с C#, может быть, там это невозможно, но на мой взгляд, если функция требует, чтобы аргументы у нее были не null, нужно, в идеале, явно запретить туда передавать null на этапе компиляции. Скажем, в c++ это было бы
вместоManager(IEventProvider& eventProvider, IDataSource& dataSource) { this->eventProvider = eventProvider; this->dataSource = dataSource; }
Manager(IEventProvider* eventProvider, IDataSource* dataSource) { Guard.ArgumentNotNull(eventProvider, "eventProvider"); Guard.ArgumentNotNull(dataSource, "dataSource"); this->eventProvider = eventProvider; this->dataSource = dataSource; }
Такой подход повышает readability, переносит проверку ошибок из run-time в compile-time и заставляет программиста, который хочет вызвать этот метод, но имеет на руках лишь указатели, задуматься, может ли там быть nullptr.ggrnd0
11.04.2017 11:37c# так не умеет…
Kerman
11.04.2017 14:13R# умеет [NotNull] в аргументах метода. После этого вызов с возможным null будет подсвечиваться.
ggrnd0
11.04.2017 14:58А какое дело ci-сборщику на подсветки решарпера?
Vadem
11.04.2017 15:20+1Вы можете использовать решарпер на билд-сервере. Это, конечно, не отменяет того факта, что в C# недостаёт контроля за ссылками на null.
impwx
11.04.2017 11:53+1В C# все ссылочные типы могут принимать значение
null. Поддержка не-обнуляемых типов планировалась на C# 7, но пока отложили.
murzilka
11.04.2017 21:25+2Ссылки не панацея.
Никто не запрещает создать объекты IEventProvider и IDataSource на стеке. При этом в конструктор или в любое другое место можно спокойно передавать ссылки на эти объекты.
Что выведет на экран этот код?
struct IEventProvider { int i; }; struct Manager { IEventProvider& eventProvider; Manager(IEventProvider& eventProvider) : eventProvider(eventProvider) { } }; int main() { IEventProvider* eventProvider = nullptr; Manager m(*eventProvider); std::cout << m.eventProvider.i; return 0; }alexeykuzmin0
12.04.2017 11:20Я утверждаю, что ссылки — панацея, но только в том случае, если их правильно готовить.
А именно: в coding style guide добавляем обязательное правило о том, что если функция принимает (или возвращает) указатель, никто и нигде не имеет права неявно предполагать, что он не nullptr. С таким подходом программист старается везде, где это имеет смысл, передать ссылку, а не указатель, а в тех местах, где указатель превращается в ссылку, приходится ставить проверку.
Таким образом, в вашем примере программист, написав "*eventProvider", сразу задумается «а может ли оно быть nullptr?», увидит, что это указатель (то есть, ответ «да»), и добавит проверку. Забыть ее становится невозможно.
С таким подходом за последние несколько лет ни одного NPE не встречал и удивлен, почему же все вокруг считают их такой серьезной проблемой.murzilka
12.04.2017 11:28+1"Панацея, если" это уже не панацея, имхо. Вы вводите новое условие: "в тех местах, где указатель превращается в ссылку...". Так как эту проверку должен будет добавлять человек, место для ошибки всё равно остаётся. То есть наличие ссылок в языке не приводит к тому, что входные данные проверять не надо :)
Самое правильное — проверить успешное выделение памяти. Но тогда и ссылки не нужны.alexeykuzmin0
12.04.2017 11:49Разумеется, панацея, просто панацеей являются не ссылки, а «ссылки + недоверие к любому указателю».
Так как эту проверку должен будет добавлять человек, место для ошибки всё равно остаётся.
Если у программиста связка «разыменование — значит, где-то рядом должна быть проверка» уже на уровне рефлексов, шансы на ошибку очень низкие. Если такой же рефлекс есть у всех, кто будет проводить code review, в подавляющем большинстве проектов вероятностью ошибки можно пренебречь. Если же вдруг по каким-то причинам хочется 100% гарантии — казалось бы, правило для статического анализатора, которое проверяет возможность nullptr, написать очень просто.
Но даже без статического анализатора, за несколько лет работы, в более чем 1 млн строк кода, написанного командой, ни я, ни мои коллеги ни разу не встречали NPE.murzilka
12.04.2017 12:30Какая-то странная панацея, которая вынуждает и программиста, и ревью обращать внимание на потенциальные ошибки, и настраивать инструменты. Не находите? Панацея должны была бы закрывать проблему полностью и без дополнительных усилий.
Если из фразы "панацеей являются не ссылки, а «ссылки + недоверие к любому указателю»" убрать слово ссылки, ничего не поменяется. Панацеей является недоверие к любому указателю. Следовательно, ссылки в данном контексте не нужны :) Да и страуструп вроде бы не с такой целью вводил ссылки в язык.alexeykuzmin0
12.04.2017 12:58вынуждает и программиста, и ревью обращать внимание на потенциальные ошибки
Если мы отказались от проверки указателя — это не потенциальная ошибка, а вполне свершившаяся. Либо в этом месте может оказаться nullptr, и тогда программа упадет, либо там никогда не может оказаться nullptr, и тогда тот факт, что был использован указатель, а не ссылка — логическая ошибка, как минимум, снижающая читаемость кода. В обоих случаях я бы предпочел, чтобы мне напомнили или коллеги, или инструмент.
Панацеей является недоверие к любому указателю. Следовательно, ссылки в данном контексте не нужны
Если без ссылок жить, но с недоверием ко всем указателям — придется в каждый метод писать проверки всех аргументов и возвращаемых значений при вызове всех методов на nullptr, что очень сильно захламляет код. Именно комбинация недоверия к указателям с использованием ссылок везде, где можно (но не где нельзя) очень хорошо подходит для решения описанной в статье проблемы.murzilka
12.04.2017 13:30Если я правильно понял вашу методику, она формулируется так: один раз выделили память в куче, записали её в указатель, проверили, дальше пользуемся ссылками не проверяя их. Верно?
Что поменяется, если переформулировать методику следующим образом: один раз выделили память в куче, проверили, записали в указатель, дальше пользуемся этим указателем без проверки.alexeykuzmin0
12.04.2017 13:40Верно.
Меняется тот факт, что в указатель можно передать nullptr. Даже если предположить, что этого никогда не произойдет, указатель, в который nullptr не передается, по сути своей уже не указатель, а ссылка. То есть, получается, что, используя везде указатели в смысле ссылок мы, таким образом, запутываем читателя.
Кроме того, может оказаться, что в каком-то месте программы все-таки nullptr является допустимым вариантом. Например, для передачи необязательного параметра в функцию или в качестве возвращаемого значения, если функция может зафейлиться. Если везде указатели считаются «уже проверенными», то для передачи такого понятия придется использовать что-то вроде optional, что, на мой взгляд, избыточно, ведь у нас уже есть указатели, которые сами по себе могут быть nullptr.
А еще при использовании ссылок компилятор в очень многих случаях сделает проверку на наличие значения за нас. Например, нельзя будет создать поле в классе и забыть инициализировать его. С указателями таких автоматических проверок, к сожалению, нет.
Да, можно вместо ссылок+указателей использовать указатели+указатели на optional, но, на мой взгляд, первый подход лучше, в первую очередь, в плане readability.murzilka
12.04.2017 16:35> Даже если предположить, что этого никогда не произойдет, указатель, в который nullptr не передается, по сути своей уже не указатель, а ссылка.
Вот тут я не согласен. Указатель, прошедший проверку, не становится ссылкой.
Не поймите неверно, я вовсе не критикую ваш подход. Просто мне кажется что он не панацея. То есть выстрелить себе в ногу всё же можно, только теперь придётся стрелять через ухо :)
А как вы сочетаете это правило использования ссылок с умными указателями и семантикой переноса?alexeykuzmin0
12.04.2017 17:11Указатель, прошедший проверку, не становится ссылкой.
Кажется, наш спор — терминологический. Если нет никакой возможности присвоить указателю nullptr (как в std::reference_wrapper, например), то, на мой взгляд, это повод назвать его ссылкой. Пусть он не будет ссылкой де-факто, но он будет ею с точки зрения поведения.
Собственно, с умными указателями и семантикой переноса такие обертки и спасают.
А насчет панацея или нет — я, честно говоря, не могу придумать, как можно выстрелить себе в ногу, если регулярно прогоняется статический анализатор, который выдает ошибку, если переменная или возвращаемый результат функции разыменован без проверки. Если можете — приведите такой пример, думаю, он откроет мне что-то новое, чего я еще не знал о программировании.murzilka
13.04.2017 14:47С ссылками весь вопрос во владении объектом и времени его жизни. Если ссылки раздаёт тот, кто владеет объектом, и есть гарантия жизни данных на всё время использования ссылки, то всё ок.
Если же вдруг кто-то начинает хранить полученную ссылку, то тут уже может случиться беда. Например, вот что вы писали выше:
> Например, нельзя будет создать поле в классе и забыть инициализировать его.
Если в поле класса у нас хранится ссылка, то при удалении изначального объекта надо что-то делать с этой протухшей ссылкой. Или будет очень плохо. Я не уверен, что какие-то статические анализаторы способны поймать такие проблемы.
novar
11.04.2017 12:11-1А в C# 7 уже можно писАть ещё короче, вместо
if (eventProvider == null) throw new ArgumentNullException("eventProvider"); this.eventProvider = eventProvider;
можно так:
this.eventProvider = eventProvider ?? throw new ArgumentNullException("eventProvider");
shai_hulud
11.04.2017 13:48+3Выпилить все Guard, Contract, ArgumentCheck классы из проекта можно с помощью Roslyn. Я даже давно писал тулзу для удаления Code Contracts из исходников проекта. Даже маленькая статья есть.
Еще в студии есть отличная замена по регулярным выражениям, которая позволяет «сконвертировать» такое проверки в обычный if/throw.
xmetropol
11.04.2017 14:14+2А что мешало вместо изменения всех мест, где используется этот хелпер, просто написать свою реализацию этого класса в проект?
HellMaster_HaiL
11.04.2017 16:21-1Нет, нет и еще раз нет! Никаких Guard'ов, особенно реализованных в каких либо Shared/Common или в сторонних библиотеках, ибо
1. Функция никак не может доверять ни входным параметрам, ни результатам работы вызываемых функций. Проверка входных параметров — критично, а от того, что класс называет Guard не добавляет ему «авторитета».
2. Никто не может гарантировать корректную реализацию таких вот сторожей, их правильною доработку/развитие. Плюс, как писали выше, с ростом исключительных ситуаций, такие классы начинают разрастаться вширь перегрузками, в которых не разобраться со временем.
3. Потенциальное место для лишних зависимостей, на выпиливание которых может потребоваться время и все равно привести к ошибкам. Особенно проблемы с зависимостями могут проявиться, если такие классы реализованы в других библиотеках.MonkAlex
11.04.2017 17:01+1Вы слишком категоричны. Куча фреймворков мелких так работает, что теперь, не пользоваться ими?
HellMaster_HaiL
11.04.2017 17:32Если Ваш мелкий фрэймворк заставляет Вас делать проверку параметров функций именно с помощью статических гуардов или без оных использование такого фрэймворка нецелесообразно или не работоспособно, то да, я бы не стал использовать подобный фрэймворк.
chumakov-ilya
12.04.2017 10:25+1Никаких Guard'ов, особенно реализованных в каких либо Shared/Common или в сторонних библиотеках
Может вообще сторонним библиотекам не доверять? Убрать из кода Json.NET, Moq, Dapper… они делают не менее критичную работу, чем проверка на null.
Никто не может гарантировать корректную реализацию таких вот сторожей, их правильною доработку/развитие.
А кто гарантирует корректную реализацию остального кода, который вы пишете?
Плюс, как писали выше, с ростом исключительных ситуаций, такие классы начинают разрастаться вширь перегрузками, в которых не разобраться со временем.
Сложно представить проект, в котором набор простейших проверок на null, длины строки и т.п. может быть ощутимо сложнее остальной части кода вплоть до "не разобраться". А если вы поместили в гвард бизнес-логику, то это уже не Guard, и на вас налагаются такие же обязанности по обслуживанию этой логики, как и по отношению к остальной части бизнесового кода.
Особенно проблемы с зависимостями могут проявиться, если такие классы реализованы в других библиотеках.
В другой ветке справедливо заметили, что решение проблемы топикстартера по выпиливанию/рефакторингу гвардов решается решарпером за минуты.
HellMaster_HaiL
12.04.2017 13:25Может вообще сторонним библиотекам не доверять? Убрать из кода Json.NET, Moq, Dapper… они делают не менее критичную работу, чем проверка на null.
Пожалуйста не смешивайте вареники с пельменями. Я не писал, что стоит отказаться от библиотек своих или сторонних. Я говорю, что стоит отказаться от гвардов, а не от библиотек. Убирать либы из проектов — я не про это. А вот про доверие к сторонним библиотекам — это вполне себе открытый вопрос. Я бы не стал слепо и безоговорочно им доверять. В том же Json.NET может быть ошибка, необработанная исключительная ситуация или на вход прилететь уж совсем невалидный json и не известно, как десериализатор себя поведет. И я бы никогда бы не отправил в продакшен код типа такого:
Movie m = JsonConvert.DeserializeObject<Movie>(json); string name = m.Name;
А кто гарантирует корректную реализацию остального кода, который вы пишете?
А никто. В идеале функция настолько параноидальна. что доверяет лишь тому коду, который она включает в себя. А все входные параметры и результаты работы других функций (даже написанных Вами) должны подвергаться сомнению и проверке. Но это, конечно, в идеале.
Сложно представить проект, в котором набор простейших проверок на null, длины строки и т.п. может быть ощутимо сложнее остальной части кода вплоть до «не разобраться». А если вы поместили в гвард бизнес-логику, то это уже не Guard, и на вас налагаются такие же обязанности по обслуживанию этой логики, как и по отношению к остальной части бизнесового кода.
Как не все йогурты одинаково полезны, так и не все функции одинаково проверяют входные параметры. Одной функции достаточно простой проверки строки на null, второй просто заюзать string.IsNullOrEmpty(), а третьей непременно надо string.IsNullOrWhiteSpace(). А потом окажется, что для четвертой входная строка никак не может быть длиной меньше 10, а для пятой не меньше 10 и не больше 15, для третьей вообще не должна быть пустой либо содержать спецсимволы. Вот и появляются в таких вот гвардах часто недокументированные методы типа:
Guard.EnumerableNotNull…
Guard.EnumerableNotNullOrEmpty…
Guard.EnumerableNotNullOtEmptyOrHasNullableItems…
Guard.EnumerableIsEmpty…
и так далее до бесконечности. А если это еще всё реализовано как советуют тут. то это вообще «ховайся в кукурузу», уж простите мне мой украинский.
В другой ветке справедливо заметили, что решение проблемы топикстартера по выпиливанию/рефакторингу гвардов решается решарпером за минуты.
На это можно ответить только анекдотом:
Русские славятся своим умением выходить из любой безвыходной ситуации. Но больше они славятся умением в такие ситуации входить.
AgentFire
12.04.2017 12:24-2Не очень правильные варианты голосования. Есть две абсолютно разные стези программирования — это когда пишешь код сам, и когда пишешь его с кем-то. В первом случае даже никакая проверка не нужна — если ты хорошо себя знаешь, то в 100% случаях не пошлешь null там, где его не должно быть. Во втором случае ситуация меняется, т. к. мозги другого человека — другие, и здесь можно использовать
Guard, но только как "сокращатор" с двух строк на одну.Lardite
12.04.2017 12:27+2извините, но контракт класса не должен полагаться на адекватность использующего его клиента.
Nonsense66
12.04.2017 12:27Использую самописный Guard в своих проектах. Guard делает код более декларативным, чистым и читаемым. Guard — однозначно один из моих самых любимых хелперов. Тем более с новыми фичами C#:
Guard.NotNull(arg, nameof(arg)); Guard.NotNullOrWhiteSpace(stringArg, nameof(stringArg)); Guard.NotEmptyList(list, nameof(list));devsane
12.04.2017 22:02Согласен, nameof сильно облегчил жизнь. Для еще большего упрощения можно было бы воспользоваться Expression и избавиться от аргумента с именем… хотя, наверное, на любителя.
public void Test(object o) { Guard.ArgumentNotNull(() => o); } public static class Guard { public static void ArgumentNotNull<T>(Expression<Func<T>> argExpr) where T: class { if (argExpr == null) { throw new ArgumentNullException(nameof(argExpr)); } var arg = argExpr.Compile()(); if (arg == null) { var argName = (argExpr.Body as MemberExpression)?.Member.Name; throw new ArgumentNullException(argName); } } }
xyzuvw
12.04.2017 12:27Код проверки на null достаточно тривиален. А что бы вы предпочли видеть?
if (double.IsNaN(weight) || double.IsInfinity(weight)) { throw new ArgumentOutOfRangeException( nameof(weight), weight, "Значение параметра не является числом."); } if (weight < 0 || weight > 1000) { throw new ArgumentOutOfRangeException( nameof(weight), weight, $"Значение параметра выходит за границы диапазона 0..1000."); }
или
Guard.ArgumentInRange(nameof(weight), weigth, 0, 1000);HellMaster_HaiL
12.04.2017 14:05+1Я бы предпочел видеть первый вариант, потому что много «если»:
— если нужно проверить только на nan или только на бесконечность? Т.е. Вам, как мне кажется, нужно минимум три метода в гварде.
— если нужно под разное условие бросить свои эксепшены? Отдельно под nan, inf и range?
— если нужно другие условия? Проверка на положительную или отрицательную бесконечность, на вхождение в несколько массивов или на невхождение в данный, на положительность (больше нуля) и так далее? Писать дополнительные методы в гварде или комбинировать ифы и вызовы гварда?
Как бы Вы реализовали следующее:
if (double.IsNaN(weight) || double.IsNegativeInfinity(weight)) { throw new ArgumentNegativeException( nameof(weight), weight, "Значение параметра не является числом или равно минус бесконечности."); } if (weight > 100 || weight < 1000) { throw new ArgumentOutOfRangeException( nameof(weight), weight, $"Значение параметра выходит за границы диапазона 0..1000."); }
с помощью Вашего единственного
Guard.ArgumentInRange(nameof(weight), weigth, 0, 1000);
?xyzuvw
12.04.2017 20:19Насколько я понял, Guard — это набор статических методов для различных ситуаций, которые вам нужно обрабатывать. Хотя название неудачное. Возможно, лучше было бы ArgumentValidation. Для вашего случая код мог бы выглядеть как-то так:
ArgumentValidation.IsFinite(nameof(weight), weigth);
ArgumentValidation.InRange(nameof(weight), weigth, 0, 1000);
Lardite
12.04.2017 15:01это зависит от того как часто такой код дублируется? меняется ли текст сообщения? возможно в каком-то случае добавляются дополнительные условия.
vyatsek
12.04.2017 19:40Проблема высосана из пальца. Вот просто на ровном месте и в случае чего лечится монотонной работой по замене.
При выкидывании либы, я бы сделал методы с аналогичной сигнатурой, положил бы в какую-нить утилитную сборку и пусть лежит.
Автор говорит что у него в VS есть снипетты и он ими пользуется. Если он пишет один ок, нет вопросов. Но если пишет команда с различным составом на протяжении продолжительного времени, то Guard явно покажет каким образом используются контракты в коде.
Вообще если автор заморачивается над такой мелочевкой, скорее говорит о том, что он не решал более серьезных проблем в коде.
Bonart
19.04.2017 11:46Опрос посвящен достижению негодной цели негодными средствами.
Вместо декларативного кода предлагается раздутый в два раза императивный.
И читаемость ухудшилась — полезный код теперь заметно дальше от заголовка.
Замена всех вызово на throw элементарно делается одним шаблоном в решарпере.
И, наконец, NullGuard.Fody сделает всю работу вообще без единой дополнительной строчки кода.
aamonster
Лично я думаю, что писать, как у вас во втором примере, не следует. Или заключайте тело if в операторные скобки, или пишите всё на одной строке — это защищает от типовой ошибки, когда к телу if что-то добавили, но в скобки не взяли.
Ну и нетрудно заметить, что со скобками получается 3-4 строки на проверку — довод в пользу хелперов. Хотя агитировать за них и не буду, думаю, тут нет серебряной пули.
Кстати, расскажите всё же подробней, почему ушло много времени на борьбу с хелперами? Не вижу ни одной причины для этого.
impwx
Я часто слышу этот довод от сторонников подхода «скобки везде», но за всю жизнь ни разу не видел, чтобы кто-то эту ошибку действительно допустил, поэтому данный совет считаю бессмысленной перестраховкой. На каждое условие тратится по 2 лишние строки, код становится рыхлым и на экране помещается гораздо меньше полезной информации — это факт. А то, что это конкретное условие потребуется расширять, программист окажется безграмотным или невнимательным, не будет тестов и код-ревью — это фантастическое стечение обстоятельств, либо полная разруха в проекте.
iXCray
Видел, и не раз :)
А в соседнем бложике от PVS-Studio еще и примеры из реальных проектов есть.
Код-ревью делают такие же люди — это самый веский довод в пользу того, чтобы делать fail-proof design, а не надеяться на то, что ревьювер будет в здравом уме в каждый конкретный момент проверки.
По части "рыхлости" — первая скобка на той же строке, а закрывающая — на новой. Перед ревью код должен быть автоматически отформатирован, чтобы отступы полностью соответствовали вложенности.
А если ревью нет, то все описанное выше все равно сработает, чего не скажешь о случае, когда нет общего согласия на использование скобок в коде.
impwx
Failproof design часто вырождается в крайнее неудобство использования — вспомните историю про хакера в столовой. На моей практике, начинать все равно нужно с подбора компетентных сотрудников на организационном уровне, а дальше уже всякие тесты-стайлгайд-статический анализ на техническом.
В оперсорсных проектах с многолетней историей или крупных фирмах с постоянным набором практикантов ситуация наверняка другая. Если с кодом потенциально может работать неадекватный персонал, то проверки нужны более строгие. Но про работу в таких условиях я ничего сказать не могу за отсутствием опыта.
Neikist
Каким бы компетентным сотрудник ни был, всегда есть вероятность что его отвлекут как раз в неподходящий момент, или навесят десятки задач так что будет идти 16-й час работы без перерывов. Никто в таких случаях не застрахован от ошибок «по невнимательности», ибо поддерживать непрерывно нужный уровень концентрации невозможно.
impwx
Никакое сугубо техническое решение описанную вами проблему не решит. Если принять, что программист не отдает себе отчета в своих действиях, он всегда найдет способ накосячить: можно написать бажный код и покрыть его бажными тестами, можно выложить не тот релиз, можно вообще неправильно понять задание!
Поэтому я за то, чтобы в первую очередь подходить к вопросу организационно. Если отвлекают и перегружают на постоянной основе, или требуют 100% фокус-фактора — это повод пересмотреть рабочий процесс, или вообще сменить место работы. Для конкретно взятого программиста это может повысить не только качество кода, но качество жизни в целом.
Neikist
Есть компании (подозреваю их очень много) где программиста постоянно дергает куча разного люда (опенспейс с шумными коллегами, постоянно подбегающие с вопросами менеджеры и саппорт и т.п.), где загрузка задачами на овер 100% (чтобы даже вероятности не появилось что человек простаивает) считается нормой (что в таком случае происходит перед дедлайнами? Правильно, завал с переработками). Плюс далеко не всем нужны звезды или даже средний уровень, много где нужно простое формошлепство и т.п. вещи, либо руководство жмотится платить больше 30-40к (говорю про провинцию) и хорошие спецы просто не пойдут туда, а работу кому то нужно делать. И я считаю что в таком случае нужно по максимуму перестраховываться с самого начала, минимизируя будущие проблемы регламентами написания и оформления кода (те самые скобочки), хелперами, тестами, языком с сильной типизацией и т.п. вещами. К слову — это действительно только мое мнение, поскольку к сожалению не имел возможностей работы со всем перечисленным мной (ну только регламенты по стилю оформления кода есть, и то их соблюдают не все и не всегда), и проблем мы из за описанных вещей имели немало, причем чем крупнее проект — тем больше вероятность появления проблем именно из за данных причин.
imwode
А я несколько часов потерял один раз на дебаг
Lardite
время ушло на замену хелперов на if(...) throw. Можно было написать свой Guard с аналогичной сигнатурой, а можно было и скрипт написать для умной замены…
werwolfby
Пишите свой Guard, а потом решарпером вставляете тело Guard везде. Profit.
Быстро, просто, легко и эффективно.
murzilka
А если решарпера нет? Он сильно платный.
werwolfby
Ну тогда я вам очень сочувствую и я бы поискал другую компанию. Которая заботится о продуктивности своих разработчиков и как результат качеству кода :)
Серьезно, очень сложно, даже сейчас, жить без решарпера.
murzilka
Серьёзно? Инструмент является ключевым показателем производительности? Что же делать всем тем программистам, которые код пишут в vim, самоустраниться?
У нас в компании используются другие языки и технологии. Шарп я выбрал для одного побочного продукта, и даже не приходило в голову ради такой мелочи запрашивать у руководства платную IDE.
Но если вы пишете, что на шарпе без неё жизни нет, значит, я совершил ошибку при выборе.
werwolfby
Если вы пишите маленький проект, так хоть в блокноте пишите. И если вы не понимаете пользу от удобных сред разработки это тоже ваше дело. Я снова готов лишь посочувствовать.
Для больших энтерпрайз проектов решарпер мастхев.
Ну и реально есть проблема в том что люди даже если решарпер поставляется компанией не умеют им пользоваться. И вот это уже бесит — не желание людей учиться.
Если вы не хотите пользоваться и считаете vim лучшим для C# ваше право, в конце концов vim тоже мощный инструмент.
И опять же я говорю про большие проеты, для некоторых вещей мне тоже саблайма хватает. Но, например на питоне не в PyCharm писать очень не уютно и гораздо менее продуктивно для меня.
murzilka
Удобство средств разработки определяется исключительно субъективно. То, что удобно для вас, может быть крайне неудобно для вашего соседа. Как только вы это осознаете, начнёте по другому относиться к людям, которые не хотят осваивать решарп.
Касательно того же питона. Писать на нём в PyCharm может быть мучением. Потому что даже на мощном компе на работе он может в неожиданном месте в неожиданное время призадуматься. Лично для меня это совершенно неприемлемо. Потому что обычно я либо быстро набираю код, либо внимательно его изучаю, переключаясь между разными методами и файлами. В обоих сценариях даже разовое притормаживание выбешивает и сказывается на производительности.
А вот для программирования на джаве в стиле постоянного рефакторинга мышевозюканием идея самое удобное средство.
werwolfby
Это ваше право, я вот скорее всего откажусь от работы без решарпера.
Мне это напоминает автосервисы, есть гаражные мастера которым ямы хватает для работы, а масляный фильтр можно обычным ремнём снять, зачем отдельный ключ для этого.
А есть автосервисы которые пользуются подъемниками а не ямами. Мы не бедем говорить о квалификации и тех и других работников. И там и там есть и отличные, и плохие специалисты.
Но работу с подьёмником всё таки делают лучше, тщательнее и быстрее.
Я не знаю, что у вас за компы, но и студия у меня раньше притормаживала, а с решарпером ещё больше. Сейчас я не исптываю таких проблем, ну практически :)
Как минимум эти тормаза для меня уже приемлемы. А раньше было просто ужасно и я понимаю вашу боль, я всегда надеялся что студия хотя бы лекарство от рака в это время ищет.
По этому просите своего работадателя о нормально оборудовании.
murzilka
Я даже не знаю, что на это ответить :)
werwolfby
Ну я тоже не знаю, что на это ответить, потому что у меня нет таких проблем, что я делаю не так?
Поэтому единственная причина для меня это разница в железе, сомневаюсь что у нас разный PyCharm.
murzilka
То есть разное окружение и разную кодовую базу, с которой приходится работать, вы исключаете. Забавно.
werwolfby
А вы думаете что у меня проектики из двух файлов, забавно :)
murzilka
Ещё раз повторю свою мысль. Она простая, но вы почему то никак не реагируете на неё.
Почему вы считаете, что удобный инструмент это исключительно решарпер? Есть же другие плагины для студии. Code rush, к примеру, и еще десяток других. По каким причинам вы считаете, что именно ваше понятие об удобстве универсально и применимо совершенно когда всем поограммистам?
Ну и ещё несколько провокационный вопрос: а какие качества программиста важнее, умение писать код, или умение пользоваться инструментом для его написания?
werwolfby
Так а я и не говорил, что решарпер это единственный верный и правильный инструмент, но мне с ним удобнее чем с Code Rush например. А может я и плохой программист раз не знаю других, вот серъёзно, я не назову других таких же крутых инструментов для студии.
Я лишь говорил, что мне без решарпера жить плохо, и если его заменят хотя бы на Code Rush, то это будет терпимым решением.
Ну, я думаю, ответ очевиден — важно умение писать код. Но я ни разу не видел плохого программиста, который умеет пользоваться инструментом. Он ни код не пишет, ни решарпер ему не нужен. Если я плохой автослесарь, то мне подьёмник не поможет от слова совсем.
А вот хороших кодеров, которые не умеют пользоваться решарпером просто полно! А хорошие программисты умеют им пользоваться.
werwolfby
Простите, но я серьёзно не уловил вашу мысль про другие инструменты. Это раз.
Во-вторых, я согласен, что можно быть хорошим программистом и писать хоть в блокноте. Но, я считаю, глупо не пользоваться инструментами, которые облегчают вам жизнь. И я не встречал таких программистов.
murzilka
Так инструментом может быть vim.
Мой бывший коллега классный программист. У него синдром пястного канала, как-то так оно называется. И его единственный инструмент это vim. Потому что требование к удобному инструменту может быть только одно: минимум нажатий на кнопки клавиатуры и вообще никакой мыши. Он в студии, наверное, не смог бы долго работать. А уж решарпер оставил бы его инвалидом, думаю.
Я это к тому написал, что даже оценка удобства и применимости инструментов может быть очень разной. А вы выше вполне однозначно писали, что если работодатель не предоставляет решарпер, работодателя надо менять.
werwolfby
Ну во-первых vim прекрасный инструмент.
Во-вторых есть множество похожих историй из других областей, про слесаря у которого была сломана рука и только он мог долезть на производстве до нужных мест. Про электрика с косятной ногой который не становился на резиновый коврик… много-много и других историй.
В каждом правиле есть исключения, даже в этом.
Я считаю решарпер отличным инструментом и мне без него будет очень не удобно работать. Надеюсь эту мысль я донёс. И у нас у всех .NET разработчиков есть решарпер. Если мы проведём опрос о том нужен или нет решарпер, или его можно заменить на что-то другое. То я уверен на 90%, что большинству нужен решарпер для работы. Есть те кто открывают студию раз в неделю, вот они могут и без него.
И я серьязно не могу понять, почему вы оспариваете, то, что без решарпера сейчас очень тяжело жить. И Code Rush как и чистая студия ему совсем не замена.
Если вы мне предоставите реальные альтернативы, то я буду вам очень признателен.
murzilka
Независимо от времени — сейчас, завтра, или позавчера — я буду оспаривать идею о том, что очень тяжело жить без вспомогательного инструмента. Есть много других важных факторов, влияющих на выбор места работы. Уровень оплаты, интересный проект, перспективы, загруженность, коллектив, бонусы типа страховки/спортзала/обучения, условия труда, удобство рабочего места и т.д. Требование наличия конкретного вспомогательного инструмента для меня лично идёт где-то в конце этого списка. Если вообще идёт, потому что я скорее вспомню пословицу про плохого танцора и помехи ему, чем про необходимость требовать наличие определенной ide.
Раз уж вы постоянно прибегаете к аналогиям, то вот вам ближайшая: слесарь Вася работает только с отверткой у которой пластиковая ручка. И наотрезвается работать с другими отвертками, вплоть до того, что меняет автосервис если ему предложить отвертку с деревянной ручкой :)
werwolfby
Вот вы серьёзно сравниваете разный материал ручек в отвертках с целыми разными инструментами, такими как: яма и подьёмник хотя бы?
Хотя даже это сравнение слишком маленькое. На самом деле разница между горажом с одной переноской и самым дешевым набором головок и целой мастерской с кучей инструмента включая динамометрические ключи магниты зеркала держатели, куча тестеров и многое другое. А вы своей аналогией сравниваете notepad с темной темой и notepad со светлой темой.
И вы никак не поймёте, что я не верю, что есть работадатель который предоставит железо на котором PyCharm не тормозит, 2 нормальных IPS монитора, удобное кресло, много свободно места, а не толканием локтями с соседом, и зажмётся на лицензию решарпера. Серьёзно?
Скорее всего он зажмётся на всё, но проще всего это узнать лицензией на решарпер :), шучу конечно. Аналогично наличие лицензии не говорит о хорошом рабочем месте. И если вы читали мои высказывания выше, то заметели, что я говорил, про то что вам стоит менять работадателя даже если у вас на компе тормозит PyCharm, а не только потому что у него только решарпера нет.
И я не собираюсь работать на табуретке без спинки, лишь бы решарпер был.
Естественно всё есть в савокупности, но вы как будто утверждаете, что есть работадатели которые сделают вашу работу комфортной, но забудут выдать вам основной инструмент который увеличит вашу продуктивность. "Вот вам отвёртка и больше ничего не получите", но работайте в просторной мастерской с бесплатным кофе и чаем. Смешно же. И если вдруг такие есть, то я все ещё советую менять место работы. Хотя если вам там платят в 2 раза больше, ну я бы наверное тоже пожил без решарпера. Или как вы говорите там очень интересные проекты, то скорее всего я бы тоже пожил без решарпера, но сказок не бывает. Не может быть всё хорошо, но не быть решарпера… Ну не верю я....
И опять же, я же не говорю, что всё что нужно C# девелоперу это только решарпер, где вы это прочитали? Я говорю, что работать без решарпера, даже на стуле за 1000$, я не стану.
murzilka
Да, я сравниваю инструменты с инструментами. Решарпер с отверткой. Code rush с другой отверткой.
А в ваших сравнениях упорно идёт одно и то же: решарпер это подъемник, все остальные — жалкие домкраты. При этом вы вроде бы не спорите с тем, что профессионалу всё равно, чем откручивать, т.е. писать код. Но вам лично не всё равно.
Про работодателя вы рисуете какие-то упрощенные картины. Конечно, жмотиться на лицензии на десяток рабочих мест работодатель не будет. Но в моём случае, как я писал, мне самому в голову не пришло просить решарпер. Просто потому что он мне не нужен :)
Я знаком с продуктами jet brains. И как бы вы ни пытались свести все проблемы к железу, тормоза в них случаются на любом железе. Вы не спрашивали ни про ос, ни про количество файлов в проектах, ни про средний loc в этих файлах, а сразу бросились утверждать, что тормоза из-за железа. Простите, но после этого я уверен, что в ваших проектах действительно пара файлов. Ну или пара десятков.
Хотя, по памяти, тупануть идея могла и на таком количестве файлов. Но может мне не везло, или вам везло, или вы привыкли и перестали замечать.
На этом предлагаю разойтись, ибо спор об удобстве инструментов всё таки бессмысленен :) Вы не убедите меня в том, что без решарпера жизни нет, я не смогу убедить вас в том, что без решарпера она есть :)
werwolfby
Я так и не смог понять вы .NET разработчик или нет?
Если нет, то я не могу понять как вы можете рассуждать о жизни без решарпера. Потому что я знаю, что это за ад.
Но вместо разговоров/споров нужно проводить опрос.
То что вы считаете, что можно прожить без решарпера — это ваше мнение. И оно не мнение большинства.
Когда у нас в компании задумались о отмене решарпера, ибо дорого, то вопрос решился в течении 2-3 дней, больше такой вопрос не поднимается. И это явный признак того что без решарпера жизнь ад. Ибо взбунтовались почти все, остальные жаловались устно на перекупах. И вот поэтому я сужу как мнение большинства, аналогичную информацию знаю из других организаций. И они разделяют моё мнение по поводу решарпера.
Я так и не понял вашу идею, но согласен, что дальше "спорить" безсмысленно.
werwolfby
Хотя вопрос наверное не корректен, так как вы использовали idea.
Так что не важно, тему стоит закрывать. Простите если обидел.
murzilka
Если отмотать разговор в самое начало, то я задал вопрос, как решить проблему, если инструмента нет. В ответ вы начали рассказывать о том, что без инструмента не жизнь. И я сделал вывод, что решить проблему без решарпера невозможно. Ну или вы не знаете, как. Вот почти вся моя идея.
Далее мы попробовали обсудить py charm, но как-то быстро снова пришли к решарперу.
Я не знаю, правда ли без решарпера ад. Субъективно ада нет :) Но в принципе я, как и другие программисты, решившие писать на c# без решарпера в студии под виндой, или под макосью, или под линуксом, вполне можем испытывать адские муки и просто не замечать их. Безусловно, вы можете просветить меня, если устроите голосование. По его итогам я пойму, что я в аду. Заранее спасибо :)
werwolfby
Я даже подумать не мог что можно мои слова так понять :) Спасибо за разъяснение.
Я же всего лишь хотел показать, что при хорошем инструменте проблема решается в считанные минуты на практически любого объёма проекте.
И дальше я утверждал, что если вы привыкли работать без решарпера — это ваше право. Но я считал глупым не пользоваться хорошими инструментами которые облегчают вашу жизнь. И если работадатель этого не понимает то это очень плохо, и я бы поменял работадателя.
Аналогично я не прошу WebStorm, мне хватает Visual Studio Code для работы с JavaScript. Потому что он стоит денег, а так как я залажу в JS код в наших проектах в лучшем случает раз в месяц, то мне реально не нужен WebStorm на постоянной основе. Но я уверен, что с ним я бы даже те мелкие таски делал бы быстрее. Но смысла в этом мало.
Так что мой посыл был прост. Если ты .NET разработчик то решарпер — это маст хэв. Если работадатель не может тебе этого дать, то нужно от него уходить.
В ваших же комментариях я вижу только:
И я этого не могу понять, почему же все не пишут просто в блокноте? Т.е. вы как бы утверждаете, что качество кода совсем не завист где я его пишу, может и подсветка синтаксиса вам не нужна?
werwolfby
Опять же, вот человек целую статью написал про выпиливание Guard.
Я бы с решарпером эту "проблему" решил бы за 15 минут и забыл бы.
Lardite
за «статью» конечно спасибо, но это все таки опрос.
И да согласен по поводу решарпера.
werwolfby
Да да, ни сколько не хотел обидеть. За статью и опрос спасибо.
Bonart
Годовая подписка на решарпер без всяких скидок полностью окупается уже за три месяца при следующих (заведомо плохих) условиях
Не верите — возьмите калькулятор и пересчитайте.
Это не решапер сильно платный, а менеджмент сильно скупой, за что и платит многократно.
devsane
Думаю, было бы удобно использовать стандартный диалог замены текста с включенной галкой «Use Regular Expressions».
Пример поискового запроса:
Пример замены (с сохранением смещения):