5 советов по проведению хорошего обзора кода перевод

Обзор кода является одной из самых ценных инженерных практик.

   1. Обзоры кода улучшают качество кода: одна голово хорошо, а две — лучше.
   2. Обзоры кода — это прекрасный инструмент для изучения разработчиками тех частей приложения, которые они в дальнейшем могут сопровождать.
   3. Обзоры кода помогают узнавать лучшие практики от других разработчиков.
   4. Обзоры кода могут использоваться для проверки понятности и простоты всего приложения в целом.

Как вы могли заметить, в этом списке я не упомянул, что проведение обзоров кода помогает находить ошибки и соблюдать стандарты кодирования, и вот почему:

   1. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью поиска ошибок.
   2. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью проверки соблюдения стандартов кодирования.

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

Исходя из этой точки зрения, позвольте вам дать 5 советов по проведению хорошего обзора кода.

1. Проводите обзоры кода часто.


В моей работе были случаи, когда я страдал от того, что обзор кода был проведен слишком поздно. Я работал в компании, где масштабный обзор кода всего приложения был последней стадией разработки. Это было абсолютно бесполезно, поскольку поздние обзоры кода имеют следующие недостатки.
  • Чем больше кода необходимо просмотреть, тем больше вероятность того, что разработчик не согласится на последующий рефакторинг.
  • Чем дольше разработчик занимается каким-то участком кода, тем более вероятно, что этот участок становится «персональным». Я бы хотел напомнить, что одна из главных проблем разработчиков — это их эго. Если разработчик написал код, который работает, то несмотря на то, что этот код может быть плохим, программистское эго воспротивится любым предложениям по улучшению кода.
  • Чем ближе дата релиза, тем менее приоритетными становятся работы по улучшению кода.

Лично я делаю обзор кода каждый раз после того как закончил писать что-нибудь нетривиальное — как правило, несколько раз в день.

2. Сделайте обзоры быстрыми и неформальными.


Забудьте про списки, просто повернитесь и попросите кого-то из коллег уделить вам 5 минут. Если же у вас есть список, то что-нибудь из этого обязательно произойдет:

   1. В ходе обзора будет просмотрено только то, что указано в вашем списке.
   2. Обзоры кода превратятся в формальность. Люди будут как можно меньше времени просматривать код и притворяться, что им это действительно надо.

Короткие и неформальные обзоры — это по сути диалоги, в которых человек не чувствует, что его рецензируют, скорее ему просто советуют.

3. Проводите обзоры кода с разными людьми.


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

   1. Полезно услышать разные точки зрения.
   2. В дальнейшем ваш код сможет сопровождать большее количество людей.
   3. Повышается командное взаимодействие.

4. Настройтесь на позитивный лад.


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

5. Научитесь получать удовольствие от проведения обзоров.


Это самый важный совет. Если вы работаете в команде, в которой люди любят проводить обзоры, то все будут вовлечены в написание кода, который нравится не только автору, но и всей команде.

__________
В комментариях можно поделиться, проводите ли вы обзоры кода, как у вас это происходит, есть ли какой-нибудь интересный опыт и т.д.
+26
9 апреля 2010, 10:55
47
andreycha 39,8

комментарии (45)

0
habralan #
пока я не перевёл «обзоры кода» на английский, я не понял, о чём вообще идет речь… как-то это звучит… подозриительно :)
0
andreycha #
Считайте это прихотью переводчика :). Я не очень люблю сплошное употребление англицизмов, и считаю, что если возможно обойтись русским словом — нужно использовать именно его. А тем более «обзор» — прекрасное слово и даже выдумывать ничего не нужно. «Ревью» все же не по-русски звучит, а «ревизия» у меня прочно ассоциируется с системами контроля версий.
+2
andreycha #
Хотя вот Википедия подсказывает, что есть еще слово «рецензия».
+1
VasilioRuzanni #
«Рецензирование кода» это обычно называют по-русски.
0
reito #
Такая фраза искажает смысл. Рецензию на код никто не пишет — продуктом этой деятельности является тот же код, а не какая-то точка зрения на него.
0
VasilioRuzanni #
Ну оно так называется, что тут поделаешь :) К тому же, рецензирование и написание рецензии — не одно и то же, а зависит от контекста.
0
ItGold #
почему бы не обойтись простым английским словом? Англицизмы отстой, и подобные переводы, где еще догадаться нужно, о чем вообще речь, тоже.
0
Nashev #
Обзор — это «Возможность охватить взором, обозревать какое-н. пространство» (с) Ушаков. Я лично поначалу представил себе процесс формирования кем-то схемы взаимосвязей чьих-то модулей, классов и т.п. — типа UML-ных картинок, и их разглядывание этим кем-то — с целью изучения и использования, а не критики и рефакторинга. Весьма неуместное слово. Поменяйте, плиз, на «рецензирование».
+2
stasikos #
Не хватает еще статьи «что такое обзор кода» и «как проводить обзор кода». :) Было бы интересно.
0
andreycha #
Посмотрите Википедию:
ru.wikipedia.org/wiki/Рецензия_кода
Статью в этом же блоге:
habrahabr.ru/blogs/code_review/88075/

