Инспектирование кода — это очень сложная и ответственная задача, которая может отнимать много ресурсов. Важно ответственно подходить к инспектированию и проводить его эффективно. Эффективно — это значит тратить мало времени и находить много дефектов. Но как повысить эффективность? Ниже представлены несколько советов, которые помогут в этом.
Зачастую при модификации кода в рамках работы над некоторой задачей приходит понимание, что этот код следовало бы сначала отрефакторить. Не нужно в одну инспекцию включать рефакторинг и реализацию задачи. Их следует разделить на две разные инспекции. Это нужно, чтобы упростить жизнь инспектору и повысить шанс обнаружения им ошибки, ведь если объединить в одну инспекцию модуль с потенциально опасными изменениями с другими 100 модулями, которые были модифицированы с помощью средства для автоматизированного рефакторинга (например, переименовали класс), то есть высокая вероятность, что инспектор пропустит потенциально опасные изменения.
Другими словами, не надо объединять в одну инспекцию решения разных задач. Рефакторинг — это отдельная задача, даже если она появилась в результате работы над другой задачей.
Многие средства, автоматизирующие инспектирование кода, позволяют оставлять комментарии к исходному коду и дефектам (например, Code Collaborator). При этом зачастую получаются бурные обсуждения, когда инспектор заводит дефект, а автор начинает ему объяснять в комментариях, что это, на самом деле, вовсе не дефект. В результате начинается священная война, которая может продолжаться бесконечно долго. Избежать этого очень просто: не нужно вообще оставлять комментарии при проведении инспекций. В этом случае процесс инспектирования протекает следующим образом:
Перед проведением инспекции обязательно надо удостовериться в том, что вы точно понимаете решаемую инпектируемым кодом задачу. Звучит, как очередное откровение Капитана Очевидность, но практика показывает, что часто инспектор механически делает свою работу, смотрит на соблюдение формальных стилистических требований, не вникая в задачу. Код может быть сколько угодно «красивым», иметь все характеристики SOLID, обрабатывать все возможные ошибки, но если он не решает поставленную задачу, то этот код годится разве что для демонстрации на очередном собеседовании.
Перед проведением инспекции, пока взгляд не замылен, подумайте, как бы вы решили поставленную задачу? Если вы вдруг обнаружите, что ваше решение существенно лучше предоставленного на инспекцию, не стесняйтесь заявить об этом автору. Пусть лучше он сейчас его исправит, пока это еще не так дорого и оно не обрасло костылями.
При заведении дефектов держите себя в руках и не скатывайтесь до вкусовщины. Вкусовщина ведет к конфликтам, потерянному времени на священные войны и не конструктивна. Избежать её очень просто: перед заведением дефекта нужно подготовить набор веских аргументов, почему этот дефект необходимо исправить. Аргументы типа «мне так больше нравится» или «так красивее» вескими не считаются. Если никаких других аргументов не осталось, то не надо заводить дефект, пусть код останется в «авторском» варианте, даже если бы вы написали его иначе.
Обращайте внимание на альтернативные сценарии. Часто программист сконцентрирован на основном потоке выполнения программы и забывает корректно обработать альтернативные. Вам, как лицу не погруженному в задачу, легче заменить подобный прокол.
Все дефекты имеют свои неповторимые запахи, которые здорово помогают их обнаружить. Запахи архитектурных дефектов хорошо описаны в книге Мартина Фаулера «Рефакторинг», которая обязательна для прочтения любым инспектором кода. Но не только архитектурные дефекты пахнут, свои нотки имеют также и алгоритмические. Например, если многопоточый код в блокировке обращается к другому коду, из которого может быть осуществлен callback, или используется несколько объектов синхронизации, то пахнет дедлоком. Запахи алгоритмических дефектов являются хорошей темой для отдельной статьи.
P.S.: Очень большая просьба делиться в комментариях своим опытом инспектирования кода. Что бы вы посоветовали для эффективного инспектирования?
Котлеты отдельно, мухи отдельно
Зачастую при модификации кода в рамках работы над некоторой задачей приходит понимание, что этот код следовало бы сначала отрефакторить. Не нужно в одну инспекцию включать рефакторинг и реализацию задачи. Их следует разделить на две разные инспекции. Это нужно, чтобы упростить жизнь инспектору и повысить шанс обнаружения им ошибки, ведь если объединить в одну инспекцию модуль с потенциально опасными изменениями с другими 100 модулями, которые были модифицированы с помощью средства для автоматизированного рефакторинга (например, переименовали класс), то есть высокая вероятность, что инспектор пропустит потенциально опасные изменения.
Другими словами, не надо объединять в одну инспекцию решения разных задач. Рефакторинг — это отдельная задача, даже если она появилась в результате работы над другой задачей.
Бокс по переписке
Многие средства, автоматизирующие инспектирование кода, позволяют оставлять комментарии к исходному коду и дефектам (например, Code Collaborator). При этом зачастую получаются бурные обсуждения, когда инспектор заводит дефект, а автор начинает ему объяснять в комментариях, что это, на самом деле, вовсе не дефект. В результате начинается священная война, которая может продолжаться бесконечно долго. Избежать этого очень просто: не нужно вообще оставлять комментарии при проведении инспекций. В этом случае процесс инспектирования протекает следующим образом:
- Инспектор заводит дефект.
- Если автор согласен с дефектом, он его исправляет, после чего инспектор закрывает этот дефект.
- Если автор не согласен с дефектом, то он устно обсуждает этот дефект с инспектором. Никакие комментарии к дефекту не добавляются.
- Если автор с инспектором успешно договорились, то, в зависимости от договоренностей, либо автор исправляет дефект, либо инспектор его удаляет.
- Если же началась священная война, в которой ни одна из сторон не желает уступить, то на помощь призывается арбитр, который рассудит инспектора с автором.
А что это вы тут делаете?
Перед проведением инспекции обязательно надо удостовериться в том, что вы точно понимаете решаемую инпектируемым кодом задачу. Звучит, как очередное откровение Капитана Очевидность, но практика показывает, что часто инспектор механически делает свою работу, смотрит на соблюдение формальных стилистических требований, не вникая в задачу. Код может быть сколько угодно «красивым», иметь все характеристики SOLID, обрабатывать все возможные ошибки, но если он не решает поставленную задачу, то этот код годится разве что для демонстрации на очередном собеседовании.
Какая гадость эта ваша заливная рыба!
Перед проведением инспекции, пока взгляд не замылен, подумайте, как бы вы решили поставленную задачу? Если вы вдруг обнаружите, что ваше решение существенно лучше предоставленного на инспекцию, не стесняйтесь заявить об этом автору. Пусть лучше он сейчас его исправит, пока это еще не так дорого и оно не обрасло костылями.
На вкус и цвет все фломастеры разные
При заведении дефектов держите себя в руках и не скатывайтесь до вкусовщины. Вкусовщина ведет к конфликтам, потерянному времени на священные войны и не конструктивна. Избежать её очень просто: перед заведением дефекта нужно подготовить набор веских аргументов, почему этот дефект необходимо исправить. Аргументы типа «мне так больше нравится» или «так красивее» вескими не считаются. Если никаких других аргументов не осталось, то не надо заводить дефект, пусть код останется в «авторском» варианте, даже если бы вы написали его иначе.
А если бы у него в кармане была граната?
Обращайте внимание на альтернативные сценарии. Часто программист сконцентрирован на основном потоке выполнения программы и забывает корректно обработать альтернативные. Вам, как лицу не погруженному в задачу, легче заменить подобный прокол.
О пользе обоняния
Все дефекты имеют свои неповторимые запахи, которые здорово помогают их обнаружить. Запахи архитектурных дефектов хорошо описаны в книге Мартина Фаулера «Рефакторинг», которая обязательна для прочтения любым инспектором кода. Но не только архитектурные дефекты пахнут, свои нотки имеют также и алгоритмические. Например, если многопоточый код в блокировке обращается к другому коду, из которого может быть осуществлен callback, или используется несколько объектов синхронизации, то пахнет дедлоком. Запахи алгоритмических дефектов являются хорошей темой для отдельной статьи.
P.S.: Очень большая просьба делиться в комментариях своим опытом инспектирования кода. Что бы вы посоветовали для эффективного инспектирования?