Comments 39
Есть такая штука как MediatR. Вот прям 1 в 1 то, что в статье написано. Плюс еще много чего.
Да, эти цепочки if'ов выглядят ужасно, но зато там всё на виду, и когда вы год спустя будете читать этот код, то с первого взгляда поймёте, что он вызывается по событию OnUserAction если выполняется условие COND.
А вот когда всё это замылено тоннами всяких хэндлеров, которые вызывают и перевызывают друг друга с помощью команд, да ещё и сверху всё засыпано динамическим связыванием, то вот вы смотрите на обработчик команды и думаете: а когда, чёрт возьми, вызывается этот код? Кем? При каких условиях? Вам даже контекстный поиск не поможет найти, где происходят вызовы и происходят ли они вообще, или это давно уже мёртвый код.
А теперь представьте, что у нас тут такие вызовы идут множественно, цепочкой. Обработчик одной команды использует другие команды для выполнения каких-то функций.
Код превращается в груду спагетти, и без запущенного отладчика вы в нём вообще никогда не разберётесь.
Понять-то поймет, бесспорно, но боюсь, что так же будет нужен листок и карандаш, а времени, вероятно, уйдет даже больше.
'Если человек понимает в ООП, SOLIDы там всякие и паттерны (чего как-бы ожидаешь от человека, пишущего на С#), то он, с вероятностью в 100% поймет что-где-как'
Тут больше вопрос к предметной области и знанию всех возможных путей обработки запросов и где и когда и какой обработчик может быть вызван
Начнём с того, что command-handler связка, да и вообще данный подход к ООП имеет такое же отношение как нога лягушки к человеку.
А так в целом на одном из уровней приложения этот подход неплох.
Только понять их станет еще сложнее.
На самом деле есть простое эмпирическое правило — для плоского варианта компарации (прямо рядок else-if или switch) лучше так и оставить плоский вариант.
Выносить куда то нужно в том случае, если у тебя есть несколько одинаковых сравнений (то есть тебе пришлось написать несколько методов с одинаковым else-if, но разными условиями)
Что касается вложенных else-if, то их стоит декомпозировать скорее через встроенные методы и явное именование
Просто не все видимо разобрались в expression problem, когда нам стоит рассуждать об объектах в терминах набора возможных видов этих самых объектов с некоторым общим полиморфным поведением (тогда ООП нам помогает, наследования, виртуальные функции и всё-всё-всё) или в терминах набора возможных фичей этих объектов, когда у нас наоборот иерархия фиксированная, а вот разных функций может быть много: типичный пример — обработка дерева выражений. Язые меняется раз в 100 лет и классы описывающие синтаксическое дерево тоже одни и те же, а вот функции-обработчики могут быть разные. В ООП языках это обычно решается всякими визиторами, но по сути это решается через ADT и матчинг, как на первой картинке.
С чего бы вдруг у авторов внезапно один подход тупо "супериор", а другой — помойка для студентов сказать трудно. И то и то полезно, вопрос в одном: планируем ли мы расширять количетсво объектов или набор функционала. Если вдруг ответ "и то и то" — то вам тогда в tagless final (object algebras для более слабых ЯП), но это явно не то про что статья.
А АДТ в шарпе последних версий можно сравнительно удобно эмулировать на рекордах:
Я бы не сказал, что идея лишена смысла. Например, примерно тот же подход используется в используемой мной библиотеке для парсинга аргументов командной строки, и это действительно удобнее if-else.
Единственное отличие — в моём случае список обработчиков задаётся явным образом, а не через атрибуты. Да, это плюс одно место, которое нужно менять при добавлении функционала, зато всё делается явно и прозрачно для IDE.
вот согласен с вами… как раз сейчас рефакторю такой проект, дали на доработку, из зависимостей обратно всё перевожу в if-else потому что сложная бизнес-логика, невозможно отлаживать. хотя сам подход хороший, наверно применять его всё-таки лучше в библиотеках а не в конечном коде, который часто подвергается изменениям под требования заказчика
И в этом-то и проблема, в шарпе нет Redux Dev Tools и потому отладка сложной логики с командами и обработчиками превратиться в ад.
Так что соглашусь, не самое лучшее решение проблемы if-else.
Мы для этой цели буквально за считанные дни написали симпатичные визуализаторы, с анимацией и куртизанками. И тесты писать не в пример легче. Декларативные conditions, с уклоном на type-driven разработку в итоговом счетё более маржинальны, чем спрятанные в коде императивные условия. И if-then, особенно с закрытыми third-party plugins, сильно усложняет систему. Как с точки зрения дизайна, так и последующей отладки.
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. С телефона сложно набирать пример, скопировал из проекта.
В предложенном решении IDE покажет иерархию вызовов конечных функций? Можно будет «побегать» по цепочке и посмотреть все условия которые выполняются для вызова функции? Мне видится что нет, а это не удобно.
При такой схеме нет необходимости изменять существующий код, даже при создании новой команды или обработчика. Всё привязывается при запуске приложения.
Код и так будет изменен как минимум в двух местах, в чем проблема сделать ровно одно дополнительное изменение, один раз.
Да, это выглядит запутанным, и на первый взгляд покажется, что поддержка такого кода будет настоящим кошмаром.
Так и будет. Код не решает реальных проблем, только создает. Он плохо читается. Может только добавить ошибок. Он медленнее чем действительно простое решение (хоть масштабы «медленности» и не важны).
Смысл в том, что написав этот код один раз, вы больше не должны будете его касаться. Достаточно написать юнит-тесты, и всё будет в порядке.
Или не писать все это и все останется в порядке.
Ну правда, я прочитал статью и так и не понял какую конкретно проблему решает описанный подход? Вместо варианта с одной функцией switch-case (будем считать что if-else уже поменяли на switch) на ~20 строк, простого как топор, супер быстрого не зависимо от количества «reason» вариантов, читаемого, с работающим поиском путей вызова, получилось не читаемое решение на сотню строк с циклами и потенциальными ошибками.
Чтобы понять код миддла, нужно быть миддлом.
Чтобы понять код сеньора, достаточно быть джуном.
(С) лень гуглить авторство
В общем, я за первый вариант, как наиболее выгодный бизнесу
Может там не надо ..else if..? А просто несколько отдельных if? Для каждого случая.
Кроме уродливого использования if-elseif-else основная проблема заключается в том, что нужно добавлять ветвление для каждой новой причины обновления. Это явное нарушение принципов открытости/закрытости и единственной ответственности.
А при необходимости написать какой-нибудь ReplaceAsync вместо того чтобы написать рядом такую же функцию с таким же свитчом рядом потребуется править все хендлеры чтобы научить обрабатывать новый тип сообщения Replace — это ни разу не нарушение принципа единой ответственности. "Смотри, не перепутай!" называется.
С такой любовью к рефлекшну и динамике автору возможно стоит посмотреть в сторону питона/жс/..,, т.к. видно что типы его только ограничивают и мешают творить.
Лучше бы этими терминами все и описали, было бы полезнее.
— чем плох switch для первоначального кода
— почему автор ударяется в долженствования по поводу мудрокода, который он наворотил.
Большую часть проекта составляет поддержка и отладка кода. Серебряной пули, которая за счет того или иного синтаксиса позволит избавиться от багов, нет. Уровни абстракции во-первых добавляют к latency, во вторых колоссально затрудняют чтение кода, особенно когда надо переходить по слабому связыванию.
Само слабое связывание хорошо, и подозреваю, чот автор что-то такое и имел в виду в кач-ве пользы. Однако оверинжиниринг и многократное увеличеине времени отладки без необходимости — это банальное воровство ресурса.
CommandDispatcher создаётся как Scoped а по факту это Singleton так как его состояние hendlerTypes существует в одном экземпляре для всего приложения. Все сервиси в нем тоже синглтоны, и ты только зря засоряешь ими свой DI, ты же их не будешь использовать через него потом, мог бы просто в List положить их, а не в ServiceCollection, а так у тебя в ServiceCollection куча мусора который замедляет работу приложения, и поиск реальных сервисов которые создаются через DI.
P.S. А сама идея хендлеров хорошая, в DDD же есть похожее, только там они реально через DI идут, а не через синглтон.
Давным давно уже существует MediatR от Jimmy Bogard.
И я подозреваю что автор знает о MediatR так как реализация один в один (только названия другие).
Но почему-то нигде в статье о всем хорошо известном MediatR не упомянул. Совпадения?
Как избавиться от if-else при помощи команд и обработчиков