vkhorikov 22 окт 2015 в 04:05

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

4 мин

26K

.NET * C# * Программирование * Качество кода *

+13

Комментарии 80

kenoma 22 окт 2015 в 05:07

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

Bonart 24 окт 2015 в 15:29

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

muradovm 22 окт 2015 в 07:58

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

Mrrl 22 окт 2015 в 08:51

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

muradovm 22 окт 2015 в 09:26

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

muradovm 22 окт 2015 в 08:01

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

muradovm 22 окт 2015 в 08: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 22 окт 2015 в 08:48

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

muradovm 22 окт 2015 в 09: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 окт 2015 в 09:39

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

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

muradovm 22 окт 2015 в 09:47

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

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

muradovm 22 окт 2015 в 08:11

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

mayorovp 22 окт 2015 в 08:52

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

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

Mrrl 22 окт 2015 в 08:54

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

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

mayorovp 22 окт 2015 в 09:14

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

Sing 22 окт 2015 в 09:25

И ещё:

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

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

vkhorikov 22 окт 2015 в 11:28

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

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

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

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

Sing 22 окт 2015 в 12:03

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

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

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

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

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

mayorovp 22 окт 2015 в 12:24

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

Sing 22 окт 2015 в 12:41

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

vkhorikov 22 окт 2015 в 12:52

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

mayorovp 22 окт 2015 в 13:16

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

vkhorikov 22 окт 2015 в 13:55

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

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

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

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

mayorovp 22 окт 2015 в 14:09

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

vkhorikov 22 окт 2015 в 14:33

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

mayorovp 22 окт 2015 в 15:28

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

vkhorikov 22 окт 2015 в 15:56

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

mayorovp 22 окт 2015 в 16:05

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

Fireball222 22 окт 2015 в 16:36

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

mayorovp 22 окт 2015 в 17:02

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

Sing 22 окт 2015 в 13:16

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

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

А там:

return this.Address?.ToString();

Правильно?

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

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

vkhorikov 22 окт 2015 в 14: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 окт 2015 в 14: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 окт 2015 в 14:43

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

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

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

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

VolCh 22 окт 2015 в 16:52

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

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

kstep 22 окт 2015 в 08:48

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

mayorovp 22 окт 2015 в 08:50

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

Mrrl 22 окт 2015 в 08:58

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

creker 22 окт 2015 в 09:07

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

kstep 22 окт 2015 в 11:49

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

creker 22 окт 2015 в 13:00

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

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

Razaz 22 окт 2015 в 18:37

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

Seekeer 22 окт 2015 в 08: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 22 окт 2015 в 09:59

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

Seekeer 22 окт 2015 в 10:36

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

Alex_At_Net 22 окт 2015 в 12:30

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

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

muradovm 22 окт 2015 в 09: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 22 окт 2015 в 10:00

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

muradovm 22 окт 2015 в 10:02

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

mayorovp 22 окт 2015 в 10:23

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

DrReiz 22 окт 2015 в 09:46

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

lair 22 окт 2015 в 10:37

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

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

DrReiz 22 окт 2015 в 15:12

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

lair 22 окт 2015 в 15:15

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

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

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

Sing 22 окт 2015 в 15:42

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

lair 22 окт 2015 в 15:44

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

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

Sing 22 окт 2015 в 15:48

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

lair 22 окт 2015 в 15:50

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

mayorovp 22 окт 2015 в 16:02

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

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

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

mayorovp 23 окт 2015 в 03:29

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

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

DrReiz 22 окт 2015 в 19:19

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

lair 22 окт 2015 в 21:51

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

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

DrReiz 22 окт 2015 в 23:43

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

lair 23 окт 2015 в 08:14

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

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

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

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

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

DrReiz 23 окт 2015 в 09:51

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

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

lair 23 окт 2015 в 10:14

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

foreach (var item in GetItems())

Alex_At_Net 22 окт 2015 в 12:34

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

Shablonarium 22 окт 2015 в 14:45

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

Joshua 22 окт 2015 в 15:26

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

lair 22 окт 2015 в 15:27

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

mayorovp 22 окт 2015 в 15:31

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

lair 22 окт 2015 в 15:32

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

mayorovp 22 окт 2015 в 15:36

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

lair 22 окт 2015 в 15:39

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

Nagg 22 окт 2015 в 16:01

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

Dywar 22 окт 2015 в 16:30

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

whitepen 23 окт 2015 в 08:28

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

lair 23 окт 2015 в 08:44

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

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

whitepen 26 окт 2015 в 07:00

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

lair 26 окт 2015 в 11:16

type Tristate =
  | Yes
  | No
  | Houston

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

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

Зарегистрируйтесь на Хабре, чтобы оставить комментарий