Pull to refresh
12
0

Software Developer

Send message
Не статья, а сборник вредных советов.
  1. Зачем вызывать Context.SaveChanges(); в методах BaseRepository? Как вы откатите Worker и Car, если упал Document.Create?
  2. services.AddTransient<IBaseRepository<*>, BaseRepository<*>>(); Зачем, если DbContext — Scoped по умолчанию?
  3. Зачем использовать JsonResut, если есть Ok(), Created(), BadRequest() и т.д.?
  4. При поиске по первичному ключу желательно использовать Find, а не FirstOrDefault
  5. Очень странное решение инжектить репозитории в сервисе через свойства, а не конструктор
  6. В Delete два раза загружается entity
  7. Возвращать из контроллера entity — плохая идея
  8. В тесте — это что вообще такое? )))
    serviceMock.Object.Work(); 
    
    serviceMock.Verify(x => x.Work());
Наверное, мы друг друга недопоняли.
В статье обсуждаются способы прокидывания детализированной ошибки из метода в вызывающий код. Maybe для этого не предназначен, а Either — очень даже.
И учитывать вызывающий код в проблематике статьи — явно не стоит. Потому что можно проигнорировать как Maybe.None, так и Either.Left.
Устные конвенции и договоренности — такая себе штука, рано или поздно кто то ее нарушит, а компилятор такое отлавливать сейчас не может.
В том то и суть, если мы вызываем некий метод
Maybe<User> FindUser(string id)
то метод либо возвратит нам пользователя, либо не возвратит (и только), причем результат выполнения будет предельно однозначным.
Считать это ошибкой или нет — дело того кода, который вызывает этот метод.
Инфраструктурные ошибки, типа отвалившейся БД — тут не в счет, дело только в логике метода
Окей, я вас понял.
Но нужно было явно указать, что:
— есть такая монада Either и объяснить ее назначение.
— не стоило Either делать на основе Maybe.
В целом, как сравнение подходов — статья неплохая, но, как всегда, есть нюанс. Опытные разработчики и так знают про монады, nullable reference types, try pattern и т.д.
А начинающие разработчики, на которых и ориентирована этат статья, узнают про Maybe (что очень хорошо), но не узнают про Either (плохо, но не очень — узнают позже). А еще увидят вашу реализацию с Result поверх Maybe (и начнут использовать в своем коде)- а вот так уже делать не стоит. Монады используют в «чистом» виде. Расширять их extension'ами — пожалуйста, но не наследоваться от них
Если идеоматичность поддерживается во всей кодовой базе — вполне норм.
Но если это библиотека для стороннего пользователя (или общая библиотека для нескольких сервисов) — то nullable reference types не подходит, потому что сторонние пользователи:
— могут забить на идеоматичность.
— библиотека может (и должна, если в ней нет чего то прям такого, что требует netstandard2.1) поддерживать старую версию фреймворка без поддержки nullable reference types.
Вот тут то и нужна Maybe/Optional именно как структура.

Я больше про контекст статьи, изобретать Either поверх Maybe — странное решение, когда все уже давно придумано, и это отдельные, не зависящие друг от друга монады
Про то и говорится, что автор зачем то переизобрел Either, приплетя сюда Maybe.
При том, что Maybe — самодостаточная монада и не нуждается ни в каком наследовании, а Either такая же самодостаточная монада, никак не связанная с Maybe.
Я уже молчу, что в контексте C# Maybe должна быть структурой, а не классом.

PS. Случайно минуснул коммент, а отменить нельзя
Мне немного стыдно, когда я вижу код минимально рабочей программы на C#
Скоро в C# 9:
using System;

