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

Есть такая штука как MediatR. Вот прям 1 в 1 то, что в статье написано. Плюс еще много чего.

Проблема в том, что замена явных вызовов неявными затрудняет восприятие логики кода.
Да, эти цепочки if'ов выглядят ужасно, но зато там всё на виду, и когда вы год спустя будете читать этот код, то с первого взгляда поймёте, что он вызывается по событию OnUserAction если выполняется условие COND.
А вот когда всё это замылено тоннами всяких хэндлеров, которые вызывают и перевызывают друг друга с помощью команд, да ещё и сверху всё засыпано динамическим связыванием, то вот вы смотрите на обработчик команды и думаете: а когда, чёрт возьми, вызывается этот код? Кем? При каких условиях? Вам даже контекстный поиск не поможет найти, где происходят вызовы и происходят ли они вообще, или это давно уже мёртвый код.
А теперь представьте, что у нас тут такие вызовы идут множественно, цепочкой. Обработчик одной команды использует другие команды для выполнения каких-то функций.
Код превращается в груду спагетти, и без запущенного отладчика вы в нём вообще никогда не разберётесь.
Если человек понимает в ООП, SOLIDы там всякие и паттерны (чего как-бы ожидаешь от человека, пишущего на С#), то он, с вероятностью в 100% поймет что-где-как. А тонна if-else-if, в которых вложено еще 5 таких-же if-else-if это и есть груда спагетти, где без листа, карандаша и кучи времени не разберешь что происходит, пока не декомпозируешь это мясо на листе

Понять-то поймет, бесспорно, но боюсь, что так же будет нужен листок и карандаш, а времени, вероятно, уйдет даже больше.

'Если человек понимает в ООП, SOLIDы там всякие и паттерны (чего как-бы ожидаешь от человека, пишущего на С#), то он, с вероятностью в 100% поймет что-где-как'


Тут больше вопрос к предметной области и знанию всех возможных путей обработки запросов и где и когда и какой обработчик может быть вызван

Начнём с того, что command-handler связка, да и вообще данный подход к ООП имеет такое же отношение как нога лягушки к человеку.


А так в целом на одном из уровней приложения этот подход неплох.

Тем не менее, этот паттерн (связка) описан в GOF в бородатом году и имеет к ООП ровно такое-же отношение, как и к любой другой парадигме, где это можно использовать. ИМХО, слишком уж резко будет говорить, что оно «как нога лягушки к человеку»)

Раскрою свою мысль.
Сам паттерн не требует ООП.


Он отлично реализуется и на процедурных и на функциональных языках.

Проблема то в том, что у тебя останется тонна else-if в которые вложены еще 5 else-if
Только понять их станет еще сложнее.
На самом деле есть простое эмпирическое правило — для плоского варианта компарации (прямо рядок else-if или switch) лучше так и оставить плоский вариант.
Выносить куда то нужно в том случае, если у тебя есть несколько одинаковых сравнений (то есть тебе пришлось написать несколько методов с одинаковым else-if, но разными условиями)

Что касается вложенных else-if, то их стоит декомпозировать скорее через встроенные методы и явное именование

Просто не все видимо разобрались в expression problem, когда нам стоит рассуждать об объектах в терминах набора возможных видов этих самых объектов с некоторым общим полиморфным поведением (тогда ООП нам помогает, наследования, виртуальные функции и всё-всё-всё) или в терминах набора возможных фичей этих объектов, когда у нас наоборот иерархия фиксированная, а вот разных функций может быть много: типичный пример — обработка дерева выражений. Язые меняется раз в 100 лет и классы описывающие синтаксическое дерево тоже одни и те же, а вот функции-обработчики могут быть разные. В ООП языках это обычно решается всякими визиторами, но по сути это решается через ADT и матчинг, как на первой картинке.


С чего бы вдруг у авторов внезапно один подход тупо "супериор", а другой — помойка для студентов сказать трудно. И то и то полезно, вопрос в одном: планируем ли мы расширять количетсво объектов или набор функционала. Если вдруг ответ "и то и то" — то вам тогда в tagless final (object algebras для более слабых ЯП), но это явно не то про что статья.


А АДТ в шарпе последних версий можно сравнительно удобно эмулировать на рекордах:


img

О! Спасибо. По отдельности и то, и другое использую. А вместе как-то не дошёл. Пошёл курить идею.
При просмотре кода в статье такие же мысли возникли) смысл того, что назвали «некрасивым, плохо расширяемым итд», дошел сразу, а то как предложили оптимизировать — пришлось напрячься

Я бы не сказал, что идея лишена смысла. Например, примерно тот же подход используется в используемой мной библиотеке для парсинга аргументов командной строки, и это действительно удобнее if-else.