«Как проводить» — частично описано в данном топике. Действительно, самое простое — попросить человека уделить вам 5 минут. Другое дело, что народ должен быть искренне заинтересован в этом.
+1
stasikos #
Я уж было подумал что это разные вещи. Все таки ревизии и рецензии кода это поиск именно ошибок, а вы говорите что нужно проводить обзор кода не с целью их поиска.
+3
smind #
> 1. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью поиска ошибок.
не понял что то я этого… ведь я произвожу ревизию кода в основном чтобы увидеть потенциальные ляпы (ошибки) которые пропустил выложивший…
0
andreycha #
Думаю, автор статьи намекает, что сейчас, когда есть автоматические тесты, акценты немного сместились в другую сторону.
+1
d0z #
Думаю, что вы не видели их в действии, ну или не знаете что результат далек по качеству и от просмотра кода 3-4 профессионалами.
0
andreycha #
Уважаемый, я не высказывал свое мнение, а лишь предположил, что имел в виду автор статьи.

А если вы хотите услышать мое мнение, то я не считаю, что разнообразное автоматическое тестирование — это панацея. Точно так же, как и просмотр кода 3-4 профессионалам не отменяет необходимость в тестах.
0
d0z #
последнее бессмысленно отрицать, но инспекция кода как таковая лишь часть процесса в который входит и автоматическое тестирование кода, и юниттестирование и, тестирование продукта, я лишь хотел сказать, что ее не стоит игнорировать. прошу прощения если чем-то задел.
0
reito #
Ох, вот бы мне 3-4 профессионала, чтоб мой код просматривали по вечерам, пока я до дома еду. :)
–3
alexs0ff #
Для целей поиска ошибок, нужно использовать юнит-тестирование.
0
smind #
ок, я все понимаю, но как написать тест для «в форме при введенной маске поиска „*\1“ происходит лишнее экранирование \1», адекватно ли время написания подобного теста с патчем в 1 строку…
0
burivuh #
Для поиска ошибок нужно использовать системный подход к тестированию. Юнит-тесты не панацея. И обзоры кода не панацея. Нужно использовать различные методики вместе, тогда можно добиться лучших результатов.
+1
smind #
У нас в команде принято производить обзор нового кода перед его влитием, без ревизии 2-х разработчиков нельзя производить влитие кода в основной ствол.
+1
andreycha #
Вот это круто.
Как бы еще людей заставить ответственно подходить к делу, а не по принципу глянули одну минуту, «Да, нормально, заливай»?
НЛО прилетело и опубликовало эту надпись здесь
0
icoder #
Как звучит! «влитие кода в основной ствол»
0
timurv #
Занимаюсь темой code review тоже, ищу инструменты для автоматизации.

Пока на выбор:
* howsmycode.com/
* www.reviewboard.org/

Было бы здорово, если бы кто нибудь посоветовал работающую схему.
0
alexs0ff #
Code collaborator smartbear.com/codecollab.php
Не видел ничего лучше.
0
timurv #
Он действительно на столько хорош, что можно его купить и потратить время на интеграцию с git?
Из коробки на сколько я понял, он имеет только консольный клиент, работающий с git.
0
alexs0ff #
Да хорош, после reviewboard смотрится как небо и земля.
0
smind #
не понимаю как обзор кода можно автоматизировать… какой именно этап имеется ввиду?

