Сравнение методик обзора кода

Думаю, многие разработки знакомы с понятием code review или обзор кода по-русски (также данный термин переводят как просмотр кода, инспектирование кода или рецензирование кода – далее, для единообразия, будет использоваться вариант «обзор кода»). Недавно я столкнулся с необходимостью «разложить по полочкам» и классифицировать знания по этой теме. Результат – данная статья. Надеюсь, она окажется полезной, а также поможет внедрить обзоры кода в свой производственный процесс тем, кто только об этом задумывается.
wtf per minute
Обзор кода является одним из наиболее эффективных методов поиска и устранения дефектов программы. Обзоры проводятся человеком, что позволяет находить широкий класс ошибок, в том числе с трудом детектируемых или вообще не детектируемых автоматическими средствами. Безусловно, обзор кода, не отменяет использование анализаторов кода или других методик обнаружения ошибок, например, unit-тестирования. К сожалению, не существует метода, который один обеспечил бы обнаружение всех дефектов программы (в исследованиях эффективность обзора кода обычно оценивается как 30-50% обнаруженных ошибок в приложении).

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

  • Улучшается архитектура приложения за счет того, что каждую часть системы продумали как минимум 2 человека.
  • Программист изначально мотивируется писать более качественный код, зная, что его будут просматривать.
  • Распространяются знания о проекте среди команды.
  • Происходит обмен программистским опытом.
  • Вырабатывается единый стиль кодирования в команде.

За все это приходится платить временем, которое могло быть потрачено на кодирование. Тем не менее, на проектах, рассчитанных хоть на какую-либо перспективу, затраченное время в будущем вернется сторицей за счет создания изначально качественного продукта, вместо судорожной «допилки» позже.

Можно выделить следующие виды обзоров кода:

  • Формальная инспекция кода.
  • Неформальная инспекция кода (другое название – анализ кода).
  • Чтение кода.
  • Парное программирование.

Рассмотрим каждый вариант по-отдельности.

Формальная инспекция кода


Формальная инспекция кода представляет собой как следует из названия формализированную процедуру просмотра кода. По МакКоннеллу она выглядит примерно так.

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

Более подробный пример формальной инспекции можно найти здесь.

Достоинства:

  • Очень высокая эффективность.
  • Благодаря составляемому списку ошибок легко проверить их устранение.
  • Инспекционные отчеты можно использовать в дальнейшем, например, для анализа характерных проблем.

Недостатки:

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


Неформальная инспекция кода


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

Достоинства:

  • Малые затраты времени.
  • Простой процесс не требующий формальных процедур.

Недостатки:

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


Чтение кода


Чтение кода – это самостоятельное изучение разработчиком чужого кода без присутствия автора. Данная практика является самой простой и распространенной из описанных здесь – думаю, любому разработчику так или иначе приходило читать чужой код. Не говоря уж про мир Open Source, где это зачастую единственный доступный метод обзора кода.

Достоинства:

  • Простота.
  • Высокая доступность – не требуется синхронизация во времени и пространстве.

Недостатки:

  • Медленная обратная связь – могут потребоваться дополнительные комментарии к коду, которые нельзя быстро получить, а иногда даже быстрее самому исправить дефект, чем сообщить об этом автору.


Парное программирование


Парное программирование является экстремальным методом обзора кода – обзор, осуществляемый постоянно: два разработчика за одним компьютером, за одним комплектом мыши и клавиатуры, вместе решают одну задачу. Широкое распространение парное программирование получило после появления методологии экстремального программирования, хотя активно использовалось и до этого. Часто парное программирование используется спонтанно: думаю, многим приходилось подходить к другому разработчику за компьютер, чтобы помочь решить сложную задачу.

Достоинства:

  • Высокая эффективность, особенно в плане обмена опыта и распространения знания о проекте.
  • Высокая концентрация на работе – работая в паре, разработчики гораздо меньше отвлекаются на посторонние вещи.
  • Естественное ограничение количества одновременно разрабатываемых командой задач – сконцентрированность на меньшем количестве задаче обеспечивает более качественную и быструю их реализацию, что позволяет непрерывно поставлять новые версии продукта.
  • Нет психологических вопросов присущих инспекциям – оба авторы кода, и оба одновременно его же инспекторы, предложения по улучшению воспринимаются именно как предложения, а не критика.
  • Повышение командного духа – успехи, достигнутые в паре, больше объединяют людей, чем индивидуальные достижения.
  • Отлично подходит для обучения новичков.