Console.WriteLine("Hello World!");
А зачем ITodoRepository регистрировать как Transient?
Конечно, await делает что то иное
Накладные расходы у вас обязательно будут, нужно только правильно их использовать.
Если у вас в async завернуты только простые CPU-Bound operations — лучше попробовать избежать асинхронности (в статье это наглядно продемонстрировано). Исключение — LongRunning tasks, но и тут есть свои нюансы (где их нет? ))).
Асинхронность дает очень заметный выигрыш при IO-Bound operations при множественных одновременных запросах. Там тоже есть накладные расходы, но в целом — утилизация ресурсов гораздо более качественная, чем при синхронном подходе.
А вот с одним условным пользователем пользователем вы даже не заметите разницы.
Про кеширование Task. Не скажу точно про js/ts (вероятно тоже самое), но в .net, если коротко:
Асинхронный метод мы совершенно спокойно (если результат не нужен прямо сейчас) можем вызвать без await.
Когда метод доходит до await Task (возвращенный из async метода, сразу или чуть позже), совсем необязательно начинают создаваться все вот эти асинхронные «усложнения». Если Task уже к этому моменту выполнен, то вызываемый метод завершается синхронно, потому что результат уже готов. Если не завершен — тогда вызывающий поток приостанавливает выполнение, вероятно уходит в пул, выполнение потом продолжается в другом (возможно) потоке и т.д.
Это если очень коротко и очень грубо.
Я думаю, вы уже поняли, почему нужно кешировать именно Task, а не документ, как результат выполнения :)
Результат из Task/Promise, если он уже выполнен, можно получать многократно, создавать каждый раз новый Task с результатом — совершенно необязательно.
Конкретно в .net недавно добавили еще и ValueTask, который это все вообще максимально упрощает, но там тоже есть свои тонкости.
Скажем так, сам принцип асинхронности — практически одинаков для всех языков, его поддерживающих «из коробки». Для начала вам нужно разобраться, какая именно у вас проблема, ее корень.
Правильные практики применения асинхронности — это не просто какое то там FAQ а-ля «делайте так и так». Это годится в 95% случаев, но вот в 5% нужно погрузиться в эту тему достаточно глубоко, чтоб не делать такие тесты производительности, как в вашей статье.

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

Из-за этого ваша статья очень спорная с практической точки зрения, потому что неокрепшие джуны (и не только) могут подумать, что асинхронность — это зло и ее стоит избегать. А ведь это достаточно сложная штука (не условный синглтон), которая при правильном приеменении приностит очень много пользы, но и в ногу стрельнуть позволяет с легкостью. Вы вот стрельнули ))
Вам уже оветили выше — кешировать нужно не документы, а Task/Promise на эти документы.
На educative.io есть прекрасный курс C# Concurrency for Senior Engineering Interviews. В курсе очень много общей теории, не касающейся C#, своих денег он однозначно стоит. Прочитав его и, самое важное — поняв, что такое асинхронность, для чего она нужна и как работает, вы научитесь не то что решать проблемы из вашей статьи, а даже не приближаться к ним в принципе
В своем проекте, в котором я стараюсь использовать DDD и Clean Architecture на полную катушку, и с опыта которого я написал статью про ValueObjects в EF Core, слой Persistence выполняет строго отведенную ему роль — обеспечивает персистентность сущностей. Другими словами — сохраняет и загружает требуемые обьекты/коллекции.
Никакой бизнес-логики, манипуляций данными и т.д. там нет — все это в отдельном слое.
Если я хочу, условно, сделать заказ, то бизнес логика сначала загружает контрагента через var partner = _partners.WithId(id), если не найден — кидает специальное исключение, потом формирует заказ примерно как var order = new Order(partner), потом сохраняет его _orders.Add(order).
В итоге репозитории — обычно простые, как угол дома, потому что методы выполняют строго отведенную им задачу.
Бизнес-логика в RequestHandler'ах точно так же наглядна и понятна.
И то, и то просто и понятно тестируется — бизнес-логика мокает методы репозиториев и других зависимостей, а сами репозитории тестируются уже с помощью InMemory провайдера. Если бы мне были критичны ограничения InMemory — я бы использовал SQLite

