Пользователь
0,0
рейтинг
19 сентября 2012 в 23:20

Разработка → Практические советы для эффективного инспектирования кода

Инспектирование кода — это очень сложная и ответственная задача, которая может отнимать много ресурсов. Важно ответственно подходить к инспектированию и проводить его эффективно. Эффективно — это значит тратить мало времени и находить много дефектов. Но как повысить эффективность? Ниже представлены несколько советов, которые помогут в этом.

Котлеты отдельно, мухи отдельно


Зачастую при модификации кода в рамках работы над некоторой задачей приходит понимание, что этот код следовало бы сначала отрефакторить. Не нужно в одну инспекцию включать рефакторинг и реализацию задачи. Их следует разделить на две разные инспекции. Это нужно, чтобы упростить жизнь инспектору и повысить шанс обнаружения им ошибки, ведь если объединить в одну инспекцию модуль с потенциально опасными изменениями с другими 100 модулями, которые были модифицированы с помощью средства для автоматизированного рефакторинга (например, переименовали класс), то есть высокая вероятность, что инспектор пропустит потенциально опасные изменения.
Другими словами, не надо объединять в одну инспекцию решения разных задач. Рефакторинг — это отдельная задача, даже если она появилась в результате работы над другой задачей.

Бокс по переписке


Многие средства, автоматизирующие инспектирование кода, позволяют оставлять комментарии к исходному коду и дефектам (например, Code Collaborator). При этом зачастую получаются бурные обсуждения, когда инспектор заводит дефект, а автор начинает ему объяснять в комментариях, что это, на самом деле, вовсе не дефект. В результате начинается священная война, которая может продолжаться бесконечно долго. Избежать этого очень просто: не нужно вообще оставлять комментарии при проведении инспекций. В этом случае процесс инспектирования протекает следующим образом:
  1. Инспектор заводит дефект.
  2. Если автор согласен с дефектом, он его исправляет, после чего инспектор закрывает этот дефект.
  3. Если автор не согласен с дефектом, то он устно обсуждает этот дефект с инспектором. Никакие комментарии к дефекту не добавляются.
  4. Если автор с инспектором успешно договорились, то, в зависимости от договоренностей, либо автор исправляет дефект, либо инспектор его удаляет.
  5. Если же началась священная война, в которой ни одна из сторон не желает уступить, то на помощь призывается арбитр, который рассудит инспектора с автором.


А что это вы тут делаете?


Перед проведением инспекции обязательно надо удостовериться в том, что вы точно понимаете решаемую инпектируемым кодом задачу. Звучит, как очередное откровение Капитана Очевидность, но практика показывает, что часто инспектор механически делает свою работу, смотрит на соблюдение формальных стилистических требований, не вникая в задачу. Код может быть сколько угодно «красивым», иметь все характеристики SOLID, обрабатывать все возможные ошибки, но если он не решает поставленную задачу, то этот код годится разве что для демонстрации на очередном собеседовании.

Какая гадость эта ваша заливная рыба!


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

На вкус и цвет все фломастеры разные


При заведении дефектов держите себя в руках и не скатывайтесь до вкусовщины. Вкусовщина ведет к конфликтам, потерянному времени на священные войны и не конструктивна. Избежать её очень просто: перед заведением дефекта нужно подготовить набор веских аргументов, почему этот дефект необходимо исправить. Аргументы типа «мне так больше нравится» или «так красивее» вескими не считаются. Если никаких других аргументов не осталось, то не надо заводить дефект, пусть код останется в «авторском» варианте, даже если бы вы написали его иначе.

А если бы у него в кармане была граната?


Обращайте внимание на альтернативные сценарии. Часто программист сконцентрирован на основном потоке выполнения программы и забывает корректно обработать альтернативные. Вам, как лицу не погруженному в задачу, легче заменить подобный прокол.

О пользе обоняния


Все дефекты имеют свои неповторимые запахи, которые здорово помогают их обнаружить. Запахи архитектурных дефектов хорошо описаны в книге Мартина Фаулера «Рефакторинг», которая обязательна для прочтения любым инспектором кода. Но не только архитектурные дефекты пахнут, свои нотки имеют также и алгоритмические. Например, если многопоточый код в блокировке обращается к другому коду, из которого может быть осуществлен callback, или используется несколько объектов синхронизации, то пахнет дедлоком. Запахи алгоритмических дефектов являются хорошей темой для отдельной статьи.

P.S.: Очень большая просьба делиться в комментариях своим опытом инспектирования кода. Что бы вы посоветовали для эффективного инспектирования?
@icoder
карма
10,0
рейтинг 0,0
Реклама помогает поддерживать и развивать наши сервисы

Подробнее
Спецпроект

Самое читаемое Разработка

