Зачем заниматься стандартизацией кодовой базы

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

Для решения этой проблемы можно проводить стандартизацию кода — определение и внедрение некоторых правил, унифицирующих решение распространённых задач.

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

Transcript

1. Зачем заниматься стандартизацией кодовой базы и Как нам помогут в этом линтеры Дмитрий Цепелев
2. @DmitryTsepelev DUMP’25 GitHub/Twitter: @dmitrytsepelev TG: @dmitry_tsepelev Блог: https:/ /dmitrytsepelev.dev 2
3. @DmitryTsepelev DUMP’25 План • что такое стандартизация кода и когда стоит начинать; • стандартизация и линтеры; • влияние стандартизации на разработку. 3
4. @DmitryTsepelev DUMP’25 Что такое стандартизация кода • набор соблюдаемых и проверяемых правил; • правила регулируют код в конкретном проекте; • например: • «публичные методы класса Repository должны возвращать relation (а не массив)»; • «не надо высылать письма из моделей»; • «не надо ходить в ENV напрямую». 4
5. Когда нужно начинать заниматься стандартизацией кодовой базы?
6. @DmitryTsepelev DUMP’25 Когда напрашиваются стандарты? 6 • SRP примитивов* не соблюдается/не определен; * примитив в данном случае — паттерн, компонент или что–то еще генерализуемое
7. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 7 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов А логику куда?
8. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 8 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Бизнес–логика
9. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 9 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Бизнес–логика
10. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 10 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов
11. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 11 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Service Бизнес–логика
12. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 12 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Service Бизнес–логика А у нас теперь сложные запросы
13. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 13 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Service Бизнес–логика Query
14. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 14 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Service Бизнес–логика Query А у нас валидации разные в разных контекстах!
15. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 15 Model Controller Service Query Form
16. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 16 Model Controller Service Query Form А теперь нужна система прав
17. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 17 Model Controller Service Query Form Policy
18. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 18 Model Controller Service Query Form Policy
19. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 19 Model Controller Service Query Form Policy • 🫠 все эти responsibility изначально были в наших двух примитивах; • 🤔 можем ли мы быть уверены, что все перенесли?
20. @DmitryTsepelev DUMP’25 Пример: разделение ответственности в Rails 20 Model Controller Запросы к БД Валидации Прием HTTP– запросов Подготовка HTTP– ответов Service Query Form Policy Проверка прав Сложные запросы Условные валидации Логика
21. @DmitryTsepelev DUMP’25 21 • SRP примитивов не соблюдается/не определен; • разные команды делают одно и то же по разному: • пример: проверка прав пользователя у одних в контроллерах, у других — в сервисах. Когда напрашиваются стандарты?
22. @DmitryTsepelev DUMP’25 22 • SRP примитивов не соблюдается/не определен; • разные команды делают одно и то же по разному; • разные команды делают одно и то же очень одинаково: • например: есть несколько классов для поиска/создания корзины. Когда напрашиваются стандарты?
23. @DmitryTsepelev DUMP’25 23 • SRP примитивов не соблюдается/не определен; • разные команды делают одно и то же по разному; • разные команды делают одно и то же очень одинаково; • многие вещи сделаны одинаково плохо: • например: часть тестов на контроллеры не проверяют сценарий запроса от неавторизованного пользователя. Когда напрашиваются стандарты?
24. @DmitryTsepelev DUMP’25 Почему сделано одинаково плохо? 24 • проще и быстрее скопировать и поменять код, чем написать с нуля; • если в кодовой базе много неудачных решений — их будут копировать чаще.
25. @DmitryTsepelev DUMP’25 25 • SRP примитивов не соблюдается/не определен; • разные команды делают одно и то же по разному; • разные команды делают одно и то же очень одинаково; • многие вещи сделаны одинаково плохо; • есть некие устные/письменные договоренности. Когда напрашиваются стандарты?
26. @DmitryTsepelev DUMP’25 Устные договоренности опасны! 26 • очень сложно понять, насколько они соблюдаются; • часть команды тратит время на их соблюдение; • часть команды тратит время на их проверку; • если они нарушаются — можно принять неверное решение. 🗿 Лучше вообще не тратить время 🗿
27. @DmitryTsepelev DUMP’25 Различие интерфейсов 27 class SomeService < BaseService def call if worked_f i ne? Success(some_object) else Failure(reason: some_reason) end end end class AnotherService < BaseService def call if worked_f i ne_too? some_object else { error: some_reason } end end end
28. @DmitryTsepelev DUMP’25 Различие интерфейсов 28 • проблема языков с динамической типизацией; • LSP нарушен; • мы не можем написать универсальные средства для работы со схожими объектами, так как у нас нет гарантий; • нужно как–то проверять единообразие. class BaseService # () - > Result<T> def call Success() end end
29. Что делать?
30. @DmitryTsepelev DUMP’25 🗺 Разобраться, что уже есть 30 • собрать все устные и письменные соглашения; • собрать данные по взаимодействию абстракций: • если есть статический анализ — можно попробовать использовать его; • для динамических языков можно попробовать собрать в рантайме или тестах.
31. @DmitryTsepelev DUMP’25 Помните Archunit? 31 @Test public void Services_should_only_be_accessed_by_Controllers() { JavaClasses importedClasses = new ClassFileImporter().importPackages("com.mycompany.myapp"); ArchRule myRule = classes() .that().resideInAPackage(" . . service . . ") .should().onlyBeAccessed().byAnyPackage(" . . controller . . ", " . . service . . "); myRule.check(importedClasses); }
32. @DmitryTsepelev DUMP’25 Как должно быть 32 • собрать список используемых паттернов/примитивов и описать их repsponsibility: • «все проверки прав пользователя — через Policy» • «контроллер только принимает HTTP–запрос и готовит ответ» • описать интерфейсы взаимодействия между ними: • «не ходим в БД из контроллеров, используем Repository»; • описать правила проектирования API: • «нельзя отдавать коллекцию без пейджинга».
33. @DmitryTsepelev DUMP’25 Как контролировать стиль кода 33 • code review; 👍 часть правил будет применяться 👎 люди тратят время на обсуждения стиля
34. @DmitryTsepelev DUMP’25 34 • code review; • регулярный аудит: 👍 Google: “readability review”; 👎 стандартизация будет фрагментарной; 👎 проблема копирования кода не решается. Как контролировать стиль кода
35. @DmitryTsepelev DUMP’25 35 • code review; • регулярный аудит; • CI + Linter с собственными правилами: 👍 после введения правила новых нарушений не будет; 👍 линтер становится средством формирования бэклога; 👍 мгновенное code review по стилю на CI; 👎 надо отдельно заниматься разработкой и поддержкой правил. Как контролировать стиль кода
36. Как я добавляю правила в линтер
37. @DmitryTsepelev DUMP’25 Rubocop — линтер в Ruby 37 # rule class ExtractInputType < Base MSG = "Consider moving arguments to a new input type" def on_class(node) schema_member = RuboCop : : GraphQL : : SchemaMember.new(node) if (body = schema_member.body) arguments = body.select { |node| argument?(node) } excess_arguments = arguments.count - cop_conf i g["MaxArguments"] return unless excess_arguments.positive? arguments.last(excess_arguments).each do |excess_argument| add_offense(excess_argument) end end end end # .rubocop.yml GraphQL/ExtractInputType: Include: "app/graphql/**/*" # .rubocop_todo.yml GraphQL/ExtractInputType: Exclude: - app/types/user_type.rb - app/types/order_type.rb
38. @DmitryTsepelev DUMP’25 Что должно быть в «хорошем» правиле линтера? 38 • понятное сообщение об ошибке; • тесты; • ссылка на документацию, включающую: • объяснение, почему так делать плохо; • инструкцию, как делать хорошо. • если все сделано правильно — получится ADR as code.
39. @DmitryTsepelev DUMP’25 Типичный flow работы с линтером 39 • ставим линтер; • генерируем список нарушений; • делаем обязательной проверку на CI; • постепенно чиним; • ⚠ часто пропускается: конфиги линтера и список нарушений — в codeowners.
40. @DmitryTsepelev DUMP’25 Предлагаемый flow работы с линтером 40 • ставим линтер; • генерируем список нарушений; • делаем обязательной проверку на CI; • ⚠ часто пропускается: конфиги линтера и список нарушений — в codeowners; • повторять бесконечно: • добавляем новое правило; • генерируем список нарушений; • чиним.
41. @DmitryTsepelev DUMP’25 Чего ожидать 41 • всем немножко не понравится; • большая часть договоренностей выполняется не всегда; • от части договоренностей придется отказаться; • исключений больше нет (они исчезают либо становятся правилами); • возможно часть правил относится только к частям проекта или отдельным командам; • технический бэклог начнет стремительно пухнуть.
42. @DmitryTsepelev DUMP’25 В каком порядке реализовывать правила? 42 • зависит от ситуации и приоритетов, например: • влияющие на производительность; • влияющие на безопасность; • улучшающие читабельность кода; • мешающие унификации.
43. @DmitryTsepelev DUMP’25 Как я формирую бэклог для стандартизации 43 • код, который часто меняется, видят и копируют чаще; • чем больше хорошего кода — тем больше вероятности, что используют его; • некоторые проблемы более опасны, чем другие. Гипотезы
44. @DmitryTsepelev DUMP’25 Как я формирую бэклог для стандартизации 44 • определить более и менее опасные проблемы; 📖 https:/ /dmitrytsepelev.dev/directing-refactoring # .rubocop_director.yml update_weight: 1 default_cop_weight: 1 weights: Graphql/AvoidFieldLoaders: 1 Graphql/NullableArrayField: 1 Isolation/ForbiddenDbCall: 2 Isolation/ForbiddenOperationCall: 2 Isolation/ForbiddenQueryCall: 1.5
45. @DmitryTsepelev DUMP’25 Как я формирую бэклог для стандартизации 45 • определить более и менее опасные проблемы; • найти часто изменяемые файлы: 📖 https:/ /dmitrytsepelev.dev/directing-refactoring git log - - since=\"2024-01-01\" \ - - pretty=format: \ - - name - only | sort | uniq - c | sort - rg 54 conf i g/locales/en.yml 43 db/schema.rb 41 app/services/feature.rb …
46. @DmitryTsepelev DUMP’25 Как я формирую бэклог для стандартизации 46 • определить более и менее опасные проблемы; • найти часто изменяемые файлы; • ранжировать и начать разбирать техдолг. 📖 https:/ /dmitrytsepelev.dev/directing-refactoring 💡 Checking git history since 1995-01-01 to fi nd hot fi les... 💡🎥 Running rubocop to get the list of o ff ences to fi x... 💡🎥🎬 Calculating a list of fi les to refactor... Path: app/controllers/user_controller.rb Updated 99 times since 2023-01-01 O ff enses: 🚓 Rails/SomeCop - 2 Refactoring value: 1.5431217598108933 (54.79575%)
47. И зачем?
48. @DmitryTsepelev DUMP’25 Сайд–эффекты от навязанных стандартов 48 • онбординг становится сложнее, но распространение знаний — быстрее и проще; • неправильное решение наносит больше ущерба, но проще исправляется; • на code review не обсуждается стиль; • задачи делаются быстрее.
49. @DmitryTsepelev DUMP’25 Унификация тестов 49 • мы знаем ответственность компонентов; • мы можем зафиксировать правила тестирования и проверить их.
50. @DmitryTsepelev DUMP’25 Унификация тестов: пример 50 describe ItemsController before do allow(ItemPolicy).to receive(:update) .and_call_original end specify do put :update, update_params expect(ItemPolicy).to have_received(:update) expect(response.status).to eq(200) # . . . остальная логика end end • правила контроллера: • обязан вызывать Policy; • не содержит логику (логика в Operation); • только принимает запросы и отправляет ответ.
51. @DmitryTsepelev DUMP’25 Унификация тестов: пример 51 describe ItemsController specify do expect { put :update, update_params } .to check_permissions(ItemPolicy, :update) .and perform_operation(ItemUpdateOperation) .and have_http_status(:ok) end end describe ItemsController before do allow(ItemPolicy).to receive(:update) .and_call_original end specify do put :update, update_params expect(ItemPolicy).to have_received(:update) expect(response.status).to eq(200) # . . . остальная логика end end
52. @DmitryTsepelev DUMP’25 Унификация тестов 52 • мы знаем ответственность компонентов; • мы можем зафиксировать правила тестирования и проверить их; • результат — покрытие 100% из коробки.
53. @DmitryTsepelev DUMP’25 Кодогенерация 53 • тесты могут быть настолько одинаковые, что их можно генерировать; • если правила достаточно зрелые, то можно попробовать генерировать и код.
54. @DmitryTsepelev DUMP’25 Кодогенерация: пример 54 • правила контроллера: • обязан вызывать Policy; • не содержит логику (логика в Operation); • только принимает запросы и отправляет ответ. # rake "generate_controller[Orders, update]" class OrdersController < ApplicationController def update authorize! order, to: :update, with: OrderPolicy result = Operations : : Order : : Update.call(params) respond_with result end end class OrderPolicy < BasePolicy def update = raise NotImplementedError end class Operations : : Order : : Update < BaseOperation def call = raise NotImplementedError end
55. @DmitryTsepelev DUMP’25 Спасибо! Вопросы? 55 Оцените доклад • устные договоренности о правилах написания кода опасны; • стандартизация кода начивается с определения ответственности компонента; • один из способов внедрять проверяемые стандарты — линтер; • стандартизированная кодовая база требует усилий, но может окупиться.