Это буква S из аббревиатуры SOLID — Single responsibility. Не должен репозиторий что то там проверять и вычислять — не его это задача.
я строю тесты последовательно, и сразу вижу какой упал — если он упал
А потом нужно модифицировать логику, и вы полдня будете искать, между какими тестами нужно вставить новый тест. Так нельзя.
прямая зависимость теста от состояния — это преимущество, а не недостаток.
Это недостаток, а не преймущество. Окружение теста должно настраиваться в самом тесте, а не «где-то там». Только так видна полная картина теста и что именно он тестирует.
как вы добавите права несуществующему пользователю?
Это бизнес-логика, и ей не место в DAL
как осуществить продажу, если нет контрагента?
Это бизнес-логика, и ей не место в DAL
вызываем функции по очереди и становится понятна иерархия вызовов
И это тоже бизнес-логика, которой не место в DAL.
смотреть что будет если пользователь введёт те или иные данные
валидация данных — это вообще Presentation Layer, а проверка бизнес-требований — это задача для слоя бизнес-логики
тест на консистентность базы данных
Вас вообще это не должно волновать. В случае RDB и правильной схемы консистентность вам гарантируется. А если приложение падает из-за того, что в какой то момент не был найден контрагент — то это явный баг в бизнес-логике. Тесты консистентности БД тут не помогут.

В общем, вы запихнули кучу логики в DAL, которой там не должно быть в принципе, пытаетесь это как то проверить и городите костыли. В целом, хоть какое то тестирование — это хорошо, но вы натягиваете свои тесты на явный архитектурный просчет.
Задача DAL проста — обеспечить персистентность данных. Это не место для логики.
Вы нарушаете одно из правил тестов — они должны быть независимы друг от друга.

Ок, если у вас в DAL 2 метода — «Добавить товар» и «Получить товар», то это 2 совершенно независимых метода, на которые пишуться совершенно разные и независимые тесты.
Первый тест — на метод «Добавить товар». Проверяется только то, что тавар добавился. Все.
Вторая группа тестов — на «Получить товар». Тут количество тестов зависит только от количества сценариев этого метода.
Тесты из этой группы не должны зависеть от предыдущего выполнения метода «Добавить товар». Да, вам придется ручками в каждом тесте настаивать заполненность таблиц (почему бы это просто не вынести в отдельным метод и вызывать в каждом тесте в начале). И протестировать вам придется несколько ситуаций, одна из которых — есть ли товар в таблице. Товара нет — другой тест.
Метод «Получить товар» не должен опираться на то, был ли до этого где-то в бизнес логике вызван метод «Добавить товар». Это самостоятельная единица. Тестируйте бизнес-логику на то, что должно быть перед чем вызвано, отдельно.

Вы делаете критическую ошибку в том, что понадеялись на особенность фреймворка, который выполняет тесты «по алфавиту». Прийдет на проект синьер и совершенно обоснованно выкинет MsTest в помойку и прикрутит современный xUnit — а вот тут то и все упадет. Потому что тесты должны быть независимы друг от друга, поймите это. И настраиваться должны все по отдельности. А еще может прийти джун, даст новом тесту «неправильное» имя, из-за чего рухнет вся ваша цепочка вызовов. А еще есть всякие умные утилиты типа NCrunch, которые запускают только изменившиеся тесты, и в вашем случае они упадут даже будучи корректными, потому что перед ними не был вызван другой тест. Перезапускать весь проект каждый раз?

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

