Asp.net mvc core приложение. Правильность архитектуры и чистый код

Question

Я сделал свой первый pet-project и у меня возникло несколько вопросов в верности выбора архитектурного решения и best practice. (https://github.com/azaza12345/CsharpDapperExample)

1. Верна ли в принципе построенная мною архитектура? Контроллеры прокидывают информацию в сервисный слой, а те, в свою очередь работают с базой данных через репозитории?

2. Стоит ли нагромождать условиями и проверками service слой? Например, присваивать значение в конструкторе так: _productRepository = productRepository ?? throw new ArgumentNullException();? Или проверять на null входящий в метод параметр?

3. Подсмотрел в других проектах использование интерфейса, например, IProductService для ProductService и не совсем разобрался для чего это нужно. По сути же этот интерфейс жёстко привязан именно к этому сервису не предполагает других его реализаций..? в чём я не прав? нормальной ли практикой будет просто регистрировать службу ProductService без интерфейса?

4. Следует ли разбивать класс миграций на несколько или можно создавать все нужные таблицы в одном?

5. Также буду рад любым найденным замечаниям и неточностям в моём проекте на основе best practice (это касается как целой архитектуры приложения, так и различных именований переменных/методов, clean code)

Answer 1

Вы хотели разбора вашего проекта, так что считайте сами напросились :)

1. Начем с названия. Название вашего проекта показывает язык/библиотеки, а не его назначение. Вы вот в "WPF_C#_C++" программе его писали или в "IDE Visual Studio"? Я, как человек, что делает ревью вашего приложения, первым делом вижу его название и оно ни о чем вообще не говорит. Называйте проект по его назначению, технологии перечислите уже в readme если в этои есть надобность. (см Naming Guidelines)

2. У вас часть контроллеров возвращает модели, а часть - view models - вы уже определитесь, есть у вас слой VM или нет.

3. Смотрю на котроллеры и не понимаю, вы как то заботитесь о HTTP кодах, что возвращаете? Почему для каких то методов вы используете ValidateAntiForgeryToken, а для каких то нет?

4. Ваши сервисы возвращают view models, вы точно знаете назначение сервисов? Почему сервисы знают о том, как информация будет отображаться? (см PoEAA)

5. У вас авторизация включена, но я не вижу, где вы её используете?

6. Я так понимаю, вы полностью отказались от логгирования? Вам интересно, что с вашим приложением делают юзеры? Как вы узнаете о деталях проблемы, если она возникнет?

7. SessionExtensions.GetItemsListFromSession<ShoppingCart>(_httpContextAccessor, WebConstants.SessionCart) - вы знаете, как пользоваться extension методами?

8. Вы храните корзину в сессии для неавторизованного юзера, к чему вы будете заказ привязывать? Попросите юзера авторизоваться?

9. Почему у вас HomeService отвечает за операции с корзиной? У вас же есть CartService? В чем вообще назначение HomeService? Зачем он нужен? Сервисы создаются для доменных объектов, а не для страничек в интерфейсе.

10. С одной стороны у вас многослойная архитектура, а с другой стороны - вы позволяете себе вот это var count = SessionExtensions.GetItemsListFromSession<ShoppingCart>(_httpContextAccessor, WebConstants.SessionCart).Count; прямо в коде view. Будьте консистентны.

11. Убирайте закомментированные участки кода. Если вы не закончили задачу, держите её в другой ветке git

12. Тесты. UnitTestsApp как название проекта с тестами. Та же самая претензия как 1.

13. Открываю CartServiceTest.cs и не понимаю, что он тестирует.

14. Assert.Equal(GetProducts().ElementAt(i).Name, result.ElementAt(i).Name); используйте методы, что сами проверяют коллекции на равенство.

15. Отсутствие тестов контроллеров. Вы уверены, что тестов для сервисов достаточно?

16. Readme - у вашего проекта нет названия, а его назначение стоит после списка технологий. Сначала назовите проект как то. Потом опишите что проект делает. Потом его назначение (например, создан как пример работы с платформой MVC чтобы прокачать скиллы), потом уже пишите про технологии и как проект запустить.

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

Ответы на ваши вопросы

Верна ли в принципе построенная мною архитектура?

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

Стоит ли нагромождать условиями и проверками service слой?

Нет, не стоит. Если параметр будет передан как null - код должен упасть при попытке вызова метода этого параметра.

нормальной ли практикой будет просто регистрировать службу ProductService

В pet проекте, которые априори небольшой, нормально регистрировать классы. В больших проектах как это делается - читайте про букву D из SOLID

Следует ли разбивать класс миграций на несколько или можно создавать все нужные таблицы в одном?

Если я верно помню (я на дотнете уже года 2 не пишу), то каждая миграция накатывается со своей транзакцией. Вот и думайте, как вам надо, чтобы ваша БД либо вся накатилась или вся не накатилась, либо использовать кучу миграций и иметь полу-накатаную БД. С точки зрения орагнизации кода разницы никакой абсолютно. Со смысловой точки каждая миграция - это законченный функционал. Если вы накатили миграции, но последняя не накатилась и ваш проект не может запуститься, ваша БД останется в неконсистентном состоянии.