Недостатки:

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

Хочется добавить несколько слов к вышенаписанному. Как можно заметить некоторые недостатки вытекают из достоинств. В этом нет ничего удивительного – парное программирование методика достаточно не простая, как может показаться с первого взгляда, и не все ее свойства очевидны. Поэтому, начав работать в парах, не стоит ожидать моментальных результатов. Только после получения некоторого опыта парное программирование начнет приносить плоды, и более того – можно будет добиться минимизации некоторых негативных эффектов.

Что выбрать?


Теперь можно попробовать определиться какой метод и в каком случае стоит использовать.

  • Если вы решили делать обзоры кода, но не хотите сильно на этом заморачиваться, то для команды расположенной в одном месте неформальные инспекции будут лучшим выбором.
  • Для распределенной команды практически единственный доступный метод обзора это чтение кода. Несмотря на то, что существуют системы для удаленного парного программирования, чисто по психологическим причинам, они не так комфортны как программирование за одним компьютером. Кроме того, чтение кода можно практиковать и в нераспределенных командах, но когда стоит выбор гадать над назначением какого-то куска кода или поинтересоваться об этом у автора, я бы выбрал последнее.
  • Формальные инспекции отлично подходят для обзора сложных или критичных участков кода – в этом случае временные затраты с лихвой окупятся результатом. Некоторые практикуют постоянное использование формальных инспекций, но мне трудно представить, как можно формальными инспекциями провести обзор всего имеющегося кода.
  • Парное программирование не зря является одним из принципов экстремального программирования. Его высокая эффективность и дополнительные бонусы, присущие только этому виду обзора кода, действительно позволяют его рекомендовать как основной способ разработки в команде (за исключением простейших или рутинных задач). Оптимизма добавляет и то, что с большей частью недостатков можно успешно бороться. Самой большой проблемой может стать попытка уговорить менеджмент использовать такой расточительный, по их мнению, метод – тут вам в помощь книги и статьи практиков экстремального программирования.

Как видите, вариантов достаточно – можно выбрать наиболее подходящий для себя способ. Кроме того, никто не запрещает комбинировать методики (например, одна часть команды работает в парах, другая часть – в одиночку, но код, написанный ими, обязательно инспектируется). Можно начинать с более простых методик и постепенно переходить к сложным – главное начать, а положительные эффекты, думаю, не заставят себя ждать.

Литература


  1. Совершенный код (Code complete). Стив МакКоннелл (Steve McConnell). Глава 21. Совместное конструирование.
  2. Экстремальное программирование: постановка процесса. С первых шагов и до победного конца (Extreme Programming Applied: playing to win). Кен Ауэр, Рой Миллер (Ken Auer, Roy Miller). Глава 14. Прекращаем работать в одиночку.
Поделиться публикацией

Похожие публикации

AdBlock похитил этот баннер, но баннеры не зубы — отрастут

Подробнее
Реклама