Кстати, если вы просто тестируете метод из DAL — это классический юнит тест. То, что вы прикрутили туда реальную СУБД и состояние — не позволяет назвать этот тест интеграционным.
Интеграционный тест — это когда некий бизнес-процесс из конечного множества действий тестируется с начала и до конца в реальных условиях. Вызов метода DAL — не бизнес-процесс, сколько бы там сложных запросов и связей ни было.
Я вам вот что пытаюсь донести:
  1. Если у вас сложная логика в методе из DAL — то создайте тестовое окружение конкретно для тестов этого метода, хотя бы даже так, как вы написали в статье. Не перекладывайте настройку окружения на тест контроллера, который использует этот метод. Что, если этот метод будут дергать несколько контроллеров? на каждый будете копипастить настройку?
  2. Если у вас сложная логика в методе из контроллера — то здесь нужно использовать только моки. Если вы всунете реальную БД в тест контроллера — то вот тут то и произойдет проникновение слоя DAL в тест и появится прямая зависимость теста от некоего внешнего состояния. А потом новый сотрудник что то изменит, и все-все тесты упали — будете очень долго разгребать, что и почему. А глядя на настроенные моки — все сразу станет понятно, какой тест упал и на каких настройках.
    Я уже писал, что тесты должны быть произведены на все сценарии. Если все настолько сложно и запутано — сколько вы делаете таких интеграционных тестов? Один с успешным сценарием? А ведь надо проверить все сценарии, будете для этого создавать множество заполнений таблиц? Поверьте, с моками все гораздо проще
Использовать MsTest для интеграционного тестирования с привлечением реальной СУБД конечно можно, но напоминает троллейбус из буханки хлеба.
базу данных не целесообразно тестировать в юнит-форме, потму что подготовка данных — это одна функция, а использование их — другая

Это отлично вписывается в паттерн AAA (Arrange-Act-Assert) для юнит тестов, где подготовка данных — arrange, использование — act, и, наконец, проверка валидности запроса — assert
Эти вопросы у вас возникают потому, что вы «из контроллера» пытаетесь «смотреть на логику» в DAL.
Вот в этом и кроется ошибка.
Контроллеру должно быть совершенно фиолетово, насколько сложен реальный запрос и сколько задействуется таблиц в методе _repository.GetSomeEntity(...). Пишется простейший мок вида:
var repositoryMock = new Mock<IRepository>();
repositoryMock
    .Setup(e => e.GetSomeEntity(...))
    .Returns(someEntity);

и забывается вся сложная логика из множества таблиц, связей и т.д на десятки строк в DAL.
Если в контроллере вызывается несколько методов репозитория — создаем несколько настроек на этот мок.
Если логика контроллера зависит от результатов вызова репозитория — пишем несколько юнит тестов.
Кто сказал что будет легко? Юнит тесты должны покрывать все возможные сценарии тестируемого метода, включая ошибки, разные результаты вызовов репозиторя, в том числе и цепочки вызовов. И при этом очень важно, чтоб они были изолированы друг от друга и не зависели от неких внешних БД, последовательности вызова тестов и т.д. Если делать только один тест на ожидаемый сценарий — то лучше вообще ничего не делать.

А вот для таких сложных и запутанных DAL методов, как вы привели в примере (пара часов и сложные зависимости таблиц) и пишутся отдельные юнит тесты, тестирующие конкретно эти методы, со своей настройкой подключения, заполнением таблиц, всеми возможными состояниями и т.д. Если в них что то нужно будет поменять — то изменятся только юнит тесты, покрывающие DAL. Все остальные потребители (контроллеры) остануться неизменными, потому что им все равно, что там происходит в DAL — им нужна SomeEntity, которая в тестах получается настройкой мока.
Запомните, логика DAL ни в коем случае не должна проникать в другие слои, в том числе и в тестах.

InMemory — это как пример очень быстрой конфигурации, работающий практически из коробки. Есть еще отличный SQLite. Хотите реальную БД — создавайте полноценное окружние с СУБД, эквивалентной боевой и там настраивайте полноценные интеграционные тесты.
инжектирование придётся настраивать в самих тестах, а по сути это будет имитация инжектирования ради самого процесса — контекст то один, и конфигурируется он в самом тесте

Именно это и есть самый настоящий юнит тест, в котором зависимости настроены на поведение, тестируемое в конкретном кейсе. Несколько вариаций поведения зависимостей (сущность найдена или не найдена) — пишем еще юнит тесты с измененной логикой зависимостей

Information

Rating
Does not participate
Location
Киев, Киевская обл., Украина
Registered
Activity