Комментарии (17)

  • 0
    — Если инспекторов несколько, то очень полезно, чтобы они не видели дефекты, найденные другими инспекторами. Это позволит потом посчитать количество дубликатов (одинаковых дефектов от разных инспекторов). Обычно чем их больше, тем выше качество инспекции.

    — Все изменения, сделанные автором в ходе исправлений нужно снова послать на инспекцию. А то иногда бывает, что исправлений больше, чем исходного кода и это все коммитится в репозиторий без инспекции.
    • 0
      > — Если инспекторов несколько, то очень полезно, чтобы они не видели дефекты, найденные другими инспекторами. Это позволит потом посчитать количество дубликатов (одинаковых дефектов от разных инспекторов). Обычно чем их больше, тем выше качество инспекции.

      Интересная мысль, спасибо!

      > — Все изменения, сделанные автором в ходе исправлений нужно снова послать на инспекцию. А то иногда бывает, что исправлений больше, чем исходного кода и это все коммитится в репозиторий без инспекции.

      Согласен, скажу даже больше: исправления должны валидироваться авторами дефектов.
  • 0
    По пунктам:

    >> А что это вы тут делаете?

    Понимание инспектируемой задачи, бесспорно, важный момент. Но определение того, выполняет ли код поставленную задачу, это прерогатива отдела тестирования, а не инспектирования. Как Вы сами сказали, «Котлеты отдельно, мухи отдельно».

    >> На вкус и цвет все фломастеры разные

    Этого легко избежать, если инспектор будет руководствоваться чётко установленными стандартами. И при возврате кода программисту на доработку можно всегда указать на тот или иной пункт в Корпоративных Стандартах. У нас в компании таких документов несколько, в том числе и стандарты стилистического оформления кода.
    • 0
      Забыл добавить, что стандарты не берутся с потолка и/или не являются чьей-то прихотью в стиле «Мне так нравится, я начальник, значит так и будет». Все стандарты в нашей компании выработаны ключевыми программистами, обсуждались всем коллективом, а потом уже внедрялись.
      • 0
        В данном случае имелись ввиду не стандарты кодирования, для контроля которых, кстати говоря, и инспектирование не нужно, так как с этим отлично справляются автоматизированные средства в момент сборки.
        Имелась ввиду вкусовщина в плане способа решения задачи (архитектура, разбиение ответственности, использованный алгоритмы, паттерны и пр.). Тут могут быть очень важные дефекты, которые обязательно нужно править, но должны быть аргументы «за» это кроме «мне декоратор нравится больше, чем стратегия»
    • 0
      > Но определение того, выполняет ли код поставленную задачу, это прерогатива отдела тестирования, а не инспектирования.
      Во-первых тестирование и инспектирование решают одну и туже задачу: ищут дефекты. При этом инспектирование позволяет обнаружить такие дефекты, которые тестирование не факт, что и найдет.
      Во-вторых, задачи бывают в том числе технические (например, рефакторинг). Тестировщикам тут вообще делать нечего.
      • 0
        Собственно, Вы сами и подтвердили свои слова: «Котлеты отдельно, мухи отдельно». Хоть инспекторы с тестировщиками и решают одну и ту же проблему поиска дефектов, они всё-таки выискивают разные по типу дефекты. Следовательно, тут и подход разный, и инструменты, и люди с их квалификацией.
        • 0
          Множества дефектов, обнаруживаемых при инспектировании и при тестировании, конечно, не совпадают, но они во многом пересекаются. Чем раньше дефект будет пойман, тем дешевле его исправление, соответственно, дефект, пойманный на инспектировании в исправлении обходится дешевле, чем тот же дефект, пойманный во время тестирования. Так что нет никаких причин во время инспектирования не искать дефекты, которые теоретически могут быть пойманы тестировщиками (если они его не прошляпят).
          • 0
            Безусловно, соглашусь с Вами. Однако, есть один тонкий момент: цена рабочего времени специалиста. Как правило, рабочее время инспектора (он же квалифицированный программист) стоит гораздо дороже времени тестировщика. Поэтому компании не выгодно, чтобы инспектор делал то, что может сделать тестировщик.
            • +1
              Все зависит от стоимости пропущенного в production дефекта, то есть от конкретного программного продукта. Иногда такие дефекты стоят очень дорого, и тут даже время генерального директора можно потратить на поиск такого дефекта, если оно принесет пользу :)
              • 0
                Ага. Но стоимость дефекта можно определить лишь после его «поимки», а мы говорим о цене самого поиска.
                • 0
                  Не обязательно. Стоимость пропущенного в production дефекта равняется стоимости выпуска критического исправления, которая более менее известна заранее и зачастую очень высокая.
                  • +2
                    Вот и понеслась Ваша
                    священная война
    • 0
      Понимание инспектируемой задачи, бесспорно, важный момент. Но определение того, выполняет ли код поставленную задачу, это прерогатива отдела тестирования, а не инспектирования. Как Вы сами сказали, «Котлеты отдельно, мухи отдельно».

      Практически, это обычно определяется тем, какой специалист в данный момент доступен. Если ему легко понять и проверить код на соответствие решаемой задачи, грех этим не воспользоваться.
  • +1
    Инспектировать код лучше в привычной среде разработки. Тот же Code Collaborator (у нас активно используется) — ужасный по качеству инструмент. Например, нельзя оставлять код в комментариях. Так что мы тоже не пользуемся комментариями в нём, но по более простой причине — они же не работают!

    Отправляя код на ревью, важно помнить что ревью призвано уменьшить количество ошибок, а не исключить их. Не надо оставлять какие-то изменения на потом при рефакторинге с мыслью «если забуду, во время ревью напишут» — не напишут. Соответственно, если после ревью в коде обнаруживается очевидный баг — не надо говорить ревьюверу «ну как же ты такой баг просмотрел?!» Такая атмосфера приведёт к трате непропорционального количество времен на инспекцию кода.
    • НЛО прилетело и опубликовало эту надпись здесь
  • 0
    По моему опыту, для инспектирования кода на языке Java гайдлайн номер ноль — это инспектировать в IDE. Иначе от инспектирования кода вреда будет больше, чем пользы, и лучше его вообще не делать или делать максимально быстро и формально, если уж распорядок требует. Выявить без IDE в новом коде какую-нибудь существенную ошибку, которую до этого не выловил компилятор, практически нереально.

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