В процессе проверки кода можно использовать множество подходов.
Самый популярный способ обзора кода, с которым я столкнулся при работе с различными компаниями, - это сосредоточение внимания на деталях. Особенно, когда мы не знаем контекст.
Я хотел бы остановиться здесь на мгновение. Рассмотрим следующую ситуацию. Вы помните время, когда вы просматривали фрагмент кода и находили, что он идеально подходит для одного из «Банды четырех» (Шаблоны проектирования: элементы многоразового объектно-ориентированного программного обеспечения (1994)) узоры? Вы пытались понять, почему кто-то им не воспользовался? Или вы почувствовали этот захватывающий момент, и сразу же приземлился указывающий пальцем комментарий на пул-реквест? Может быть причина, по которой этот очевидный шаблон не использовался там, но вы не могли этого знать, потому что не знали контекста.
Знание этого также может помочь вам лучше понять наименования, группировку и другие решения, принятые предыдущими инженерами. С другой стороны, мы должны помнить, что контекст - это обоюдоострый меч, который может закрыть нам разум. Если мы знаем контекст, все будет ясно и читаемо, поэтому мы не увидим много проблем, которые могли бы уловить, не зная об этом. Помните об этом и всегда старайтесь надеть обувь новичка, когда выбираете лучший класс или имя переменной.
Не поймите меня неправильно, но думать о проверке кода только на уровне «чистого кода» может быть просто полировкой на поверхности. Выявление проблем с вложенными условными выражениями или несколькими параметрами в методе полезно, и мы должны продолжать это делать. Но, честно говоря, это не самый ценный результат проверки кода. Даже с учетом таких ошибок программа будет работать должным образом. И правда в том, что опытный программист может без проблем поддерживать его. Кроме того, пара вложенных операторов «if» более читабельна и понятна среднему программисту, чем шаблон декоратора. Нашим основным фокусом должен быть контекст, в который помещается проверяемый фрагмент кода и ищем в нем возможные проблемы.
Архитектура программного обеспечения должна отражать бизнес-логику и является наиболее важной частью программного обеспечения, на которой мы должны сосредоточиться. Также и особенно во время проверки кода. Если мы выявим проблемы в этом ядре и быстро на них ответим, это принесет продукту наибольшую ценность.
Мы определенно не должны пересматривать код с агрессивной позиции. Желание найти как можно больше ошибок и указать на них вслух в комментариях - вот что обычно делают молодые программисты. Или программисты, ищущие «мести». Это может нас расстроить, если в конце мы не обнаружим ничего плохого :). Такой подход заставляет задуматься о мелких деталях. Мы теряем фокус на общей картине архитектуры, в которой будет размещен этот фрагмент кода. Всегда лучше начинать обсуждение возможных вариантов использования, проблем и потребностей масштабирования в конкретных сценариях, а не навязывать шаблон проектирования, потому что он там совпадает.
Однако даже некачественная проверка кода лучше, чем ничего
Есть много компаний по всему миру, которые не проверяют свой код перед слиянием. Иногда в таких местах происходят быстрые действия, когда один (возможно, новый) член команды устанавливает инструмент проверки кода и объявляет об инициативе проведения проверки кода. Начиная с сегодняшнего дня! И это работает. На месяц. Затем все забывают о проверке кода из-за нехватки времени, ресурсов и других оправданий, которые вы можете придумать. Всегда можно найти оправдание. Процесс проверки кода останется прежним, но это не будет ничем иным, как просто сбором одобрений. Отсутствие проверки кода приводит к созданию некачественного программного обеспечения. Компании-разработчики программного обеспечения и инженеры знают о последствиях этого.
Привычки проверять код
Стоит упомянуть о различиях в привычках проверки кода в командах. Он будет другим из-за разнообразия опыта членов команды. Мы можем увидеть некоторые конструктивные комментарии и уловки, которые привносят большую ценность в код. Благодаря взаимодействию нескольких младших и старших инженеров можно долго обсуждать проблему.
Немного по-другому, когда это команда юниоров под руководством одного старшего. Проверка кода в этой комбинации будет выглядеть как руководство.
Интересный случай, когда проверка кода происходит в команде старших инженеров. Иногда при такой настройке каждое из предложенных решений не лучше другого. Это больше о предпочтениях и изученных руководствах. У большинства старших инженеров есть свои привычки и шаблоны, которым нужно следовать при написании кода. Попытки изменить это могут быть трудными. Они будут бороться за свои «права» с очень разумными и правдивыми аргументами. Очень важная способность - сдаться и найти компромисс. Это то, что отличает пожилых людей от других программистов.
Общей проблемой инженеров-программистов является то, что они чувствуют особую «связь» с кодом. Из-за этого они могут воспринимать каждый комментарий лично. Рецензенты должны быть очень осторожны, комментируя и предлагая изменения. Это может вызвать дискомфорт, который может стать преградой между автором и рецензентом.
Рецензенты ключевого кода
Знание архитектуры и специфики проекта является обязанностью наиболее вовлеченных и опытных инженеров. И они являются ключевыми рецензентами кода. Они могут легко обнаружить низкую связность, нарушение правил SOLID, утечки номенклатуры слоев и другие возможные ошибки, которые могут прикусить нам язык в будущем. Они могут предлагать разделение, извлечение, повторное использование или перемещение частей кода для облегчения обслуживания и удобства чтения. И все остальные преимущества, которые он может нам дать. Предвидеть последствия предлагаемого решения, учитывая возможное неправильное смещение или дублирование кода - это то, что они могут делать лучше всего.
Это те вещи, которые должны быть в центре нашего внимания при проверке кода.
Несомненно, нарушения архитектуры приводят к худшим последствиям. Хуже, чем пара аргументов в одном методе или слишком много строк класса.
Три уровня проверки кода
Учитывая все упомянутые правила и способы выполнения обзора кода, я подумал о разделении обзора кода в иерархическом порядке в зависимости от его важности и эффекта.
Архитектура
Первым и наиболее важным должно быть рассмотрение кода, ориентированное на архитектуру. Здесь следует учитывать специфику проекта и договоренности, которые мы заключили ранее. Этот уровень проверки кода должен выполняться в основном наиболее опытными программистами, которые понимают архитектуру проекта. Юниоры также могут попробовать выполнить проверку кода этого уровня, и это может быть хорошим упражнением. Однако им не нужно слишком напрягать свой разум, если они не чувствуют себя достаточно сильными для этого.
На этом этапе очень важно распознать возможные утечки и нарушения номенклатуры. Может быть правило, которое вам понятно, потому что вы знаете проект, но не обязательно всем известно об этом, или оно может быть прозрачным для них. Это хороший момент для начала обсуждения и выяснения мотивации автора.
Архитекторы и инженеры, занимающиеся проверкой кода архитектурного уровня, также должны помнить о необходимости масштабирования и повторного использования предлагаемого запроса на вытягивание. Знание этого может безоговорочно изменить реализацию.
Безопасность приложений также является одним из ключевых факторов, на которых необходимо сосредоточить внимание здесь.
Есть и другие архитектурные вещи, которые мы можем рассмотреть при проверке кода. Думаю, вы уже знаете, насколько ценным может быть обнаружение проблемы на этом уровне.
Очистка кода
На втором этапе обзора кода мы можем глубже погрузиться в сам код и сосредоточиться на деталях. Это можно назвать проверкой очистки кода.
Теперь мы можем спорить об уровнях сплоченности и составе классов. Полезно обсудить все сомнения и недоразумения, чтобы лучше понять процесс мышления, лежащий в основе кода. Мы всегда должны сначала спросить, прежде чем обвинять кого-то. Может быть, это что-то столь же умное, чего мы не могли понять.
Говоря о «чистом коде» (аплодирует дяде Бобу Мартингу и его книге «чистый код»), мы не можем забыть о правилах SOLID и тестировании. Если какое-либо из этих правил нарушается, мы должны быстро выяснить, почему. Может быть причина, по которой инженер решил не следовать этому правилу, но это также могло быть ошибкой. Ах, хороший улов!
Одна из самых сложных частей программирования - это наименование. Сложно правильно называть переменные, методы, функции, классы и структуры данных. Некоторые имена могут быть настолько очевидны для вас и совершенно неприемлемы для других. Даже несмотря на то, что мы всегда должны пытаться найти лучшие «имена», проверка кода второго уровня - это место, где можно начать обсуждение.
Полировка поверхности
Третий и последний этап - это полировка поверхности, проверка кода. Здесь мы можем указать на вещи, которые обычно могут быть автоматически обнаружены линтерами или компиляторами, например, форматирование кода, отсутствие точек с запятой и другие тривиальные проблемы.
Также пришло время для небольших обновлений сортировки / группировки, жалоб на используемые структуры данных, будь то List ‹› или Set ‹› и так далее.
Неважно, кто открыл пулреквест. Проверка кода всегда должна выполняться с одинаковой тщательностью. Даже опытные программисты не знают всего. Даже по поводу продукта, над которым они работают. В конце концов, мы все люди и делаем ошибки.
