Обновить

Комментарии 20

Нюанс в том, что, видимо, он раньше не писал на C#

Простите, конечно, за негатив. Но после прочтения статьи у меня сложилось мнение, что вы тоже не очень-то хорошо знаете этот язык :(

Можете раскрыть почему?

Ну ок, по пунктам:

1. Аргументы: тут соглашусь, распарсить и провалидировать их отдельно — всегда плюс.

2. Функциональное программирование: код, нарезанный на функции, не становится ФП. У фп есть строгое (математическое) определение. Для описанного случая есть другой термин — процедурное программирование.

Для этого, конечно, пришлось подключить DI.

«Конечно», «пришлось» — сфига ли? Можно и без

3. Логи

Ну и? Что изменилось-то? Со стороны пользователя — примерно ничего, а то, что реализация была кривая, так это другой вопрос. Более того, новый логгер тоже имеет свои (другие) проблемы (left as an exercise for the reader)

4. Универсальные классы

Да, это беда. Но итог кажется как «из огня да в полымя»: Стало: 200+ файлов, где единицы размером по 300+ строк.

Если из 200+ файлов только единицы содержат какое-то осознанное количество кода, то значит появилось 190+ файлов, в которых код нарезан на карпаччо, где каждый класс и его метод просто передаёт ответственность дальше.

5. Репозитории — ок

6. Асинхронность — ок

7. Исключения — ок

8. Предупреждения — не поспоришь

9. Провайдеры — ничего не скажу, надо смотреть код, может быть овер-абстракцией, а может и очень полезным.

10. Структура файлов — да, dynamic зло, и если нет схемы, то придётся жевать dictionary

11. Методы в классах — в целом да, у меня есть претензия, но она не к автору, а в целом к текущему подходу к POD-классам, так что тут всё ок.

12. Помощники и расширения — шило на мыло, вкусовщина

13. Константы: > Это очень плохо: занимает лишнюю память
ШТА????!!!11 какую память и где?

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

14. Запросы к БД. Поля

А JOIN для кого сделан? В примере кошмар-какой SQL заменён на просто плохой SQL

15. Запросы к БД. Повторы

Если есть что-то очень константное, то оно должно быть или совсем константным, или его надо оставить на стороне SQL.

16. Запросы к БД. Существование

ШТОЯТОЛЬКОЧТОПРОЧИТАЛ???? Изучите хотя-бы одну книгу по работе с БД

17. Конкатенация строк — да, всё верно

18. Зависимости:

не вызывает проблем в функциональном подходе

в настоящем ФП как раз надо сильно постараться, чтобы получить циклические зависимости. Но как выше было написано, у вас тут не ФП

По поводу прокидывания исключений на самый верх тоже можно было бы поспорить.

Ну это хотя-бы лучше, чем использовать их для control-flow

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

Спасибо

2 Можно создавать объекты руками и передавать их параметрами, но зачем?

3. Новый логер легко использовать в тестах, универсальный, в случае необходимости можно легко поменять библиотеку для логов или добавить дру. Логи стали структурированными.

4. Они все содержат осознанное число строк в зависимости от внутренней логики. Часть из них действительно передаёт ответственность дальше, но это укладывается в логику и помогает лучше понять логику работы сервиса.

12. Да, и я описал, почему и что использую.

14. Конечно, я знаю про JOIN, но не всегда есть возможность это сделать. Например, данные берутся из разных баз. Я хотел показать, что запросы в цикле - это плохо.

16. Сколько записей обработает запрос по неиндексированному полю, чтобы посчитать количество? И сколько для получения первого попавшегося?

2: а можно не городить ООП и ынтерпрайз-архитектуру там, где она не нужна. Судя по тому, что вы пишете дальше, это какая-то тулза для обслуживания, а не LOB-система.

16: сделайте explain старого и нового запроса — узнаете.

Если поле не индексировано (а какого фига вы по нему тогда вообще ищете?), то фуллскан будет в любом случае, просто он завершится в среднем в 2 раза быстрее. Но эта разница не на порядки, и не O(log n) против O(n)

Обычная история, программист $имя1 унаследовал программу программиста $имя2. Благодаря подробности изложения, даже в основном понятно, что говорил бы $имя2, рефакторя обратно.

По поводу отношения к функциональщине не соглашусь.

>Сервис написан на C#, где всё — классы.

C# - мультипарадигменный язык, который сочетает классический ООП с различными функциональными фишками. То, что вы описываете - файлы по три тысячи строк, 15 аргументов в методе, обращение к БД и парсинг аргументов в середине бизнес-логики безусловно заслуживает всяческого порицания, но рефакторить это можно разными способами и ООП тут не панацея и даже не must-have.

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

Разбиение GOD-методов на маленькие, хорошо структурированные и хорошо читаемые классы? Хорошо, но ведь их можно было бы так же разбить на маленькие, хорошо структурированные и легко читаемые статические хелперы.

Дальше. Тестирование статики. It depends. Я понимаю, что тот код, который Вы описываете, где из каждого угла может происходить обращение к внешним зависимостям, покрыть юнит-тестами действительно нельзя. Но нет ничего проще для тестирования, чем чистая _статическая_ функция, которая имеет только параметры на входе и данные на выходе и не обращается к инфраструктурным зависимостям. Тогда блок // Arrange - это формирование параметров, // Assert - проверка ответа. И никакой возни с моками и с "удобными" библиотеками мокирования (есть исключения, но не буду вдаваться в подробности). Для того, чтобы такой подход возможно было применить, надо выносить всю сложную бизнес-логику, ориентированную на данные, в функциональное ядро, а возню с инфраструктурными зависимостями оставлять сервисам. Статику - юнит-тестам, сервисы - интеграционным. Т.е., здесь опять есть решение не в чистом ООП, а на стыке двух парадигм и я считаю его оптимальным, потому что вынос функционального ядра и покрытие тестами именно этого ядра делает тесты более устойчивыми к дальнейшему рефакторингу. Мне за последние пару лет пришлось в этом много раз убедиться.

А так спасибо за статью, многие описанные вещи до боли знакомы.

Если бы там были чистые функции. Там от функционального подхода было лишь использование статических методов повсеместно. Ну, и хотелось предсказуемые для C# тесты написать, а не что-то на стыке. И, всё-таки, не правильно в рамках одной компании делать разные сервисы с очень разными подходами.

>В коде встретилось огромное количество повторяющихся строковых констант. Это очень плохо: занимает лишнюю память

Не должны повторяющиеся строковые константы занимать "лишнюю память" ибо string pool.

Да, верно.

Если речь именно о константах, то память не должны занимать вообще

Изучите, как работает CLR, удивитесь

Сталкивался с подобными задачами. Писать тесты надо ДО рефакторинга. Хотя бы интеграционные, рассматривая как black box.

До рефакторинга был большой взрыв. Я создал галактики. Чтобы писать сначала тесты, надо хотя бы представлять, как оно всё должно работать. Понимание пришло только после того, как я нарисовал схему взаимодействия всех сервисов.

Более того, пришлось даже добавить обёртку вокруг вызова  File.WriteAllTextAsync(...)

Есть же готовый NuGet-пакет System.IO.Abstractions как раз чтобы легко работать с файловой системой через интерфейс IFileSystem.

легко понять и прочитать

Сосед пробовал понять и прочитать?

А то может это эффект ИКЕА

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации