Практика code review или, если перевести на русский язык, инспекций кода появилась давно и уже успешно встроена в процессы разработки во многих компаниях. Команды программистов таких компаний, как Google, Square, Amazon, активно ее используют, и у них нет даже мысли о том, чтобы от нее отказаться. В то же время, в большом количестве компаний эту практику или совсем не применяют, или применяют от случая к случаю.Процесс внедрения инспекций кода в нашей команде был начат несколько лет назад. Прежде, чем прийти к текущему состоянию, были опробованы различные подходы, методики и инструменты, было перечитано огромное количество постов и книг. Наибольшую пользу нам принесло изучение опыта других команд (как положительного, так и отрицательного). Это помогало определяться с вектором развития и быть подготовленными к возникающим трудностям. Сейчас же, я думаю, настала пора поделиться нашим опытом внедрения и использования инспекций кода при разработке приложений.
Почему важны инспекции кода?
Любой человек, связанный с разработкой программного обеспечения, не задумываясь, скажет вам, что стоимость исправления дефектов в продукте зависит от времени их обнаружения. Чем раньше мы найдем ошибку, тем дешевле будет избавиться от неё. Поэтому, очевидно, что любая практика, позволяющая сократить количество ошибок в продукте, вызывает интерес у всех заинтересованных лиц: программистов, менеджеров, заказчиков.
В то же время инспекция кода помогает в обучении младших программистов. В силу своей неопытности они допускают довольно большое количество ошибок в архитектуре, коде, комментариях. Поэтому очень важно с первого дня приучать их следовать установленным правилам, касающимися написания нового кода и хорошему тону взаимодействия с кодом существующим.
Программисты, читая код, который был написан их коллегами, неизбежно знакомятся и с ним, и с задачами, которые он должен решать. В результате у команды повышается уровень владения кодом. Разработчики начинают понимать, как работают модули приложения, написанные другими людьми. Уменьшается вероятность дублирования существующего функционала (не кода!). Сокращаются риски, связанные с уходом или временным отсутствием члена команды. В итоге, любой разработчик готов к выполнению даже тех задач, которые затрагивают не только его код.
В случае, когда разработчики знают, что их код будет обязательно просмотрен, они начинают относится к его написанию более ответственно: тщательно продумывают, как встроить функционал в существующую архитектуру, добавляют достаточное количество комментариев и т.д.
Перечисленные выше аргументы очевидны, и с ними обычно никто не спорит. Но, на мой взгляд, наибольшее внимание в инспекциях кода представляют преимущества, которые носят долгосрочный характер. Ведь именно в них должен быть заинтересован руководящий состав или заказчик (в случае заказной разработки).
Во-первых, если инспектирование кода не проводится, то очень легко оказаться в ситуации, когда внедрение нового функционала будет вызывать серьезные проблемы у программистов. В свою очередь заказчик будет недоумевать: почему на добавление небольшого функционала требуется так много времени? Ответ достаточно прост – за качеством архитектуры приложения, его кода и вносимых в них изменений никто не следил.
Во-вторых, в коде с низким качеством сложно ориентироваться, что ведет к появлению новых ошибок, как в местах исправлении старых дефектов, так и при добавлении нового функционала. В результате, сроки релизов начинают отодвигаться. Несмотря на все усилия команды тестирования, релизы все равно выходят с дефектами. Это, в свою очередь, увеличивает нагрузку на службу технической поддержки, что в итоге приводит к недовольству пользователей приложения.
Почему инспекции кода все равно не проводятся?
Программисты, в силу специфики своей профессии, очень умные люди и умеют считать. В особенности, они очень хорошо считают свое время и следят за своей продуктивностью. Согласно их мнению, инспекция кода отвлекает от текущих задач и снижает продуктивность. Дефекты, найденные в ходе инспекции кода, также не представляют для них особого интереса, потому что в основном представляют собой список мелких ошибок, допущенных младшими разработчиками. Более того этот список стабильно повторяется, и его обсуждение представляет собой довольно скучный процесс для опытных разработчиков. Проведение инспекции кода предполагает, что все найденные дефекты будут исправлены. Однако, тут проявляется огромное нежелание программистов вносить изменения в работающий код. Также, если инспекции кода проводятся редко, то программист, которому было это поручено, просто теряется в огромном количестве нового кода. И для него это выглядит как наказание. Эти моменты не мотивируют команду к систематическому проведению инспекций кода. Программисты считают, что результат полученный в ходе инспекций кода не стоит затраченных усилий. Это является следствием недочетов в подходе к проведению инспекций кода и нежеланию их исправлять.
Ранее упоминалось, что в проведении инспекций кода должен быть заинтересован и заказчик. Но, как показывает практика, приложение в первую очередь оценивают по заявленному функционалу и результатам тестирования. Практически никто не предъявляет требований к исходному коду и, соответственно, никто его не проверяет. Вопрос же о низком качестве кода появляется в двух случаях:
- при получении проекта от другой команды;
- при постоянном переносе сроков сдачи из-за большого количества ошибок.
К сожалению, что-то менять в этих случаях уже поздно, но существует несколько вариантов выхода из сложившейся ситуации. Первый, очевидный, — поддаться на уговоры программистов «все переписать». Однако, есть шанс довериться слабой команде с неотлаженными процессами и, на выходе, снова получить еще одну мешанину кода. Второй — проанализировать (проинспектировать) существующий код и подготовить стратегию его реанимации. Этот вариант более долгий и требует огромных усилий от проектировщиков и разработчиков.
Типы инспекций кода
Существует несколько типов инспекций кода [1]:
- формальные инспекции (formal inspections);
- инспекция через плечо (over-the-shoulder review);
- инспекция кода с помощью e-mail (e-mail pass-around reviews);
- парное программирование (pair-programming);
- инспекция кода с помощью специальных средств (tool-assisted reviews).
В формальных инспекциях, как правило, участвует вся команда. Данный тип предполагает, что каждый программист проанализирует исходный код и подготовит свой список замечаний. Неотъемлемой частью является собрание, на котором происходит обсуждение найденных дефектов. Результатом собрания должен быть список дальнейших действий команды относительно найденных ошибок.
При парном программировании два разработчика решают вместе одну задачу за одним компьютером. В случае инспекции через плечо разработчик просто показывает код своему коллеге.
При инспекции с помощью e-mail измененный код автоматически отправляется на анализ другому программисту.
Инспекция кода с помощью специальных средств предполагает наличие утилит помогающих в проведении анализа.
Они облегчают:
- сбор внесенных в код изменений;
- сравнение текущего и предыдущего состояний кода;
- ведение диалога, касающегося определенного кода;
- сбор статистики;
- форсирование проведения инспекции кода и исправления найденных дефектов;
- и т.д.
Наш опыт проведения инспекций кода
Мобильная разработка начала активное развития после презентации первого телефона iPhone. В силу того, что это довольно молодая область, программистам работающем в ней важна кооперация, адаптация существующих подходов из области веб или десктоп разработки, работа над новыми идеями. И здесь очень сильно выручает практика инспекций кода.
Наша команда мобильной разработки прошла длинный путь внедрения инспекций кода. Все начиналось с периодических формальных инспекций. Сейчас же используются практически все типы инспекций кода. Большая часть однотипных действий при анализе кода автоматизирована, что позволило безболезненно внедрить данную практику в процесс разработки программного обеспечения.
Прежде всего в дополнение к формальным инспекциям кода была интегрирована проверка проектов статическими анализаторами кода. В случае разработки под Android использовались Pmd, Findbug, Checkstyle, Lint. Это позволило в автоматическом режиме находить все проблемы кода, связанные с несоблюдением стиля кодирования, неправильным использованием системных API, ошибок в построении интерфейса пользователя и т.д. По началу интеграция данных анализаторов на билд-сервере полностью удовлетворяла наши потребности. Однако, со временем, данный подход перестал нас устраивать по многим причинам. В особенности нам не нравилось, что:
- каждый анализатор приходилось настраивать отдельно, а при решении использовать новый анализатор – его добавление во все проекты делалось вручную;
- не было удобной и централизованной возможности управлять правилами анализаторов (отключать, добавлять, редактировать);
- отсутствовала возможность игнорировать ложные срабатывания не изменяя код;
- нельзя было хоть как-то взаимодействовать с замечаниями (например, откладывать их исправление, комментировать и т.д.);
- замечания, генерируемые различными анализаторами нельзя было объединить в один отчет.
Результаты анализа проекта с помощью PMD (Jenkins plugin)
Подробная информация о найденном дефекте с помощью PMD (Jenkins Plugin)
Вскоре мы поняли, что дальше так продолжаться не может – команда росла, и каждому новому программисту приходилось усваивать довольно большой объем информации относительно существующих процессов разработки и используемых утилит. Тогда было принято решение попробовать платформу SonarQube[3], которая предназначена для постоянного отслеживания качества кода. Несмотря на наши сомнения, платформа показала себя только с хорошей стороны. Она решила большую часть наших проблем, связанных со статическими анализаторами кода, добавила новые метрики, по которым можно отслеживать состояние проекта. Управление правилами стало централизованным, появилась возможность составлять планы исправления найденных дефектов и т.д.
Результаты анализа проекта в SonarQube
Подробная информация о найденном дефекте в SonarQube
Статические анализаторы кода, конечно, очень помогают, но полностью полагаться на них нельзя. Они позволяют избавить формальные инспекции кода от обсуждения очевидных дефектов и указать на подозрительные места. Но посоветовать что-то, например, в плане архитектуры приложения они не в силах.
Проведение формальных инспекций кода, даже с учетом помощи статических анализаторов, слишком затратное по времени мероприятие. Поэтому, с одной стороны, если его проводить часто, продуктивность программистов действительно снизится, с другой – если проводить редко, увеличится количество кода, которое необходимо проинспектировать. Согласно же проведенным исследованиям большое количество кода только снизит число найденных дефектов [2].
В поисках решения, которое было бы так же эффективно, как формальная инспекция кода, но не отнимало так много времени, мы пришли к использованию Atlassian Crucible [4]. Фактически это средство взяло на себя всю работу по:
- подготовке изменений кода к проверке;
- навигации в коде;
- ведению диалогов относительно определенных строк кода;
- приглашению разработчиков для проведения инспекции;
- отслеживанию исправлений, согласно замечаниям коллег;
- и т.д.
Самая же интересная возможность, которой мы активно пользуемся в Crucible, состоит в том, что теперь все изменения в коде привязаны к определенной задаче. То есть, это позволяет проводить инспекции кода относительно определенной задачи. В результате, замечания коллег-программистов носят более предметных характер и действительно помогают найти и исправить нетривиальные ошибки.
Пример экрана пользователя при проведение инспекции кода с помощью Atlassian Crucible
Пример экрана создания инспекции кода в Atlassian Crucible
Таким образом, в нашей команде каждый коммит сделанный разработчиком проверяется на билд-сервере статическими анализаторами. Вся собранная статистика аккумулируется в SonarQube и используется всеми заинтересованными лицами: программистами – для самопроверки и определения проблемных мест, руководством – для понимания состояния проекта. Когда программист заканчивает работу над задачей, он начинает инспектирование кода. С помощью Crucible выделяются изменения связанные с завершенной задачей и приглашаются члены команды. Когда код будет всех устраивать инспекция кода завершается. В результате каждая строчка кода подвергается анализу, и за нее отвечает теперь команда целиком.
Хотя мы и перешли к проведению инспекций кода посредством специальных средств, полного отказа от формальных инспекций не было. Они все равно проводятся, но носят более глобальных характер, т.к. на них обсуждается вопросы связанные с архитектурой приложения. Формальные же инспекции проводятся в обязательном порядке для всех новых проектов, которые поступают к нам.
Что мы имеем?
Инспекции кода несомненно помогают в разработке качественных приложений как со стороны пользователя (приложение не падает, не тормозит, работа с ним интуитивно понятна), так и со стороны разработчиков (отличная архитектура приложения, отсутствие технического долга). В то же время каждая команда программистов уникальна, и поэтому обычно требуется особый подход к внедрению каких-либо новых практик. В нашем случае была поддержка как со стороны программистов, так и со стороны руководства, что позволило успешно встроить инспектирование кода в процессы разработки. Результатом данной работы стало:
- увеличение количества дефектов обнаруженных и исправленных до момента тестирования;
- уменьшение времени затрачиваемого на инспекцию кода;
- уменьшение времени затрачиваемого на тестирование приложения;
- заинтересованность команды в проведении анализа кода;
- отслеживание результатов проведенных инспекций кода;
- расширение области знаний разработчиков о коде проектов;
- более быстрый обмен опытом между начинающими и продвинутыми разработчиками;
- наблюдаться сокращение замечаний предъявляемых к коду (в силу роста общей компетенции команды);
- уменьшение рисков затягивания сроков поставки;
- появление метрик, по которым можно косвенно судить о состоянии проектов.
В итоге
Необходимо отметить, что инспекции кода являются не единственной практикой, которая помогает при разработке программного обеспечения. Целью данной статьи было рассказать именно о ней, поэтому отвлечений на другие происходящие процессы не было сделано. В целом наша команда довольна полученным результатом, но не собирается останавливаться на достигнутом. Надеемся наш опыт окажется вам полезен.
Ссылки
- Best Kept Secrets of Peer Code Review / J. Cohen, St. Teleki, E. Brown
- 11 Best Practices for Peer Code Review // smartbear.com/SmartBear/media/pdfs/11_Best_Practices_for_Peer_Code_Review.pdf
- Atlassian Crucible Overview — www.atlassian.com/software/crucible/overview
- SonarQube — www.sonarqube.org