Code Review. 80 lvl

Любой программный продукт, будь то веб-сайт или мобильное приложение, основан на коде. Чем согласованнее и целостнее эта база, тем удобнее с ней будет работать, например, при необходимости доработки проекта, передачи на сопровождение другой команде.

Основными критериями качественного кода являются следующие: простота восприятия, гибкость для модификаций, возможность обновления, понятность, тестируемость. Однако зачастую работа над проектом ведется в спешке, под давлением и код пишется людьми с разным уровнем квалификации (с разным мышлением). И даже опытные разработчики не всегда пишут код самого высокого качества. Поэтому для повышения качества кода проводится процедура code review.

Code review - это процесс, в котором другие разработчики программного обеспечения проверяют код конкретного инженера с целью проверить его соответствие стандартам, выявить ошибки, выявить несоответствия в стиле кодирования и проверить соответствие написанного кода поставленным задачам, и в целом характеристикам качественного кода.

Правильно проведенный процесс code review дает несколько преимуществ, которые важны как в целом, так и по отдельности. Укажем некоторые из них:

• Возможность исправить ошибки, которые могут вызвать проблемы в будущем;

• Соблюдение стандартов и стиля кода в команде;

• Улучшение архитектуры и дизайна решения;

• Проверка соответствия кода поставленной задаче;

• Повышение общего качества кода;

• Повышение квалификации и обучение программистов.

Благодаря code review уменьшается так называемый bus factor, поскольку при передаче проекта другой команде или привлечении к работе другого человека им будет легче понять, как все работает.

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

Несмотря на все вспоминаемые плюсы, у code review есть и недостатки. Первый - это длительность процесса, потому что некоторым людям приходится тратить время на просмотр кода, а другим - на его исправление. Другой недостаток - повышенная стоимость реализации проекта, так как разработчикам приходится оплачивать время.

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

Давайте посмотрим, на что обычно смотрит разработчик при проведении code review.

• Форматирование, стиль, наименование - Где находятся пробелы, переносы строк? Использованы табы или пробелы? Правильно ли расположены фигурные скобки? Как называются переменные? Правильно ли они объявлены? В каком месте объявлены?И т.д. То есть в целом проверяется стиль кода. Если в компании есть стандарт кода, то проверяется следование этому стандарту.

• Покрытие тестами - Есть ли тесты для модифицированного или нового кода?

Это некий гигиенический минимум для code review. Но этого крайне мало, чтобы получить качественный код!

Есть еще минимум три (четыре) аспекта, на что ревьюер (ревьюеры при групповом ревью) должен обратить внимание:

• Дизайн

• Читаемость, гибкость и модифицируемость

• Функциональность

  

На этапе ревью дизайна надо получить ответы на следующие вопросы.

• Как новый код вписывается в общую архитектуру? Придерживался ли разработчик общего архитектурного стиля или написал что-то, что сильно выбивается?

• Следует ли код дизайн парадигмам таким как SOLID, DDD, BDD, TDD или другим, которые приняты и используются в команде, компании при разработке продукта?

• Какие дизайн паттерны используются? Насколько используемый дизайн паттерн в целом применим при решении задачи?

• Находится ли новый код в логически правильном месте? Подходит ли он по контексту и смыслу той части кода, где он находится?

• Мог ли новый код повторно использовать что-то из существующего кода? Предоставляет ли новый код что-то, что мы можем повторно использовать в существующем коде? Дублирует ли новый код существующий? Если да, то следует ли его отрефакторить на более используемый на будущее, или это приемлемо на данном этапе?

• Не слишком ли код сложный (over-engineered)? Обеспечивает ли он возможность многократного использования, которого сейчас не требуется? Не нарушает ли новый код баланса повторного использования согласно принципам YAGNI?

С дизайном разобрались. Переходим к проверке читаемости, гибкости и модифицируемости кода.

• Отражают ли имена (полей, переменных, параметров, методов и классов) то, что они представляют, под что они заводились?

• Понимаю ли я что делает код, прочитав его?

• Понимаю ли я что делают тесты?

• Охватывают ли тесты достаточный набор кейсов? Учтены ли они все пути, альтернативные и исключительные случаи? Есть ли кейсы, которые не были рассмотрены?

• Понятны ли сообщения об ошибках при обработке исключений? И обрабатываются ли исключения?

• Есть ли документация или покрытие тестами для сложного и неодназначного кода?

Проверяя функциональную часть, ревьюер должен заострить внимание на следующих моментах.

• Действительно ли код делает то, что должен был делать? Если есть автотесты для проверки правильности кода, действительно ли тесты проверяют соответствие кода согласованным требованиям?

• Похоже ли, что код содержит небольшие ошибки, например, использование неправильной переменной для проверки или случайное использование и вместо или?

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

• Есть ли потенциальные проблемы с безопасностью в коде? Создает ли код новые дыры в безопасности системы?

• Есть ли нормативные требования, которые необходимо соблюдать? Не нарушил ли новый код нормативные требования, которые были приняты ранее? (К примеру, при прохождении сертификаций по безопасности)

• Вносит ли новый код проблемы с производительностью в области, которые не охвачены автоматическими тестами по производительности? Есть ли в коде, например ненужные вызовы базы данных или удаленной сервисов? Можно ли избежать этих вызовов?

• Нужна ли новая публичная документация по реализованной части? Нужно ли обновить существующую?

• Проверялись ли сообщения, предназначенные для пользователей, на правильность, корректность? Являются ли эти сообщения user-friendly?

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

Сode review - это процесс, который должен быть вшит в процесс разработки. Если есть возможность, то лучше делать так называемое групповое code review. Привлекать надо всю команду - от джунов до лидов (джуны будут познавать новый мир, лиды смотреть на код используя чек-лист code review). Без этого процесса построить качественный продукт очень сложно.