vkhorikov Oct 22 2015 at 07:05

3 способа использовать оператор?.. неправильно в C# 6

4 min

26K

Programming*Perfect code*.NET*C#*

+14

Comments 80

kenoma Oct 22 2015 at 08:07

Альтернатива использованию контрактов — обычные проверки?

Bonart Oct 24 2015 at 18:29

Fody.NullGuard — полный спектр проверок на null при сохранении кода гладким и шелковистым :)

muradovm Oct 22 2015 at 10:58

Использование "?." вместо "." везде невозможна потому, что BCL предусмотрены значимые типы.
int i = Person.Age;

Mrrl Oct 22 2015 at 11:51

А разве такой код не сработает?
int? i=Person?.Age;

muradovm Oct 22 2015 at 12:26

Сработает. Но на .NET уже написано много кода с использованием значимых типов. А это значит, что «подставить» везде "?." вместо "." — отказаться от обратной совместимости.

muradovm Oct 22 2015 at 11:01

Вариант 2 — первый пример с list. Можно научить компилятор оптимизировать такие вещи.

muradovm Oct 22 2015 at 11:04

public 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 Oct 22 2015 at 11:48

Но это была бы проверка контракта, а не одна из допустимых веток кода.

muradovm Oct 22 2015 at 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 Oct 22 2015 at 12:39

Речь шла не о рефакторинге, а о написании кода в первый раз.

Да и при рефакторинге менять поведение — также допустимо, просто изучать и переписывать в таком случае надо больше кода.

muradovm Oct 22 2015 at 12:47

>> Речь шла не о рефакторинге, а о написании кода в первый раз
Это спор о терминах. Автор предлагает один код вместо другого, но их поведение не эквивалентно.

>>Да и при рефакторинге менять поведение — также допустимо, просто изучать и переписывать в таком случае надо больше кода.
Как бы то ни было автор предлагает именно этот кусок кода. Мы можем предположить, что он изолирован от всего остального.

muradovm Oct 22 2015 at 11:11

Вариант 3 очень спорный. Customer намеренно открыл Employees — какие тут могут быть нарушения.

mayorovp Oct 22 2015 at 11:52

Согласен. Для доменной модели такой код был бы довольно странным — но для модели EF это норма.

В конце концов, предложенный автором метод GetAdminAddress тоже кто-то должен реализовать — и как же он будет реализован? :)

Mrrl Oct 22 2015 at 11:54

и как же он будет реализован? :)

Цепочкой ещё из 3-4 функций такого же вида, вероятно.

mayorovp Oct 22 2015 at 12:14

Угу. И назовут это «прозрачный и понятный код»…

Sing Oct 22 2015 at 12:25

И ещё:

> устраняя необходимость в проверках на нал

То есть, внутри той функции они внезапно не смогут быть null или как оно устранено?

vkhorikov Oct 22 2015 at 14:28

> устраняя необходимость в проверках на нал
Имеется ввиду что второй случай — это и инкапсуляция, и отсутвие проверок на null в самом методе, первый случай — только отсуствие проверок на null.

По поводу того, что внутри GetAdminAddress будет такие примерно такой же код:

return Employees
        ?.SingleOrDefault(x => x.IsAdmin)?.Address?.ToString();

В данном случае речь не о том, что оператор нельзя использовать (использовать его можно), а о том, что он скрывает проблемы в дизайне первоначального метода. Т.е. первый случай его использования нарушает принципы инкапсуляции, второй — нет, т.к. Customer обращается ко своим внутренним членам.

Sing Oct 22 2015 at 15:03

> Имеется ввиду что второй случай — это и инкапсуляция, и отсутвие проверок на null в самом методе, первый случай — только отсуствие проверок на null.

Отсутствие проверок на null «в самом методе» — не устранение необходимости в проверках, особенно учитывая, что:

> внутри GetAdminAddress будет такие примерно такой же код

Здесь уже спрашивали, но я спрошу ещё раз, раз речь про инкапсуляцию: как она нарушается?
Если у клиента Customer открыты Employees, что нам мешает их опрашивать?
Что, если класс Customer писали не мы, а другая компания?
Что, если надо будет получить не адрес, а телефон? Не у админа, а у Семён Семёныча?

Если админов много, то ваш метод GetAdminAddress выдаст null, даже если там у каждого есть адрес. Вообще, такой метод нужен в ответ на запрос «Если в компании есть администратор, притом только один, мне нужно знать его адрес в таком виде, в каком получится». Я даже не знаю, сколько принципов нарушает такой подход.

mayorovp Oct 22 2015 at 15:24

Поправка: если админов много, метод SingleOrDefault кинет исключение.

Sing Oct 22 2015 at 15:41

