Вариант 1
Я помню как я читал на одном из форумов как кто-то высказывался, что команда, разрабатывающая C#, должна сделать поведение при "?." (автоматическую проверку на null) поведением по умолчанию при навигации внутрь объектов вместо того, что сейчас происходит при ".". Давайте порассуждаем немного об этом. Действительно, почему бы и не изменить имплементацию для "."? Другими словами, почему не использовать "?." оператор везде, просто на всякий случай?
Одно из основных назначений вашего кода — раскрывать ваши намерения другим разработчикам. Повсеместное использование оператора "?." вместо "." приводит к ситуации, в которой разработчики не способны понять, действительно ли код ожидает null или кто-то поместил эту проверку просто потому что это было легко сделать.
public void DoSomething(Order order)
{
string customerName = order?.Customer?.Name;
}
Действительно ли этот метод ожидает, что параметр order может быть null? Может ли свойство Customer также возвращать null? Этот код сообщает, что да: и параметр order, и свойство Customer могут обращаться в null. Но это перестает быть таковым если автор поместил эти проверки просто так, без понимания того, что он делает.
Использование оператора "?." в коде без действительной необходимости в нем приводит к путанице и ухудшает читаемость кода.
Эта проблема тесно связана с отсутствием ненулевых ссылочных типов в C#. Было бы гораздо проще читать код если они были бы добавлены на уровне языка. К тому же, подобный код приводил бы к ошибке компиляции:
public void DoSomething(Order! order)
{
Customer customer = order?.Customer; // Compiler error: order can’t be null
}
Вариант 2
Еще один способ злоупотребления оператором "?." — полагаться на null там, где это не требуется. Рассмотрим пример:
List<string> list = null;
int count;
if (list != null)
{
count = list.Count;
}
else
{
count = 0;
}
Этот код имеет очевидный «запах». Вместо использования null, в данном случае лучше использовать паттерн Null object:
// Пустой список - пример использования паттерна Null object
List<string> list = new List<string>();
int count = list.Count;
Теперь же с новым "?." оператором, «запах» уже не так очевиден:
List<string> list = null;
int count = list?.Count ?? 0;
В большинстве случаев, паттерн Null object — намного более предпочтительный выбор чем null. Он не только позволяет устранить проверки на null, но также помогает лучше выразить доменную модель в коде. Использование оператора "?." может помочь с устранением проверок на null, но никогда не сможет заменить необходимость построения качественной модели предметной области в коде.
С новым оператором очень легко писать подобный код:
public void Process()
{
int result = DoProcess(new Order(), null);
}
private int DoProcess(Order order, Processor processor)
{
return processor?.Process(order) ?? 0;
}
В то время как было бы правильней выразить эту логику с использованием Null object:
public void Process()
{
var processor = new EmptyProcessor();
int result = DoProcess(new Order(), processor);
}
private int DoProcess(Order order, Processor processor)
{
return processor.Process(order);
}
Вариант 3
Часто подобный код показывается как хороший пример применения нового оператора:
public void DoSomething(Customer customer)
{
string address = customer?.Employees
?.SingleOrDefault(x => x.IsAdmin)?.Address?.ToString();
SendPackage(address);
}
В то время как подобный подход действительно позволяет сократить число строк в методе, он подразумевает, что сам по себе такой код — это нечто приемлемое.
Код выше нарушает принципы инкапсуляции. С точки зрения доменной модели, намного более правильным было бы добавить отдельный метод:
public void DoSomething(Customer customer)
{
Contract.Requires(customer != null);
string address = customer.GetAdminAddress();
SendPackage(address);
}
Сохраняя таким образом инкапсуляцию и устраняя необходимость в проверках на null. Использование оператора "?." может замаскировать проблемы с инкапсуляцией. Лучше не поддаваться искушению использовать оператор "?." в подобных случаях, даже если «сцеплять» вызовы методов друг за другом может казаться очень простой задачей.
Валидные примеры использования
В каких же случаях использование оператора "?." допустимо? В первую очередь, это легаси код. Если вы работаете с кодом или библиотекой, к которой не имеете доступа (или просто не хотите трогать исходники), у вас может не остаться другого выхода кроме как подстраиваться под этот код. В этом случае, оператор "?." может помочь уменьшить количество кода, необходимого для работы с подобной code base.
Другой хороший пример — вызов ивента:
protected void OnNameChanged(string name)
{
NameChanged?.Invoke(name);
}
Остальные примеры сводятся к тем, которые не подпадают под три варианта невалидного использования, описанных выше.
Заключение
В то время как оператор "?." может помочь уменьшить количество кода в некоторых случаях, он также может замаскировать признаки плохого дизайна (design smells), которые могли бы быть более очевидными без него.
Для того чтобы решить, нужно или нет использовать этот оператор в каком-то конкретном случае, просто подумайте будет ли код, написанный «по старинке» допустимым. Если да, то смело используйте этот оператор. Иначе, постарайтесь отрефакторить код и убрать недостатки в дизайне.
Английская версия статьи: 3 misuses of "?." operator in C# 6
Комментарии (80)
muradovm
22.10.2015 10:58Использование "?." вместо "." везде невозможна потому, что BCL предусмотрены значимые типы.
int i = Person.Age;
muradovm
22.10.2015 11:01Вариант 2 — первый пример с list. Можно научить компилятор оптимизировать такие вещи.
muradovm
22.10.2015 11:04public void Process() { var processor = new EmptyProcessor(); int result = DoProcess(new Order(), processor); } private int DoProcess(Order order, Processor processor) { return processor.Process(order); }
Если DoProcess не был бы приватным, то все равно пришлось бы писать проверку на null.mayorovp
22.10.2015 11:48Но это была бы проверка контракта, а не одна из допустимых веток кода.
muradovm
22.10.2015 12:34Давайте разберем изначальный код:
public void Process() { int result = DoProcess(new Order(), null); } private int DoProcess(Order order, Processor processor) { return processor?.Process(order) ?? 0; }
Если processor == null, то нужно вернуть 0. Если бы DoProcess был бы открытым, то мы вынуждены были бы оставить это поведение при рефакторинге. Соответственно, оставили бы как одну из допустимых веток кода.mayorovp
22.10.2015 12:39Речь шла не о рефакторинге, а о написании кода в первый раз.
Да и при рефакторинге менять поведение — также допустимо, просто изучать и переписывать в таком случае надо больше кода.muradovm
22.10.2015 12:47>> Речь шла не о рефакторинге, а о написании кода в первый раз
Это спор о терминах. Автор предлагает один код вместо другого, но их поведение не эквивалентно.
>>Да и при рефакторинге менять поведение — также допустимо, просто изучать и переписывать в таком случае надо больше кода.
Как бы то ни было автор предлагает именно этот кусок кода. Мы можем предположить, что он изолирован от всего остального.
muradovm
22.10.2015 11:11Вариант 3 очень спорный. Customer намеренно открыл Employees — какие тут могут быть нарушения.
mayorovp
22.10.2015 11:52+1Согласен. Для доменной модели такой код был бы довольно странным — но для модели EF это норма.
В конце концов, предложенный автором метод GetAdminAddress тоже кто-то должен реализовать — и как же он будет реализован? :)
Sing
22.10.2015 12:25И ещё:
> устраняя необходимость в проверках на нал
То есть, внутри той функции они внезапно не смогут быть null или как оно устранено?
vkhorikov
22.10.2015 14:28> устраняя необходимость в проверках на нал
Имеется ввиду что второй случай — это и инкапсуляция, и отсутвие проверок на null в самом методе, первый случай — только отсуствие проверок на null.
По поводу того, что внутри GetAdminAddress будет такие примерно такой же код:
return Employees ?.SingleOrDefault(x => x.IsAdmin)?.Address?.ToString();
В данном случае речь не о том, что оператор нельзя использовать (использовать его можно), а о том, что он скрывает проблемы в дизайне первоначального метода. Т.е. первый случай его использования нарушает принципы инкапсуляции, второй — нет, т.к. Customer обращается ко своим внутренним членам.Sing
22.10.2015 15:03+3> Имеется ввиду что второй случай — это и инкапсуляция, и отсутвие проверок на null в самом методе, первый случай — только отсуствие проверок на null.
Отсутствие проверок на null «в самом методе» — не устранение необходимости в проверках, особенно учитывая, что:
> внутри GetAdminAddress будет такие примерно такой же код
Здесь уже спрашивали, но я спрошу ещё раз, раз речь про инкапсуляцию: как она нарушается?
Если у клиента Customer открыты Employees, что нам мешает их опрашивать?
Что, если класс Customer писали не мы, а другая компания?
Что, если надо будет получить не адрес, а телефон? Не у админа, а у Семён Семёныча?
Если админов много, то ваш метод GetAdminAddress выдаст null, даже если там у каждого есть адрес. Вообще, такой метод нужен в ответ на запрос «Если в компании есть администратор, притом только один, мне нужно знать его адрес в таком виде, в каком получится». Я даже не знаю, сколько принципов нарушает такой подход.vkhorikov
22.10.2015 15:52>Здесь уже спрашивали, но я спрошу ещё раз, раз речь про инкапсуляцию: как она нарушается?
Инкапсуляция нарушается тем, что метод DoSomething делает суждения полностью базируясь на внутренностях класса Customer. Это по сути определение инкапсуляции: если данные полностью принадлежат одному классу, то методы по работе с этими данными должны также быть в этом классе. Второй пример показывает восстановление инкапсуляции — логика по получению адреса админа перенесена в Customer.mayorovp
22.10.2015 16:16Изменим вопрос: почему открытое свойство Employees считается внутренностями класса Customer?
vkhorikov
22.10.2015 16:55Потому что оно находится внутри класса Customer.
Разверну ответ. Для многих подобный код считается нормой:
var boss = customer.Employees.Max(x => x.Salary).FirstOrDefault();
Тем не менее, это также является нарушением инкапсуляции, т.к. здесь логика по определению «босса» отвязана от данных, с которыми эта логика работает. Правильным с т.з. принципов инкапсуляции решением будет вынести эту логику в класс Customer.mayorovp
22.10.2015 17:09Почему открытое свойство «находится внутри класса» — а открытый метод нет?
vkhorikov
22.10.2015 17:33Дело не в технических отличиях метода и проперти, дело в наличии бизнес-логики и в том, где она хранится.
mayorovp
22.10.2015 18:28Вот именно. Бизнес-логика должна быть в слое бизнес-логики. Зачем вы пытаетесь засунуть ее в слой доступа к данным?
vkhorikov
22.10.2015 18:56Причем здесь слой доступа к данным вообще?
mayorovp
22.10.2015 19:05-1Притом, что Customer выглядит как типичная сущность EF Code First, судя по способу работы с ней. А это — либо слой доступа к данным, либо еще более низкий слой.
Fireball222
22.10.2015 19:36Основная идея EF Code First в том, чтобы сущность из «слоя доступа к данным» перешла в доменную модель.
Sing
22.10.2015 16:16Допустим, чтобы не было противоречий, у вас дальше должен быть код:
return Employees.SingleOrDefault(x => x.IsAdmin).GetAddress();
А там:
return this.Address?.ToString();
Правильно?
Я лично, всегда понимал инкапсуляцию как защиту внутреннего состояния объекта от внешнего вмешательства. Данные внутренним состоянием, вроде, являться не должны.
Вы так и не ответили на остальные вопросы. Для каждого поля делать по методу?vkhorikov
22.10.2015 17:02Нет, инкапсуляция — это объединение данных с логикой + сокрытие информации от клиентов: en.wikipedia.org/wiki/Encapsulation_(computer_programming). Защита внутреннего состояние объекта — это соблюдение инвариантов и больше в сторону контрактного программирования.
>Правильно?
Поясните вопрос, не уверен что понял.
>(1)Если у клиента Customer открыты Employees, что нам мешает их опрашивать?
Ничего до тех пор пока логика опроса не базируется полностью на данных из Customer. К примеру опрос с целью сохранения части кастомеров во внешней коллекции по каким-то признакам — не является нарушением инкапсуляции. Пример выше:
var boss = customer.Employees.Max(x => x.Salary).FirstOrDefault();
— является
>(2)Что, если класс Customer писали не мы, а другая компания?
Если доступа к классу нет, то тут уж ничего не поделаешь. Это тем не менее также будет или не будет являться нарушением в зависимости от (1)
>Что, если надо будет получить не адрес, а телефон? Не у админа, а у Семён Семёныча?
Нужно больше данных, непонятно что конкретно вы имеете ввидуSing
22.10.2015 17:28Нет, инкапсуляция — это объединение данных с логикой + сокрытие информации от клиентов:
Как это нет, когда по вашей ссылке:Encapsulation is used to hide the values or state of a structured data object inside a class, preventing unauthorized parties' direct access to them.
Поясните вопрос, не уверен что понял.
Далее, мы используем класс Employee, стало быть для получения адреса нам нужно вызвать его метод. Иначе, мы используем функцию ToString() на внутренних данных другого класса, что по вашей логике есть нарушение инкапсуляции. IsAdmin, кстати, тоже, видимо не получится использовать и придётся писать ещё и GetAdmin()
К примеру опрос с целью сохранения части кастомеров во внешней коллекции по каким-то признакам — не является нарушением инкапсуляции.
Ага, то есть, если я хочу сохранить часть кастомеров — это хорошо. А если я хочу сохранить часть работников кастомера — это плохо. То есть, я могу, но только через метод. Хотя геттер — тоже метод, но он даёт мне возможность выбирать любые варианты. Не сходится.
Пример выше:
var boss = customer.Employees.Max(x => x.Salary).FirstOrDefault();
— являетсяНужно больше данных, непонятно что конкретно вы имеете ввиду
В данном методе вы ищете администратора. Хорошо, бывает. Потом, я захотел найти не-администратора с зарплатой до 100 рублей и фамилией Нафтазаров, и выяснить его стаж, а не адрес. Ещё один метод писать?vkhorikov
22.10.2015 17:43>Как это нет, когда по вашей ссылке:
Если вы про это: " preventing unauthorized parties' direct", то это о сокрытии информации, а не о соблюдении инвариантов.
>что по вашей логике есть нарушение инкапсуляции
Да, верно, внутри GetAdminAddress — такие же правила что и в DoSomething()
>Хотя геттер — тоже метод, но он даёт мне возможность выбирать любые варианты
Опять же — тут дело в наличии бизнес логики и в том, где она находится
>Потом, я захотел найти не-администратора с зарплатой до 100 рублей и фамилией Нафтазаров, и выяснить его стаж, а не адрес. Ещё один метод писать?
Да, либо еще один метод, либо параметр в существующий, в зависимости от ситуации
VolCh
22.10.2015 19:52Здесь уже спрашивали, но я спрошу ещё раз, раз речь про инкапсуляцию: как она нарушается?
Если у клиента Customer открыты Employees, что нам мешает их опрашивать?
Тут не столько инкапсуляция нарушается (если, конечно все эти методы и свойства открыты не исключительно для получения адреса админа), сколько сильно увеличивается связанность.
kstep
22.10.2015 11:48-2А всего-то надо было убрать из языка null и ввести Option.
mayorovp
22.10.2015 11:50+4(тут должна быть картинка с Боромиром)
Нельзя так просто взять и убрать null из языка в его шестой версии.
creker
22.10.2015 12:07+1И появилось бы не меньшее количество статей о том, как правильно пользоваться таким языком и как было бы проще жить с null. На github вроде бы поддерживается идея того, на что ссылается статья — пометки для компилятора о том, что не может быть null. Вполне реально, что появится, потому что не требует модификации CLR и является лишь проверкой времени компиляции.
kstep
22.10.2015 14:49Ну так Option и есть такая «пометка для компилятора о том, что (не) может быть null». Только отслеживается это на уровне системы типов. А в виде оператора ".?" это больше похоже на костыль.
creker
22.10.2015 16:00Скорее синтаксический сахар — никто этим оператором проблему null решать не собирался вроде. Вот тема с обсуждением этого предложения github.com/dotnet/roslyn/issues/227 Пока все не так однозначно, потому в том же swift, который имеет нечто похожее, что предлагается, это лишь в теории красиво и прекрасно, а на практике решает одни проблемы и создает новые. Собственно, как это всегда и бывает.
В любом случае, на данном этапе отказаться от null будет невозможно. Так же как заменить все на non-nullable по-умолчанию.
Razaz
22.10.2015 21:37Ну в связке с [NotNull] вполне приемлемо. Это уже часть гайдлайнов к Asp.Net. Надеюсь включат в компилятор.
Seekeer
22.10.2015 11:54public void Process()
{
var processor = new EmptyProcessor();
int result = DoProcess(new Order(), processor);
}
private int DoProcess(Order order, Processor processor)
{
return processor.Process(order);
}
То есть теперь перед каждым вызовом DoProcess нужно быть уверенным, что мы передаём ему не пустой объект. В итоге получается, что результат оба способа дают одинаковый, но первый может упасть из-за NullReferenceException. В чём выигрыш-то?
Код выше нарушает принципы инкапсуляции.
Абсолютно неочевидно. Почему вы так решили?
Alex_At_Net
22.10.2015 12:59Могу предположить, что алгоритм определения отправки выносится из Customer в DoSomething. Автор, очевидно, рассматривает инкапсуляцию как «все правила Customer помещать в него». В принципе ок, но Customer может стать очень большим :)
Seekeer
22.10.2015 13:36+1>«все правила Customer помещать в него»
То есть если мне нужно будет потом получить возраст админа, то в Customer добавится ещё метод GetAdminAge? :)
Замечательная инкапсуляция:)Alex_At_Net
22.10.2015 15:30В принципе, это всего-лишь принцип. Его надо правильно применять. Даже с GetAdminAge() не столь все однозначно — если это процесс сложный (ну там не просто года вычислить, а может какой более сложный алгоритм (учитывать года проведенные в анабиозе или нет или летал ли он с околосветовой скоростью и сколько)) то дублировать эту логику в каждом месте, где надо его вычислить будет не очень. Так что это кандидат на метод Employee. Если используется несколько данных из Customer для вычисления — то в Customer is Ok. Если, конечно, это специфично для какого-то бизнес-процесса (начисления з/п за стаж?) и больше нигде не надо, то, конечно, вообще в него.
В общем написал человек и написал На 75% можно согласиться :)
muradovm
22.10.2015 12:42public void DoSomething(Customer customer) { string address = customer?.Employees ?.SingleOrDefault(x => x.IsAdmin)?.Address?.ToString(); SendPackage(address); }
и
public void DoSomething(Customer customer) { Contract.Requires(customer != null); string address = customer.GetAdminAddress(); SendPackage(address); }
Так все таки разрешается передавать нулевой customer в метод или нет? Два этих участка кода не эквивалентны — рефакторинг не удался.
DrReiz
22.10.2015 12:46+2В чем преимущество создания отдельного объекта под паттерн Null Object? Почему не стоит под паттерн NullObject использовать значение null?
lair
22.10.2015 13:37+3очему не стоит под паттерн NullObject использовать значение null?
Потому что единственное поведение, которым обладаетnull— это бросить NRE, в то время как Null Object может предоставлять осмысленные (в данном контексте) заглушки.DrReiz
22.10.2015 18:12+1Значение null совместно с operator-ом '?.' дает поведение: возврат null или выполнение ничего. В 99% задач достаточно такого поведения для паттерна Null Object.
lair
22.10.2015 18:15+2В 99% задач достаточно такого поведения для паттерна Null Object.
Откуда статистика?
Понимаете, Null Object — это когда я работаю с объектом как обычно (т.е. безо всяких?.), и получаю разумные значения. Скажем, характерный пример Null Object — это когда метод должен возвращатьIEnumerable<T>, и если значений нет, то он возвращает неnull, а пустой энумератор, что позволяет писать код без дополнительных проверок.Sing
22.10.2015 18:42А ловить ошибки? Сравнивать с default()? Или для каждого возвращаемого значения придумывать своё «неправильное»?
lair
22.10.2015 18:44Какие ошибки? Для ошибок есть исключения.
(речь идет о ситуациях, когда «ничего не найдено» — это норма)Sing
22.10.2015 18:48Аааа, понятно. То есть, для коллекций в основном?
mayorovp
22.10.2015 19:02+4И не только. Стратегии, которые ничего не делают. Парсеры, которые ничего не парсят. Файловые потоки, которые всегда пустые…
/dev/null, он же nil — это тоже пример Null Object, уже на уровне операционной системы.
IP-адреса 127.0.0.1 и 0.0.0.0 также зачастую выступают в качестве Null Object если их прописывают в файле hosts.mayorovp
23.10.2015 06:29UPD: /dev/null, он же nul — это тоже пример Null Object, уже на уровне операционной системы
И никто не обратил внимания :)
DrReiz
22.10.2015 22:19Статистика из опредения паттерна Null Object. Null object при вызовах над ним — или возвращает Null Object, или выполняет ничего.
Если обычным кодом назвать код с '?.', то будет выполняться требование «работаю с объектом как обычно и получаю разумные значения».lair
23.10.2015 00:51Если обычным кодом назвать код с '?.',
Вот только это не обычный код, у него семантика другая. И в конце он все равно потребует coalescing, что радикально ухудшит читабельность.DrReiz
23.10.2015 02:43Семантика исходная с небольшим изменением: было T1 -> T2, стало T1 | null -> T2 | null.
Зачем coalescing, если достаточно расширить результирующий тип до: TResult | null?lair
23.10.2015 11:14Семантика исходная с небольшим изменением:
Вот это изменение и меняет семантику.
Зачем coalescing, если достаточно расширить результирующий тип до: TResult | null?
Затем, что (например) C# не умеет делатьforeachпоnull, аStream.CopyTo(null)дастArgumentNullException. И так далее.
А хуже всего то, что единожды вернувnullгде-то глубоко внутри, вы вынуждены теперь оборачивать проверками каждый уровень до самого верха (или пока вы не сделаете coalesce).DrReiz
23.10.2015 12:51Почему coalesce ухудшает читабельность? По семантике — это замена одного Null Pattern-а на другой.
foreach (var item in GetItems().Or_Empty())lair
23.10.2015 13:14+2Во-первых, этот код неконсистентен с предложенным вами же правилом «использовать
?.».
Во-вторых, код ниже читается лучше:
foreach (var item in GetItems())
Alex_At_Net
22.10.2015 15:34+5Пожалейте мои глаза, не пишите «проверки на нал». Как не прочту, все какая-то ерунда мерещится :)
Shablonarium
22.10.2015 17:45Из приведенных примеров совершенно не очевидно в чем преимущество паттерна Null object перед?.. operator, непонятно кто адресат этой статьи. Я вот думаю над инкапсуляцией и все же мне кажется, что не вся логика обработки данных объекта должна быть внутри него, иногда какой-то процесс манипулирует несколькими исходными объектами для получения новых. А иногда объекты и вовсе не сущности, а просто ссылочные структуры данных. Поправьте если не прав.
Joshua
22.10.2015 18:26По моему не раскрыта главная причина использования — LINQ. Новый оператор позволяет делать читаемые запросы к модели, в которой обоснованно используются null.
whitepen
23.10.2015 11:28+1Есть двоичная логика, а есть троичная — {да, нет, шеф все пропало}. Для одних объектов уместна одна, для других — другая. Можно за уши притянуть одну логику к другой. Вот если бы к типу объекта приколачивался размер логики… а редактор подкрашивал желтеньким… а конструктор возвращал облом… Вон даже в oracle строчки прыгают с одной логики на другую — чисто для совместимости с прошлым веком.
lair
23.10.2015 11:44Вот если бы к типу объекта приколачивался размер логики…
Ну так discriminated unions в полный рост.
kenoma
Альтернатива использованию контрактов — обычные проверки?
Bonart
Fody.NullGuard — полный спектр проверок на null при сохранении кода гладким и шелковистым :)