Это из внутреннего электронного письма, которым я поделился с командой год назад. За последние несколько лет, просматривая запросы на включение (PR) и рецензируя свой и чужой код, я заметил, что есть шаблон, которому я обычно следую.

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

Количественный

Это вещи, которые можно измерить или сравнить с чем-то объективным (число, как в покрытии кода, ответ «да/нет», четко определенные передовые практики в отрасли, соглашения о кодировании, согласованные командой и т. д.).

  • Делает ли код то, что должен делать? — это легко проверить, просмотрев заявку в вашем инструменте управления заявками. Убедитесь, что вы четко понимаете требование и критерии приемки, прежде чем смотреть код.
  • Проверено ли это? — несмотря на некоторую субъективность в отношении модульных или интеграционных тестов, тестирования фиктивных и реальных данных, вы можете получить "уровень комфорта" тестирования, посмотрев на испытаниях. Всегда сначала ищите тесты, прежде чем смотреть код.
  • NFR (нефункциональные требования) — хорошо ли он работает, безопасен ли он, достаточно ли журналов и мониторинга? Есть много очевидных ловушек, которые вы можете найти — вложенные циклы, ненужные итерации, слишком много вызовов ввода-вывода, запросы без предложений where или неправильные соединения — все это можно проверить невооруженным глазом, не запуская код. Если у вас есть автоматизированные тесты производительности, вы, вероятно, входите в 1% лучших компаний.
  • Делает ли он одну и только одну вещь? — бывают случаи, когда люди добавляют две незначительные ошибки в один PR, потому что это удобно. В достаточно небольшой кодовой базе и достаточно небольшой команде это может быть нормально. При этом есть несколько неудобств — увеличивает когнитивную нагрузку рецензента, усложняет автоматизацию закрытия заявок, в одном развертывании есть две вещи для тестирования, и если один тест проходит, а другой имеет проблемы, нам придется повторно посетить объединенный Пиаритесь и ищите отдельные коммиты (надеюсь, они охватываются 2-мя коммитами).

Качественный

Это вещи, которые субъективны и не имеют четкого правильного/неправильного.

  • Действительно ли нам нужно делать это таким образом? Есть ли какой-либо другой способ удовлетворить потребности пользователя и, желательно, без написания кода — путем разработки, устранения взаимодействия, которое могло бы привести к ошибке, предварительная проверка ввода формы?
  • Соответствует ли код/дизайн тому, что мы уже сделали или согласовали ранее? Соглашения об именах, шаблоны проектирования, правила, подобные eslint, объем тестирования, вид тестирования, документация?
  • Поддается ли это сопровождению? Сопровождаемость частично зависит от удобочитаемости, а другая — от расширяемости.
  • Достаточно ли это просто, чтобы люди могли понять его без объяснений? Организация кода, простые для понимания имена переменных, упрощенная логика, отсутствие кода черной магии или «не надо». изменить эту логику.
  • Можно ли это импровизировать итеративно или это попытка сделать все правильно с первого раза?Это сложно, так как вы просматриваете PR только после того, как он отправлен, на этом этапе разработчик уже, возможно, потратил много времени. Это полезно в упражнении по взаимному программированию.
  • Может ли этот код использоваться другим разработчиком для выполнения того, для чего он не предназначен? Сделали ли мы его достаточно простым, чтобы он выполнял только одну функцию? Если мы перегружаемся в первую очередь, кто-то перегрузит его снова через несколько недель, и вскоре вы получите чудовище, которое трудно даже рефакторить.

В конце концов, я верю в два руководящих принципа (помимо того, что обычно считается хорошим кодом):

  • Лучший код — это вообще отсутствие кода — https://blog.codinghorror.com/the-best-code-is-no-code-at-all/ — Джефф Этвуд
  • Сложность сложной части кода со временем возрастает в геометрической прогрессии — возможно, это я так говорю. Каждый раз, когда я вижу действительно сложный фрагмент кода, несколько месяцев спустя вокруг него происходит так много всего, что его становится безумно трудно отлаживать/рефакторить.

Обязательная оговорка: как и любое обобщение, это субъективная и моя точка зрения. Могут быть обстоятельства, когда я могу выбросить их из окна и что-то объединить. Но это для другого разговора.