Да, точно, спасибо. То есть ещё хуже: Нам нужен адрес админа, мы вызываем GetAdminAddress() и получаем InvalidOperationException.

vkhorikov Oct 22 2015 at 15:52

>Здесь уже спрашивали, но я спрошу ещё раз, раз речь про инкапсуляцию: как она нарушается?
Инкапсуляция нарушается тем, что метод DoSomething делает суждения полностью базируясь на внутренностях класса Customer. Это по сути определение инкапсуляции: если данные полностью принадлежат одному классу, то методы по работе с этими данными должны также быть в этом классе. Второй пример показывает восстановление инкапсуляции — логика по получению адреса админа перенесена в Customer.

mayorovp Oct 22 2015 at 16:16

Изменим вопрос: почему открытое свойство Employees считается внутренностями класса Customer?

vkhorikov Oct 22 2015 at 16:55

Потому что оно находится внутри класса Customer.

Разверну ответ. Для многих подобный код считается нормой:

var boss = customer.Employees.Max(x => x.Salary).FirstOrDefault();

Тем не менее, это также является нарушением инкапсуляции, т.к. здесь логика по определению «босса» отвязана от данных, с которыми эта логика работает. Правильным с т.з. принципов инкапсуляции решением будет вынести эту логику в класс Customer.

mayorovp Oct 22 2015 at 17:09

Почему открытое свойство «находится внутри класса» — а открытый метод нет?

vkhorikov Oct 22 2015 at 17:33

Дело не в технических отличиях метода и проперти, дело в наличии бизнес-логики и в том, где она хранится.

mayorovp Oct 22 2015 at 18:28

Вот именно. Бизнес-логика должна быть в слое бизнес-логики. Зачем вы пытаетесь засунуть ее в слой доступа к данным?

vkhorikov Oct 22 2015 at 18:56

Причем здесь слой доступа к данным вообще?

mayorovp Oct 22 2015 at 19:05

Притом, что Customer выглядит как типичная сущность EF Code First, судя по способу работы с ней. А это — либо слой доступа к данным, либо еще более низкий слой.

-1

Fireball222 Oct 22 2015 at 19:36

Основная идея EF Code First в том, чтобы сущность из «слоя доступа к данным» перешла в доменную модель.

mayorovp Oct 22 2015 at 20:02

И тем не менее, это не всегда возможно.

Sing Oct 22 2015 at 16:16

Допустим, чтобы не было противоречий, у вас дальше должен быть код:

return Employees.SingleOrDefault(x => x.IsAdmin).GetAddress();

А там:

return this.Address?.ToString();

Правильно?

Я лично, всегда понимал инкапсуляцию как защиту внутреннего состояния объекта от внешнего вмешательства. Данные внутренним состоянием, вроде, являться не должны.

Вы так и не ответили на остальные вопросы. Для каждого поля делать по методу?

vkhorikov Oct 22 2015 at 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 Oct 22 2015 at 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 Oct 22 2015 at 17:43

>Как это нет, когда по вашей ссылке:
Если вы про это: " preventing unauthorized parties' direct", то это о сокрытии информации, а не о соблюдении инвариантов.

>что по вашей логике есть нарушение инкапсуляции
Да, верно, внутри GetAdminAddress — такие же правила что и в DoSomething()

>Хотя геттер — тоже метод, но он даёт мне возможность выбирать любые варианты
Опять же — тут дело в наличии бизнес логики и в том, где она находится

>Потом, я захотел найти не-администратора с зарплатой до 100 рублей и фамилией Нафтазаров, и выяснить его стаж, а не адрес. Ещё один метод писать?
Да, либо еще один метод, либо параметр в существующий, в зависимости от ситуации

VolCh Oct 22 2015 at 19:52

Здесь уже спрашивали, но я спрошу ещё раз, раз речь про инкапсуляцию: как она нарушается?
Если у клиента Customer открыты Employees, что нам мешает их опрашивать?

Тут не столько инкапсуляция нарушается (если, конечно все эти методы и свойства открыты не исключительно для получения адреса админа), сколько сильно увеличивается связанность.

kstep Oct 22 2015 at 11:48

А всего-то надо было убрать из языка null и ввести Option.

-2

mayorovp Oct 22 2015 at 11:50

(тут должна быть картинка с Боромиром)
Нельзя так просто взять и убрать null из языка в его шестой версии.

Mrrl Oct 22 2015 at 11:58

Чем будут инициализироваться свежие массивы и поля объектов?

creker Oct 22 2015 at 12:07

И появилось бы не меньшее количество статей о том, как правильно пользоваться таким языком и как было бы проще жить с null. На github вроде бы поддерживается идея того, на что ссылается статья — пометки для компилятора о том, что не может быть null. Вполне реально, что появится, потому что не требует модификации CLR и является лишь проверкой времени компиляции.