Единственное отличие — в моём случае список обработчиков задаётся явным образом, а не через атрибуты. Да, это плюс одно место, которое нужно менять при добавлении функционала, зато всё делается явно и прозрачно для IDE.

вот согласен с вами… как раз сейчас рефакторю такой проект, дали на доработку, из зависимостей обратно всё перевожу в if-else потому что сложная бизнес-логика, невозможно отлаживать. хотя сам подход хороший, наверно применять его всё-таки лучше в библиотеках а не в конечном коде, который часто подвергается изменениям под требования заказчика

Этот подход не бесплатный с точки зрения производительности. Поэтому в библиотеках он ещё хуже. Библиотека может оказаться довольно глубоко в цепочке зависимостей и любая её неэффективность будет множителем в общей производительности. Библиотеки как раз надо оптимизировать под периодическую читаемость(вероятно даже не авторами), стабильность требований и высокую производительность. А тут предлагается решение как раз для облегчения модификации, если оно эту задачу не решает, то оно так же эффективно как и SOLID.
Описанное в статье очень близко к тому, как работают actions и effects в flux. По крайней мере, в Angular реализации — ngrx. Так вот, чтобы нормально работать с ngrx в Angular, ставишь Redux Dev Tools и тогда все эти связи, как на ладони.
И в этом-то и проблема, в шарпе нет Redux Dev Tools и потому отладка сложной логики с командами и обработчиками превратиться в ад.
Так что соглашусь, не самое лучшее решение проблемы if-else.

Мы для этой цели буквально за считанные дни написали симпатичные визуализаторы, с анимацией и куртизанками. И тесты писать не в пример легче. Декларативные conditions, с уклоном на type-driven разработку в итоговом счетё более маржинальны, чем спрятанные в коде императивные условия. И if-then, особенно с закрытыми third-party plugins, сильно усложняет систему. Как с точки зрения дизайна, так и последующей отладки.

то есть пришлось писать закрытый велосипед. Так победим
Во-первых, все было просто и понятно, а вы усложнили код на два порядка просто так. Во-вторых, если так хочется избавиться от case по значению, посмотрите на мульмиметоды. Почти в каждом языке их легко написать самому.

ru.wikipedia.org/wiki/%D0%9C%D1%83%D0%BB%D1%8C%D1%82%D0%B8%D0%BC%D0%B5%D1%82%D0%BE%D0%B4
Кроме уродливого использования if-elseif-else основная проблема заключается в том, что нужно добавлять ветвление для каждой новой причины обновления.

Тихим голосом…
А вот в Rust можно было бы воспользоваться enum'ом и вместо if-elseif-else взять match. Тогда при добавлении нового типа компилятор подсветил бы где нужно не забыть добавить обработчик в match для этого emun'а.
enum Coin {
    Penny,
    Nickel,
    Dime,
    Quarter,
}

fn value_in_cents(coin: Coin) -> u8 {
    match coin {
        Coin::Penny => 1,
        Coin::Nickel => 5,
        Coin::Dime => 10,
        //Coin::Quarter => 25,
    }
}


