Комментарии 28
За Пуха плюсую, в целом по делу, но только когда пишем как не надо, лучше приписывать как надо! А то не совсем качественное ревью выходит).
Собственно, а зачем контроллеру надо быть тонким? Статья ставит это в аксиому непонятно почему.
Или это перевод какой-то статьи и спрашивать автора нет смысла?
Я встречал на одной из работ тимлида, который придерживался очень похожих пунктов, хотя проект был на другом языке.
Возможно, где-то сформулировали такие правила рефакторинга, а потом они пошли в народ.
а зачем контроллеру надо быть тонким?
У этого довольно простые цели:
- сокращение длины методов,
- стандартизация выполняемых действий (удобно разделить контроллер на получение/валидацию данных, обработку через сервис и выдачу через DTO и сериализатор — это повышает вероятность того что важные этапы вроде валидации не будут пропущены),
- уменьшение общего объёма кода (к примеру, такие сущности как DTO нередко могут быть общими у разных контроллеров).
Не всегда так. На мой взгляд нет ни чего плохого в определенных не сложных действиях на контроллере. Это может быть к примеру маршутизация и многое другое.
Добавлю еще один кейс: тестируемость.
При применении good practices, описанных в посте, у вас в методе контроллера обычно будет вызов сервиса, содержащего бизнес логику. Сам сервис имеет явные зависимости, и юнит тесты на бизнес логику будут написаны на него.
Если же роль сервиса будет выполнять сам контроллер, то по помимо тестов на обработку невалидного состояния модели, на различные типы ответов (с разными HTTP кодами, например), которые как раз относятся к ответственности контроллера, будет необходимость так же писать тесты на бизнес логику.
Да, это — перевод, подробности см. мой комментарий ниже.
Да, вы правы — это перевод, несмотря на это в комментариях уже написали множество хороших ответов: сокращение методов, стандартизация, тестирование, и так далее.
Самое главное, на мой взгляд, в том, что у контроллера должна быть только одна ответственность — работа с запросами на HTTP уровне. То есть, получить HTTP запрос и сделать вызов кода на обработку запроса, а также выбрать правильный HTTP код и заголовки для ответа в зависимости от результатов обработки.
Самое главное, на мой взгляд, в том, что у контроллера должна быть только одна ответственность — работа с запросами на HTTP уровне. То есть, получить HTTP запрос и сделать вызов кода на обработку запроса, а также выбрать правильный HTTP код и заголовки для ответа в зависимости от результатов обработки.
НЛО прилетело и опубликовало эту надпись здесь
Кому интересно — вот ссылка на оригинал на английском языке, 2020 год публикации. А это — перевод.
И (для порядку поинтересуюсь) если правильно понял, автор оргинала и автор перевода — разные люди, так?
И (для порядку поинтересуюсь) если правильно понял, автор оргинала и автор перевода — разные люди, так?
Да, вы абсолютно правы. К сожалению, пока статья проходила ревью у модераторов у неё куда-то слетела метка, что это перевод. Хорошо хоть я в конце написал
Перевод статьи подготовлен в преддверии старта курса «C# ASP.NET Core разработчик».но это в разы сложнее заметить, чем стандартную плашку о переводе и ссылке на оригинал. Спасибо вам большое, что заметили, вернул метку на место.
По существу вроде все правильно, но вот метафора шеф-повара выглядит странно. Какой же он шеф, если его функция — принять вызов и передать его ядру с бизнес-логикой? Больше бы подошел образ девушки на ресепшене.
Если вы проверяете свойства объекта User внутри контроллера, чтобы разрешить/запретить ему что-то, то, кажется, что у вас есть кое-что для рефакторинга.
Скажем так- зависит от случая.
Есть такая авторизация называется «на основе ресурсов» resource base auth, так вот там Microsoft рекомендует проводить ее в методах контроллера.
docs.microsoft.com/ru-ru/aspnet/core/security/authorization/resourcebased?view=aspnetcore-5.0
Отличное дополнение, спасибо большое, alexs0ff.
Эх… Капитанские декларации без объявления мотивации.
Чем плохи «толстые» контроллеры: копипаста, сложно тестировать и покрывать юнит-тестами, сложно рефакторить, сложно развивать.
Шеф-повар, конечно, плохая аналогия, потому как он — бизнес-сервис, а не контроллер.
Контроллеры — это публичное апи вашего веб-приложения.
Если уж искать аналогию — то возьмите офис банка. Там очень наглядные контроллеры, например: отдел потребительских кредитов, в котором есть методы «оформить кредит», «узнать остаток», «внести платёж» и т.д. А ещё часто есть улыбчивая девушка при входе, которая отвечает за роутинг. И при этом никто в современном банке вам лично кредит не выдаст. Операционист примет от вас ввод, т.е. запросит нужные данные, и вернёт ответ системы. Всё остальное делается делается другими службами и бизнес-сервисами.
Чем плохи «толстые» контроллеры: копипаста, сложно тестировать и покрывать юнит-тестами, сложно рефакторить, сложно развивать.
Шеф-повар, конечно, плохая аналогия, потому как он — бизнес-сервис, а не контроллер.
Контроллеры — это публичное апи вашего веб-приложения.
Если уж искать аналогию — то возьмите офис банка. Там очень наглядные контроллеры, например: отдел потребительских кредитов, в котором есть методы «оформить кредит», «узнать остаток», «внести платёж» и т.д. А ещё часто есть улыбчивая девушка при входе, которая отвечает за роутинг. И при этом никто в современном банке вам лично кредит не выдаст. Операционист примет от вас ввод, т.е. запросит нужные данные, и вернёт ответ системы. Всё остальное делается делается другими службами и бизнес-сервисами.
Почему они должны быть тонкими? — ни слова.
Какой в этом плюс? — ни слова.
Как сделать их тонкими, если они сейчас не такие? — ни слова.
Как сохранить их тонкими? — ни слова.
О чем заметка? Просто сообщить о нескольких проблемах?
Лично я зашел почитать ответы на те 4 вопроса которые Вы задали в начале. Но их почему-то нет.
Какой в этом плюс? — ни слова.
Как сделать их тонкими, если они сейчас не такие? — ни слова.
Как сохранить их тонкими? — ни слова.
О чем заметка? Просто сообщить о нескольких проблемах?
Лично я зашел почитать ответы на те 4 вопроса которые Вы задали в начале. Но их почему-то нет.
О чем заметка?
"Всех, кто желает подробнее узнать о курсе и программе обучения, приглашаю записаться на день открытых дверей, который я проведу уже 5 мая."
Да, то, что статья переводная и автор оригинальной статьи пишет
Ответим на «Почему они должны быть тонкими?» и «Какой в этом плюс?».
Как уже правильно написали Rive, donotcodeit и Mikluho, плюсы тонких контроллеров в их читаемости, тестируемости, простоте поддержки и переиспользовании кода, вынесенного в сервис, вместо копипасты в разных контроллерах. От себя дополню список тем, что у контроллера должна быть только одна ответственность — работа с запросами на HTTP уровне. То есть, получить HTTP запрос и сделать вызов кода на обработку запроса, а также выбрать правильный HTTP код и заголовки для ответа в зависимости от результатов обработки.
Ответами же на вопросы «Как сделать их тонкими, если они сейчас не такие?» и «Как сохранить их тонкими?» как раз и занимается текущая статья — не нужно допускать лишний код в контроллеры, а если он там есть, то выносить.
Правильные (и частые) вопросы. Обсуждение части этих вопросов можно найти в моих ранних статьях, поэтому сейчас мы посмотрим на проблему с другой стороны.не умаляет проблемы с тем, что ответов на них нет. Давайте исправим это недоразумение.
Ответим на «Почему они должны быть тонкими?» и «Какой в этом плюс?».
Как уже правильно написали Rive, donotcodeit и Mikluho, плюсы тонких контроллеров в их читаемости, тестируемости, простоте поддержки и переиспользовании кода, вынесенного в сервис, вместо копипасты в разных контроллерах. От себя дополню список тем, что у контроллера должна быть только одна ответственность — работа с запросами на HTTP уровне. То есть, получить HTTP запрос и сделать вызов кода на обработку запроса, а также выбрать правильный HTTP код и заголовки для ответа в зависимости от результатов обработки.
Ответами же на вопросы «Как сделать их тонкими, если они сейчас не такие?» и «Как сохранить их тонкими?» как раз и занимается текущая статья — не нужно допускать лишний код в контроллеры, а если он там есть, то выносить.
Причины по которым нужно делать контроллер тонким:
- Правило — валидация должна быть только в сервисе бизнес логики. Валидация должна быть расположена в одном месте и выполняться один раз. Зачастую валидацию делают единажды в контроллере/фильтре и пропускают в сервисе бизнес логики, что чревато отсутствием валидации при вызове сервиса бизнес логики напрямую. Бывает, что валидацию дублируют в фильтре/контроллере и в сервисе бизнес логики, что избыточно.
- Юнит тестирование должно покрывать сервис бизнес логики, включая валидацию. Это уменьшит количество и упростит автоматизированные тесты для контроллера. По большому счету автоматизированные тесты будут сфокусированы более на HTTP уровнь.
Правило — юнит тесты должны покрывать всю бизнес логику - гибкость при подходе с тонким контроллером — все сервисы и логика может быть вынесена в отдельную бибилиотеку и переиспользована.
Правило — бизнес логику (включая валидацию, маппинг) можно и нужно вынести в отдельную библиотеку и переиспользовать по необходимости (в тестировании например) - централизация обработки ошибок — это мимимум, который должен быть всегда. Так будет легче тестировать обработку ошибок. Код в одном месте — читаемость выше.
Правило — Не нужно писать try/catch в каждом методе контроллера (это делается крайне редко). Используй централизованный обработчик ошибок
Золотое правило разработки — не переусложнять.
То, что вы описали, безусловно правильно. И целевая схема на крупном проекте с большой командой именно такая. Однако в реальной жизни нужно понимать еще вот какие факторы:
1) Плохо написанный завершенный проект лучше любого хорошо пишущегося, но никогда не завершенного проекта. Увязнув бест практайс и клин архитекчур вы рискуете никогда не выпустить даже прототип.
2) Нужно понимать, что часто ДТО нельзя никак переиспользовать, потому что они решают именно определенную бизнес логику. Конечно, ради трех полей мы можем подключить автомапер и определить конфигурацию (и да, так будет правильно), но мы будем тратить больше времени
3) Валидация DTO и валидация бизнес-логики — это две существенно разные вещи. Например, вы создаете аккаунт и пользователь выбирает свободное имя больше 6 символов. Длина должна проверяться на DTO, в то время как логика проверки уникальности — это уже бизнес-логика. Или например перевод денег. Проверить что сумма больше/меньше пороговых значений — это интерфейсная часть. Проверка реального баланса перед транзакцией — бизнес логика. Это я к тому, что if (!ModelState.IsValid) вполне себе жизнеспособен на контроллере.
4) Любители чистой архитектуры вообще склонны к ненужному переусложнению. Например взять и обернуть DbSetв репозиторий. А над репозиторием бахнуть еще и сервис DAL (ну там запросы переиспользовать). И в результате получить fizzbuzz enterprise edition
github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition
То, что вы описали, безусловно правильно. И целевая схема на крупном проекте с большой командой именно такая. Однако в реальной жизни нужно понимать еще вот какие факторы:
1) Плохо написанный завершенный проект лучше любого хорошо пишущегося, но никогда не завершенного проекта. Увязнув бест практайс и клин архитекчур вы рискуете никогда не выпустить даже прототип.
2) Нужно понимать, что часто ДТО нельзя никак переиспользовать, потому что они решают именно определенную бизнес логику. Конечно, ради трех полей мы можем подключить автомапер и определить конфигурацию (и да, так будет правильно), но мы будем тратить больше времени
3) Валидация DTO и валидация бизнес-логики — это две существенно разные вещи. Например, вы создаете аккаунт и пользователь выбирает свободное имя больше 6 символов. Длина должна проверяться на DTO, в то время как логика проверки уникальности — это уже бизнес-логика. Или например перевод денег. Проверить что сумма больше/меньше пороговых значений — это интерфейсная часть. Проверка реального баланса перед транзакцией — бизнес логика. Это я к тому, что if (!ModelState.IsValid) вполне себе жизнеспособен на контроллере.
4) Любители чистой архитектуры вообще склонны к ненужному переусложнению. Например взять и обернуть DbSetв репозиторий. А над репозиторием бахнуть еще и сервис DAL (ну там запросы переиспользовать). И в результате получить fizzbuzz enterprise edition
github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition
1) все зависит от сроков сдачи проекта — тут спору нет. Но я не вижу проблемы вынести в отдельную библиотеку бизнес логику и оставить контроллер чистым сразу. Не думаю, что разраб съэкономит много времени, если будет сразу в контроллер писать. А приучить себя к чистому коду лучше сразу (в разумных приделах срока проекта).
2) +
3)
Проверить что сумма больше/меньше пороговых значений — это интерфейсная часть.
if (!ModelState.IsValid) вполне себе жизнеспособен на контроллере
да, если такая же валидация (или расширенная) продублирована в сервисе логики, иначе прямой вызов этого сервиса может приводить к проблеме из-за отсутсвия части валидации.
4) абстаркции ради абстракций делать не нужно. За чистый контроллер Вам скажут все спасибо в будущем. Еще ни разу не видел восторга от коллег, когда они встречали код напрямую в контролле.
Не очень понял пункт авторизация — это как понимаю проверка прав доступа? Или же определение кто вошел в систему? Последнее кажется лучше делать именно в контроллере
и то и другое лучше делать прозрачно, например на атрибутах + middleware.
В мидлтвари такое делать надо…
Общепринятое назание для «определение кто вошел в систему» — аутентификация. Авторизация — это именно проверка разрешения на доступ. И эти действия могут быть разнесены по разным компонентам.
В контроллере лучше не заниматься ни тем, не другим, хотя бы по соображениям безопасности: обычно прикладной программист не является экспертом в области безопасности, поэтому при самостоятельно реализации он слишком легко в этой области может что-нибудь упустить.
Благо уж где-где, а в ASP.NET MVC механизмы аутентификации и авторизации уже реализованы разработчиками фреймворка, и в большинстве случаев остается только ими воспользоваться.
Кроме того, обычно аутентификация с авторизацией используются единообразно для всего приложения (или его существеной части), то есть — являются общим для приложения кодом, а потому им место — не в контроллерах, обрабатывающих конкретные запросы: код все же лучше не дублировать.
В контроллере лучше не заниматься ни тем, не другим, хотя бы по соображениям безопасности: обычно прикладной программист не является экспертом в области безопасности, поэтому при самостоятельно реализации он слишком легко в этой области может что-нибудь упустить.
Благо уж где-где, а в ASP.NET MVC механизмы аутентификации и авторизации уже реализованы разработчиками фреймворка, и в большинстве случаев остается только ими воспользоваться.
Кроме того, обычно аутентификация с авторизацией используются единообразно для всего приложения (или его существеной части), то есть — являются общим для приложения кодом, а потому им место — не в контроллерах, обрабатывающих конкретные запросы: код все же лучше не дублировать.
Больше абстракций богу абстракций. Больше бойлерплейта — шоб посолиднее проект выглядел
А потом сидят и смотрят на стек вызовов из 100+ Гадят в память и жрут ресурсы CPU… А девопс накидывает всё новые сервера…
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
6 вещей, которые не стоит делать в ASP.NET контроллерах