kstep Oct 22 2015 at 14:49

Ну так Option и есть такая «пометка для компилятора о том, что (не) может быть null». Только отслеживается это на уровне системы типов. А в виде оператора ".?" это больше похоже на костыль.

creker Oct 22 2015 at 16:00

Скорее синтаксический сахар — никто этим оператором проблему null решать не собирался вроде. Вот тема с обсуждением этого предложения github.com/dotnet/roslyn/issues/227 Пока все не так однозначно, потому в том же swift, который имеет нечто похожее, что предлагается, это лишь в теории красиво и прекрасно, а на практике решает одни проблемы и создает новые. Собственно, как это всегда и бывает.

В любом случае, на данном этапе отказаться от null будет невозможно. Так же как заменить все на non-nullable по-умолчанию.

Razaz Oct 22 2015 at 21:37

Ну в связке с [NotNull] вполне приемлемо. Это уже часть гайдлайнов к Asp.Net. Надеюсь включат в компилятор.

Seekeer Oct 22 2015 at 11:54

public 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 Oct 22 2015 at 12:59

Могу предположить, что алгоритм определения отправки выносится из Customer в DoSomething. Автор, очевидно, рассматривает инкапсуляцию как «все правила Customer помещать в него». В принципе ок, но Customer может стать очень большим :)

Seekeer Oct 22 2015 at 13:36

>«все правила Customer помещать в него»
То есть если мне нужно будет потом получить возраст админа, то в Customer добавится ещё метод GetAdminAge? :)
Замечательная инкапсуляция:)

Alex_At_Net Oct 22 2015 at 15:30

В принципе, это всего-лишь принцип. Его надо правильно применять. Даже с GetAdminAge() не столь все однозначно — если это процесс сложный (ну там не просто года вычислить, а может какой более сложный алгоритм (учитывать года проведенные в анабиозе или нет или летал ли он с околосветовой скоростью и сколько)) то дублировать эту логику в каждом месте, где надо его вычислить будет не очень. Так что это кандидат на метод Employee. Если используется несколько данных из Customer для вычисления — то в Customer is Ok. Если, конечно, это специфично для какого-то бизнес-процесса (начисления з/п за стаж?) и больше нигде не надо, то, конечно, вообще в него.

В общем написал человек и написал На 75% можно согласиться :)

muradovm Oct 22 2015 at 12:42

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);
}

Так все таки разрешается передавать нулевой customer в метод или нет? Два этих участка кода не эквивалентны — рефакторинг не удался.

mayorovp Oct 22 2015 at 13:00

Запрещается в обоих случаях, просто в первом случае нет проверки.

muradovm Oct 22 2015 at 13:02

Есть проверка в первом случае — «customer?.»

mayorovp Oct 22 2015 at 13:23

Вот только является ли она частью логики — или кто-то добавил знак вопроса просто «на всякий случай», игнорируя принцип fail-fast?

DrReiz Oct 22 2015 at 12:46

В чем преимущество создания отдельного объекта под паттерн Null Object? Почему не стоит под паттерн NullObject использовать значение null?

lair Oct 22 2015 at 13:37

очему не стоит под паттерн NullObject использовать значение null?

Потому что единственное поведение, которым обладает null — это бросить NRE, в то время как Null Object может предоставлять осмысленные (в данном контексте) заглушки.

DrReiz Oct 22 2015 at 18:12

Значение null совместно с operator-ом '?.' дает поведение: возврат null или выполнение ничего. В 99% задач достаточно такого поведения для паттерна Null Object.

lair Oct 22 2015 at 18:15

В 99% задач достаточно такого поведения для паттерна Null Object.

Откуда статистика?

Понимаете, Null Object — это когда я работаю с объектом как обычно (т.е. безо всяких ?.), и получаю разумные значения. Скажем, характерный пример Null Object — это когда метод должен возвращать IEnumerable<T>, и если значений нет, то он возвращает не null, а пустой энумератор, что позволяет писать код без дополнительных проверок.

Sing Oct 22 2015 at 18:42

А ловить ошибки? Сравнивать с default()? Или для каждого возвращаемого значения придумывать своё «неправильное»?

lair Oct 22 2015 at 18:44

Какие ошибки? Для ошибок есть исключения.

(речь идет о ситуациях, когда «ничего не найдено» — это норма)

Sing Oct 22 2015 at 18:48

Аааа, понятно. То есть, для коллекций в основном?

lair Oct 22 2015 at 18:50

Да много для чего. Например, конфигурация.

mayorovp Oct 22 2015 at 19:02

И не только. Стратегии, которые ничего не делают. Парсеры, которые ничего не парсят. Файловые потоки, которые всегда пустые…