Комментарии 18

    0
    Можно посмотреть как это происходит в chrome: codereview.chromium.org/. По-моему это не совпадает ни с одним из вариантов и лежит между формальной и неформальной инспекциями.
      0
      Вот кстати да, в google code (это вроде он) очень грамотно сделали. Есть поток изменений и по каждому ты сразу можешь смотреть дифф и ставить свои комменты. Я в одном своем проекте использовал и нам очень удобно было, самый удобный инструмент для коде ревью, который видел.

      У меня на этой и прошлой работе проблема с коде ревью состоит в том, что нет
      1) какого то формального способа рассмотреть все коммиты, пройтись по каждому
      2) нет формального и прозрачного способа привязать свои вопросы и мысли к коду (сейчас просто рассыпаю TODO комменты в надежде, что их поправят на второй итерации)

      Если бы был такой standalone тул, как google code, активно бы его использовал именно для ревью.
    +1
    Решили недавно в команде из 2 человек, для одного из проектов внедрить регулярное неформальное инспектирование кода.
    Товарищ написал 40 строк элементарного javascript кода, я написал 60 строк простейшего php кода. Мы знали, что будем проверять друг друга и от того пытались писать максимально качественно. За 5 минут проверки:

    У меня:
    3 стилистических ошибки.
    1 логический дефект.

    У товарища:
    2 стилистические ошибки.
    1 логическая ошибка.

    Не стану приводить исходники, но еще раз заверю, что они были элементарными, а ошибки — глупыми. Мной допущенный дефект тестирование бы не обнаружило. И главное то, что он бы свои ошибки у себя не нашел, а я бы не нашел у себя свои… Инспекция кода — крайне эффективно себя показала, и в дальнейшем без нее — мы не сделаем ни шагу. Рекомендую всем, оценка: 10.
    • НЛО прилетело и опубликовало эту надпись здесь
        0
        В Agile сложно планировать всю архитектуру с самого начала, приложение строится законченными, работоспособными частями.
          +1
          Даже хорошие программисты частенько ошибаются. Иногда по мелочи. Вспомним MVC, часто бывает так, что большую часть кода можно убрать из контроллеров в модельный слой. Толстые модели можно разбить по мелким модулям. И так далее.

          Все это в целом улучшает архитектуру (если широко смотреть на значение этого слова).
            0
            Если бы все можно было изначально правильно продумать и написать с первого раза умные дядьки не придумали бы такую вещь как рефакторинг. Зачастую только в процессе разработки становятся понятными тонкости, а иногда даже написав большой кусок кода понимаешь, что правильнее было бы переписать его совсем по-другому.
            Про непрерывность процесса совершенствования кода отлично написано в этой статье.
            • НЛО прилетело и опубликовало эту надпись здесь
            +1
            Снова общие слова и цитаты известных книг.
            Главная проблема всех этих слов: книги к тексту которых все оперируют — как правило говорят о большой разработке. много, очень много кода. Но дело в том, что большинство проектов «простые» и «маленькие» и применение подобных методик просто пустая трата ресурсов.
            Гораздо интересней вопрос — где же проходит та черта. Я не встречал обоснованного статистикой ответа на этот вопрос. А без него — это всё очередной выпуск бюллетеня КО.
              0
              Недавно накосячил в «Hello world» на С. После этого считаю, что в любом проекте инспеция полезная штука.
              А так — по опыту, наверное. Кто-то может написать проект размером X без ошибок, кто-то нет.
              +1
              Инспекция кода даже в самых маленьких проектах, в их самых малых частях, помогает найти ошибки.
              0
              Отличная статья! Я просмотрел страницы 3 хабра, и это единственный пост действительно полезный! «Совершенный код» считаю лучшей книгой по руководству тех процессом из всех которые я прочитал. Обзор кода по соотношению эффективность/времязатраты лучшее решение для предотвращения ошибок и улучшения качества кода. Только по формальному тестированию я бы делал так, один инспектор до собрания анализирует код, затем вся команда собирается и начинается обсуждение, слишком уж затратно назначать нескольких инспекторов. По экстремальному программированию — считаю достаточным детальное обсуждение архитектуры — построение диаграмм в паре. Например вы обсудили в паре архитектуру, кто-то один реализовал, затем напарник сделал неформальный обзор кода. А спорные вопросы можно выносить на формальные обзоры. Причём формальные обзоры проводить раз в неделю или раз за спринт, т.е. значительно реже.
                0
                P.S. Общие правила утверждённые на формальных обзорах обязательно вносятся в wiki проекта, для ознакомления будущих участников проекта и тех членов команды, которые ещё с ними не знакомы.
                0
                У нас все было очень просто:
                1. Любой feature-код, пишется в отдельной ветке в git-репозитории.
                2. Когда код написан, задаче ставится «Решено», о чем уведомляется инспектор кода.
                2. Вливать ветку в develop обязан лично инспектор. Он же и просматривает, что он вливает. Если есть замечания, задача переоткрывается, комментарии обычно пишутся прямо к каждой строчке кода в диффе.
                3. Если претензий нет, задаче ставится статус «Проверена» и ветка вливается в develop.
                  0
                  Отличный вариант не отнимающий много времени. А по какому принципу у вас назначаются инспекторы на конкретные фичи?
                    0
                    Обычно инспектором выступает главный разработчик конкретного проекта, для которого пишется фича. Этот человек отвечает за архитектуру и развитие проекта в целом, знаком с ним «от» и «до».
                    Практика показывает, что если такого человека нет, или их несколько, рано или поздно в коде начинается бардак, все что-то делают, но в общем структура проекта теряет целостность, появляется большое количество недоработок и затычек.
                    Если же что-то пойдет не так, претензии в первую очередь будут предъявлены именно главному разработчику (инспектору), в не зависимости от того, кто, когда и при каких условиях писал проблемный код, потому инспектор явно заинтересован в проведении инспекции кода, вливаемого в его проект.

                Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                Самое читаемое