Хабр Курсы для бэкендеров
РЕКЛАМА
Практикум, Хекслет, SkyPro, авторские курсы — собрали всех и попросили скидки. Осталось выбрать!
Поток Бэкенд доступен 24/7 благодаря поддержке друзей Хабра
Комментарии 10
Если с первыми двумя антипаттернами я согласен правда непонятно почему это архитектурные проблемы то вот третий стоит обсудить. Название "Доменная логика прибита к фреймворку" и Вы демонстрируете как в сложном примере просто вынесли часть кода куда-то еще, причем тут вообще неважно куда, можно было вынести и в приватные (или публичные если надо их тестировать) методы. Я к тому что во первых на архитектурную проблему это снова не тянет. По факту если на проекте хоть в каком-то виде пишутся тесты то такого кода скорей всего в принципе не будет написано. А в случае когда тестирование только ручное то когда наоборот алгоритм целиком в одном месте и все понятно сразу из одного места, в некоторых случаях это скорее хорошо чем плохо, почему потому что легко понять горизонт логики. Я видел обратную сторону такого подхода когда открываешь метод, там 3 строчки но чтобы понять логику что происходит надо прыгать по сотне других методов по перекрестным ссылкам.
По поводу «вынести в приватные методы» — если вынести switch в приватный метод того же класса, класс всё равно зависит от dbContext, externalApi и _logService. Тестировать бизнес-правило отдельно от инфраструктуры не получится — моки тех же 5 зависимостей. Суть не в том, чтобы код стал короче, а в том, чтобы бизнес-логика не зависела от инфраструктуры.
Про обратную сторону — это может быть проблемой. Именно поэтому в статье написано «Слоёв нужно столько, сколько нужно. Для простого CRUD — может, и одного достаточно.» Если алгоритм линейный и тестов нет — вынос в 10 классов сделает только хуже. Но когда появляются тесты или меняется инфраструктура (другой банк, другой ORM) — цена «всё в одном месте» резко растёт.
По поводу «вынести в приватные методы» — если вынести switch в приватный метод того же класса, класс всё равно зависит от
dbContext,externalApiи_logService. Тестировать бизнес-правило отдельно от инфраструктуры не получится — моки тех же 5 зависимостей. Суть не в том, чтобы код стал короче, а в том, чтобы бизнес-логика не зависела от инфраструктуры.
Ну хорошо можно вынести switch в статичный метод и тогда зависимости и мокинг перестают быть проблемой.
Про обратную сторону — это может быть проблемой. Именно поэтому в статье написано «Слоёв нужно столько, сколько нужно. Для простого CRUD — может, и одного достаточно.» Если алгоритм линейный и тестов нет — вынос в 10 классов сделает только хуже. Но когда появляются тесты или меняется инфраструктура (другой банк, другой ORM) — цена «всё в одном месте» резко растёт.
"Слоёв нужно столько, сколько нужно" да вот только как понять сколько нужно? Это как раз фундаментальная проблема всех архитектур, когда люди начинающие проект думают что их слишком много или слишком мало а потом оказывается что наоборот, тут просело а там наоборот вылезло. И для архитектуры это выглядит как совет "не делай плохо, делай хорошо". Я к тому что что такие советы слишком абстрактны и очень вероятно что человек прочитавший такой совет не сможет применить его в своем проекте потому что ту грань о которой Вы говорите определить очень сложно.
Ну хорошо можно вынести switch в статичный метод и тогда зависимости и мокинг перестают быть проблемой.
Со статическим методом — для чистого switch это сработает, но это процедурный подход. В ООП логичнее инкапсулировать логику в сам объект. Логика принятия решения живёт в самом объекте решения — ни зависимостей, ни моков, ни статических утилит.
“Слоёв нужно столько, сколько нужно” да вот только как понять сколько нужно?
Про «сколько слоёв нужно» — для большинства задач классических трёх слоёв из Clean Architecture хватает. Если становится тесно — стоит делить слой на фичи (vertical slices), а не добавлять новые слои. Да если весь сервис — это «взять из базы, вернуть клиенту» — одного слоя достаточно.
Кажется, это можно сформулировать проще: каждая функция должна работать с одним объектом. Если нужно работать с еще одним объектом, вместо этого нужно вызывать соответствующую функцию.
И с практической точки зрения - если код вашей функции занимает больше экрана, вероятно, что она плохо декомпозирована.
Я бы добавил 4-ю: на каждый чих выкидывать исключение вместо возвращения значения.
Например, если DI настроен нормально, то строки типа _imageService = imageService ?? throw new ArgumentNullException(nameof(imageService));
не имеют смысла. Там всегда не null.
А когда исключения лезут в бизнес-логику, то из-за этого обычно код обрастает множеством try-catch на всех уровнях и никогда не знаешь, где именно это исключение будет перехвачено и как обработано. Более того, оно ещё по-разному может обрабатываться в разных местах, что приводит к созданию отдельных исключений и их обработке для какого-то отдельного случая. В одном проекте я со временем удалил с десяток таких одноразовых исключений.
Сейчас один сервис рефакторил. Так там чуть ли не каждый метод обёрнут в исключения, большинство из которых никогда не сработают. Даже вокруг такого кода, который никогда не выбросит исключения.
Если очень надо, а рефакторить такой код дорого, можно сделать PublicException, который будет показывать пользователю ошибку как есть. А всё остальное всё равно должно быть обработано. Желательно в одном месте (ExceptionHandlerMiddleware). Исключение должно сообщать о проблеме в коде/логике, а не быть способом передачи информации.
Тот же необработанный ArgumentException сообщает пользователю название параметра с неверными данными. Зачем это надо? (вопрос риторический)
Например, если DI настроен нормально, то строки типа _imageService = imageService ?? throw new ArgumentNullException(nameof(imageService)); не имеют смысла. Там всегда не null.
Про ArgumentNullException в конструкторах — если следовать принципу, что слой приложения не знает об инфраструктуре, то он не может полагаться на то, что DI-контейнер всё правильно зарегистрировал. Проверка на null — это контракт метода, а не дублирование работы DI. К тому же сервисы покрываются юнит-тестами, где DI нет — и эти проверки защищают от неожиданного поведения в тестах.
А когда исключения лезут в бизнес-логику, то из-за этого обычно код обрастает множеством try-catch на всех уровнях и никогда не знаешь, где именно это исключение будет перехвачено и как обработано. Более того, оно ещё по-разному может обрабатываться в разных местах, что приводит к созданию отдельных исключений и их обработке для какого-то отдельного случая. В одном проекте я со временем удалил с десяток таких одноразовых исключений.
Про исключения vs Result-паттерн — тема холиварная. Мой подход: исключения — нормальный механизм, но они не должны перехватываться на каждом уровне. Бизнес-исключение (OrderNotFoundException, StatusNotAllowedException) должно долетать до единого обработчика (middleware), который формирует ответ клиенту. Если вместо этого на каждом слое try-catch с разной логикой — тогда да, проблема. Но это проблема не самих исключений, а их неправильной обработки.
Если я пишу какой-то сервис. для API, то я уже знаю о работе DI и что он мне вернёт. Нет смысла придумывать, что там что-то внезапно поменяется и из-за этого засорять конструкторы и тесты кодом, который никогда не будет выполнен и тестирует бесполезные вещи. Это может подойти для библиотеки, но не для класса внутри сервиса с известными параметрами работы. Иначе "на всякий случай" можно и код для .NET Core 3 версии написать, имея сервис на 10-й. Вдруг чего изменится.
Ещё добавлю про Result vs исключения: Result-паттерн хорошо работает в языках, где он встроен в систему типов — Rust, F#. Там компилятор заставляет обработать все варианты, и забыть не получится. В C# он добавляет церемонию: каждый вызывающий метод должен проверить result.IsSuccess, и если забыл — ошибка молча проглатывается. С исключениями наоборот — если не обработал, оно само долетит до middleware. В новом C# обещают discriminated unions, возможно с ними стоит пересмотреть подход.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
3 архитектурные ошибки в C#, из-за которых проект становится неуправляемым