/dev/null, он же nil — это тоже пример Null Object, уже на уровне операционной системы.

IP-адреса 127.0.0.1 и 0.0.0.0 также зачастую выступают в качестве Null Object если их прописывают в файле hosts.

mayorovp Oct 23 2015 at 06:29

UPD: /dev/null, он же nul — это тоже пример Null Object, уже на уровне операционной системы

И никто не обратил внимания :)

DrReiz Oct 22 2015 at 22:19

Статистика из опредения паттерна Null Object. Null object при вызовах над ним — или возвращает Null Object, или выполняет ничего.
Если обычным кодом назвать код с '?.', то будет выполняться требование «работаю с объектом как обычно и получаю разумные значения».

lair Oct 23 2015 at 00:51

Если обычным кодом назвать код с '?.',

Вот только это не обычный код, у него семантика другая. И в конце он все равно потребует coalescing, что радикально ухудшит читабельность.

DrReiz Oct 23 2015 at 02:43

Семантика исходная с небольшим изменением: было T1 -> T2, стало T1 | null -> T2 | null.
Зачем coalescing, если достаточно расширить результирующий тип до: TResult | null?

lair Oct 23 2015 at 11:14

Семантика исходная с небольшим изменением:

Вот это изменение и меняет семантику.

Зачем coalescing, если достаточно расширить результирующий тип до: TResult | null?

Затем, что (например) C# не умеет делать foreach по null, а Stream.CopyTo(null) даст ArgumentNullException. И так далее.

А хуже всего то, что единожды вернув null где-то глубоко внутри, вы вынуждены теперь оборачивать проверками каждый уровень до самого верха (или пока вы не сделаете coalesce).

DrReiz Oct 23 2015 at 12:51

Почему coalesce ухудшает читабельность? По семантике — это замена одного Null Pattern-а на другой.

foreach (var item in GetItems().Or_Empty())

lair Oct 23 2015 at 13:14

Во-первых, этот код неконсистентен с предложенным вами же правилом «использовать ?.».
Во-вторых, код ниже читается лучше:

foreach (var item in GetItems())

Alex_At_Net Oct 22 2015 at 15:34

Пожалейте мои глаза, не пишите «проверки на нал». Как не прочту, все какая-то ерунда мерещится :)

Shablonarium Oct 22 2015 at 17:45

Из приведенных примеров совершенно не очевидно в чем преимущество паттерна Null object перед?.. operator, непонятно кто адресат этой статьи. Я вот думаю над инкапсуляцией и все же мне кажется, что не вся логика обработки данных объекта должна быть внутри него, иногда какой-то процесс манипулирует несколькими исходными объектами для получения новых. А иногда объекты и вовсе не сущности, а просто ссылочные структуры данных. Поправьте если не прав.

Joshua Oct 22 2015 at 18:26

По моему не раскрыта главная причина использования — LINQ. Новый оператор позволяет делать читаемые запросы к модели, в которой обоснованно используются null.

lair Oct 22 2015 at 18:27

А EF начиная с какой версии его поддерживает?

mayorovp Oct 22 2015 at 18:31

EF будет думать, что это тернарный оператор.

lair Oct 22 2015 at 18:32

И во что это развернется в финальном SQL-запросе?

mayorovp Oct 22 2015 at 18:36

Если использовать его в выражениях Select — то в трудночитаемый человеком, но все еще понятный серверу запрос. Если использовать его в условиях Where — то в ночной кошмар оптимизатора запросов :)

lair Oct 22 2015 at 18:39

Вот-вот. При этом обычные точки, как ни странно, работают и не выносят мозг.

Nagg Oct 22 2015 at 19:01

Вы уверены что это вообще поддерживается?

Dywar Oct 22 2015 at 19:30

«Всё хорошо, что в меру». Гиппократ

whitepen Oct 23 2015 at 11:28

Есть двоичная логика, а есть троичная — {да, нет, шеф все пропало}. Для одних объектов уместна одна, для других — другая. Можно за уши притянуть одну логику к другой. Вот если бы к типу объекта приколачивался размер логики… а редактор подкрашивал желтеньким… а конструктор возвращал облом… Вон даже в oracle строчки прыгают с одной логики на другую — чисто для совместимости с прошлым веком.

lair Oct 23 2015 at 11:44

Вот если бы к типу объекта приколачивался размер логики…

Ну так discriminated unions в полный рост.

whitepen Oct 26 2015 at 10:00

Это вы к чему ваше дурнопахнущее трольство?

-1

lair Oct 26 2015 at 14:16

type Tristate =
  | Yes
  | No
  | Houston

type PersonRec =
  | Person of id : int
  | Unknown
  | None

type String = 
  | Value of data : char[]
  | Null

Show the best of all time