Выдаст ошибку компиляции:
18 | match coin {
| ^^^^ pattern `Quarter` not covered
|

Так это много где можно, в том числе и в C#, ЕМНИП.


Вообще, какая-то боянистая статья ради статьи. Как и "If-Else Is a Poor Man’s Polymorphism", на которую он ссылается. Он выдает за какое-то откровение сверху вещи, которые уже тысячу раз за последние несколько десятков лет обжевывали: if vs polymorphism vs pattern matching.

Та давно это можно и в шарпе.

var ordered = firstSort.Field.Trim().ToLower() switch
                {
                    "id" => SortProcessor.OrderBy(shipments, firstSort.Order, s => s.Id),
                    "createdby" => await SortProcessor.OrderBy(
                        shipments,
                        firstSort.Order,
                        async () => await _userManager.Enumerate(),
                        (s, users) => users.FirstOrDefault(u => u.Id == s.CreatedById)?.Name),
                    "createdat" => SortProcessor.OrderBy(
                        shipments,
                        firstSort.Order,
                        s => s.CreatedAt),
                    "status" => SortProcessor.OrderBy(
                        shipments,
                        firstSort.Order,
                        s => s.Status),
                    "department" => await SortProcessor.OrderBy(
                        shipments,
                        firstSort.Order,
                        async () => await _departmentService.Enumerate(),
                       (s, departments) => departments.FirstOrDefault(d => d.Id == s.DepartmentId)?.Name),

                    _ => shipments.OrderByDescending(s => s.CreatedAt)
                };


P.S. С телефона сложно набирать пример, скопировал из проекта.
ИМХО, конечно, но в описанном случае if-else действительно не очень. Switch-case уместнее.

В предложенном решении IDE покажет иерархию вызовов конечных функций? Можно будет «побегать» по цепочке и посмотреть все условия которые выполняются для вызова функции? Мне видится что нет, а это не удобно.

При такой схеме нет необходимости изменять существующий код, даже при создании новой команды или обработчика. Всё привязывается при запуске приложения.

Код и так будет изменен как минимум в двух местах, в чем проблема сделать ровно одно дополнительное изменение, один раз.

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

Так и будет. Код не решает реальных проблем, только создает. Он плохо читается. Может только добавить ошибок. Он медленнее чем действительно простое решение (хоть масштабы «медленности» и не важны).

Смысл в том, что написав этот код один раз, вы больше не должны будете его касаться. Достаточно написать юнит-тесты, и всё будет в порядке.

Или не писать все это и все останется в порядке.

Ну правда, я прочитал статью и так и не понял какую конкретно проблему решает описанный подход? Вместо варианта с одной функцией switch-case (будем считать что if-else уже поменяли на switch) на ~20 строк, простого как топор, супер быстрого не зависимо от количества «reason» вариантов, читаемого, с работающим поиском путей вызова, получилось не читаемое решение на сотню строк с циклами и потенциальными ошибками.
Осталось проверить насколько следуют обозначенным принципам наши новые абстракции.
На мой взгляд. Это плохая практика. Через некоторое время никто не поймёт, что здесь происходит. Понимаемость свелась к нулю.

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

Мм… А если одновременно поменяется и адрес и мыло?
Может там не надо ..else if..? А просто несколько отдельных if? Для каждого случая.
Кроме уродливого использования if-elseif-else основная проблема заключается в том, что нужно добавлять ветвление для каждой новой причины обновления. Это явное нарушение принципов открытости/закрытости и единственной ответственности.

А при необходимости написать какой-нибудь ReplaceAsync вместо того чтобы написать рядом такую же функцию с таким же свитчом рядом потребуется править все хендлеры чтобы научить обрабатывать новый тип сообщения Replace — это ни разу не нарушение принципа единой ответственности. "Смотри, не перепутай!" называется.


С такой любовью к рефлекшну и динамике автору возможно стоит посмотреть в сторону питона/жс/..,, т.к. видно что типы его только ограничивают и мешают творить.

Хочу обратить внимание, что в статье речь не о чистой замене if-elseif-else, а о замене if-elseif-else-goto, где goto (уже заменен на функцию) зацикливает выполнение if. Получается предлагаемая замена годится только для динамического if, если кто понимает мою мысль.
Я так понимаю, в статье в очередной раз переизобрели CQRS через Mediatr.

Лучше бы этими терминами все и описали, было бы полезнее.
Стесняюст спросить две вещи:
— чем плох switch для первоначального кода
— почему автор ударяется в долженствования по поводу мудрокода, который он наворотил.

Большую часть проекта составляет поддержка и отладка кода. Серебряной пули, которая за счет того или иного синтаксиса позволит избавиться от багов, нет. Уровни абстракции во-первых добавляют к latency, во вторых колоссально затрудняют чтение кода, особенно когда надо переходить по слабому связыванию.

Само слабое связывание хорошо, и подозреваю, чот автор что-то такое и имел в виду в кач-ве пользы. Однако оверинжиниринг и многократное увеличеине времени отладки без необходимости — это банальное воровство ресурса.
Ты неправильно используешь DI .NET-а
CommandDispatcher создаётся как Scoped а по факту это Singleton так как его состояние hendlerTypes существует в одном экземпляре для всего приложения. Все сервиси в нем тоже синглтоны, и ты только зря засоряешь ими свой DI, ты же их не будешь использовать через него потом, мог бы просто в List положить их, а не в ServiceCollection, а так у тебя в ServiceCollection куча мусора который замедляет работу приложения, и поиск реальных сервисов которые создаются через DI.
P.S. А сама идея хендлеров хорошая, в DDD же есть похожее, только там они реально через DI идут, а не через синглтон.
Зачем делать свой велосипед в 2021?
Давным давно уже существует MediatR от Jimmy Bogard.
И я подозреваю что автор знает о MediatR так как реализация один в один (только названия другие).
Но почему-то нигде в статье о всем хорошо известном MediatR не упомянул. Совпадения?
Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

Информация

Дата основания
Местоположение
Россия
Сайт
vdsina.ru
Численность
11–30 человек
Дата регистрации

Блог на Хабре