я это делаю примерно так — качаю брнч подготовленный к ревью, собираю, смотрю наличие варнингов (их быть не должно)
просматриваю коммиты по очереди, их как правило не много, не больше 5-10, смотрю появившиеся новые объекты которые возможно не освобождены в нужном месте, смотрю на-сколько код красив (не в плане отступов, а вообще его оптимальность), ставлю подпись если все устраивает и бранч соответствует заявленным функциям.
0
alexs0ff #
Автоматизация — это введение инструментов для облегчения code review. По идее, в самом простом варианте можно перед комитом подходить к разработчику и смотреть в монитере его изменения.
0
timurv #
Как раз ищется инструмент, чтобы просматривать коммиты, комментировать коммиты построчно, ставить подпись.
0
conf #
Смотрите мой перевод и комменты к нему, там про инструменты тоже есть.
0
smind #
я пользуюсь mc для просмотра кода и изменений в нем
вот как тут на картинке funkyimg.com/u2/310/066/ydiff_PNG.png
0
pento #
Это называется «анализ кода».

> 1. Обзоры кода улучшают качество кода: одна голово хорошо, а две — лучше.
> 3. Обзоры кода помогают узнавать лучшие практики от других разработчиков.

0
StrangeAttractor #
> 1. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью поиска ошибок.
> 2. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью проверки соблюдения стандартов кодирования.
> 10 лет назад эти два пункта имели бы смысл в обзорах кода. Однако сейчас вы должны использовать
> автоматические средства тестирования и инструменты, следящие за оформлением кода.

Можно примеры инструментов, следящих за оформлением кода?
0
andreycha #
ReSharper, StyleCop. Во втором, насколько я знаю, больше база «правил».
0
andreycha #
Так же есть плагин, который скрещивает эти два инструмента:

stylecopforresharper.codeplex.com/
+1
d0z #
позволю добавить выжимку из правил компании в которой я работаю:
— сама инспекция кода должна производится до митинга. На митинге допускается только озвучивание и обсуждение ранее выявленых недостатков.
— митинг должен длится не более 2-х часов с обязательным перерывом.(причина интуитивно понятна)
— если объем кода велик и не укладывается в одину инпекцию и его можно разбить на составляющие компоненты, то нужно это сделать и обсуждать их отдельно.
— список лиц ответсвенных присутсвующих на инспекции:
1. разработчик (автор)
2. модератор — человек которы контролирует процесс обзора и выносит решения
3. 3-4 инстпектора — люди которые предварительно проинспектировали код
4. человек который ведет учет(чаще всего это автор).
— ни в коем случае код не правится на инспекции — это самая грубая ошибка.
— если инспектора не подготовились то модератор имеет полное право отметить инспекцию (и это правильно потому что просмотр кода на самом миитнге это лишняя трата времени и сил)
— количество человек на инспекции лучше ограничить 5-7 людьми.
— лог инспеции должен быть в общем доступе(своеобразынй обмен опытом)
0
andreycha #
Спасибо, интересно.

Правильно ли я понял, что вы делаете инспекции довольно объемных участков кода (например, раз в несколько дней или после завершения куска функциональности)?
0
d0z #
Ну в данный момент, в нашем проекте код это динамические библиотеки, потому инспекции делаются для этих либ. Некоторые, наиболее объемные, разбиваются на компоненты (в основном классы) и рассматриваются по частям. Насчет пошагового просмотра кода согласится с автором не могу, но и опровергнуть тоже, такие методы не практиковались и как это работает на деле оценить не могу. В целом я описал лишь общий процесс, для каждой разработки используется свой подход.
0
burivuh #
Очень похоже на описание формальных инспекций у Макконнела в «Совершенном коде», все по-взрослому =)
0
sindrom #
Кстати, как вам идея создать сервис для проведения таких code review? Т.е. некий программист загружает фрагмент своего кода в публичное хранилище, чтобы его оценили другие специалисты, зарегистрированные на этом сайте. Разумеется, надо как-то организовать контроль квалификации оценивающих, но эта проблема решаема.
Такой сервис будет полезен для начинающих джуниоров, которым еще не посчастливилось работать в серьезных организациях, но без оценки и грамотных советов от более мудрых коллег, очень сложно расти в профессиональном плане.
0
Vidmak #
Интересно, govnokod.ru/, только не агрессивный. Только есть проблемы с авторским правом, и нельзя остальное увидеть. А если можно то это www.codeplex.com
0
enej #
Мне больше нравиться идея парного программирования, так как по сути это тоже самое что и обзор кода, вот только лишено некоторых минусов, например:
* не нужно никого просить, у тебя уже есть напарник
* гораздо меньше нужно вносить изменений, потому что ваш напарник сразу же высказывает свои замечания
* дата релиза не давит
Наверное, есть еще, это только те плюсы которые у автора были минусами обзора кода. Хотя не сомневаюсь что этот подход имеет и свои минусы.
0
ParadiseCracked #
У нас это систематизировано с помощью Crucible и открытой возможности джоиниться в review.

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