Код ревью. Почему это так трудно

25 Sep 2013

Решил разбавить тематику и написать что-то вроде справочника, на который всегда можно дать ссылку и обновить. Некоторые вещи лучше не внедрять без объяснений. Нельзя сказать, завтра вы будете применять agile, парное программирование, tdd, так как это делают все крутые ребята.

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

Причин такого "неадекватного" поведения несколько, попробую хотя бы половину вспомнить и перечислить.

Саморазвитие

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

Знание проекта

Знание проекта, того что в нем происходит, не проходит мимо тебя. Ты знаешь, что делает или сделал сосед. Иначе, можно дать просто права на push всем участникам, и каждый будет писать свой проект. Если вам не повезет, и каждый из участников проекта делает индивидуально большой кусок кода от 0 до 100 процентов, то через полгода вы не сможете вернуть свои глаза на место при необходимости внести даже небольшие изменения. Это равносильно тому, что программиста с улицы взять и посадить писать код. Будут только вздохи, охи, ахи, периодическая ругань. Одни эмоции и переживания, никакого кодинга. Возможна и другая ситуация, что вы железный дровосек без сердца, и можете только рубить лес от точки А до точки Б, не задумываясь о том, зачем этот лес посадили, и почему вы вырубаете именно эту часть леса.

Мы одна команда, одной крови

Рано или поздно возникнет ситуация, когда вы с соседом сделаете одну и ту же вещь по-разному. Сложно сразу что-то такое вспомнить. Хотя бы следующие простейшие примеры.

"$a = %d, $b = %d, $a + $b = %d" , $a , $b , $c ); ?>

$value ) { # code... } for ($i = 0 ; $i < $countElements = $count ($array ); $i ++ ) { # code... } $i = 0 ; $countElements = $count ($array ); while ($i < $countElements ) { # code... } ?>

Со временем даже от таких мелочей проект превращается в помойку. Сначала мы получаем два стиля в разных кусках кода, постепенно они перемешиваются, и...

Со сложными примерами, когда какая-то логика приложения делается по-разному, результат еще более близок к могиле. Вы либо получаете дублирование кода, либо первый результат в более плачевном виде. Например, кто-то бросает эксепшины, кто-то возвращает true/false, кто-то в строковом виде сообщение об ошибке, четвертый создает методы "черные дыры".

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

Звезда по имени Я

Проходит звездность, приходит профессионализм. Это чаще всего заметно на "новичках". Всю жизнь ты писал так и никак иначе, а тут тебе на ревью: "что за ерунда?". Главное правильно отнестись к этому. Через это все прошли, никто не родился архитектором, никто не получил опыт при рождении.

Банальности

В заключении стоит привести какие-то очевидные плюсы:

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

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

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

Очень часто, вы что-то прочитаете, услышите, а применить и самому написать некогда. Большая вероятность, что сосед уже это пишет и вам нужно только просто глянуть;).

Полезные ссылки

Вообще, в сети много статей разных на эту тему. Но одна ссылка мне очень понравилась. Там наверняка есть все, что я забыл рассказать, в тезисах.

Posted by fightmaster code reviewclean codehandbook Категории clean code, code review, handbook

Мы говорили о недавнем внедрении в работу особого типа инженерных практик, а именно о Code Review. Думаю, не лишним будет вкратце напомнить, что под Code review мы понимаем систематическую проверку кода с целью обнаружения ошибок, недочетов, расхождений, не замеченных на начальной стадии разработки. Инспекцию кода проводят периодически так называемые ревьюеры, при этом ими контролируется не только сам код (его соответствие стандартам, корректность и прочее), но и общая логика реализации задачи.

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

В подтверждении своих слов ниже приведу описание положительных тенденций, которые мы пронаблюдали после введения Code Review.

1. Обнаружение большего числа дефектов до передачи в QA стало возможным в силу нескольких причин:

  • Быстрая работа над ошибками. Если ревьюер знает разработчика, имеет опыт работы с его кодом, то без особых усилий может “отловить” характерные ошибки своего коллеги;
  • Опыт. Ревьюер, будучи более подкованным и опытным программистом, вполне возможно, уже сталкивался с решением подобной задачи и знает потенциально слабые места программного кода;
  • Взгляд со стороны. Каждый может пропустить ошибку, а повторный просмотр снижает вероятность ее наличия.
  • Дополнительная мотивация. Если разработчик знает, что код будет просматриваться, то сам вполне заинтересован в том, чтобы допускать как можно меньше ошибок.

2. Улучшение качества кода за счет обсуждения реализации между разработчиками:

  • Логика и подходы к решению задачи, алгоритмы, имена сущностей и другие детали работы – все можно обсудить и найти наиболее подходящий вариант, отвечающий непосредственно данной проблеме;
  • Разработчик видит не только свой код, но и код коллеги, может сравнить их и при необходимости использовать в своей работе.

3. Упрощение поддержки кода стало возможным, благодаря совместной работе над ним.

4. Code Review как инструмент обучения сотрудника

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

Со своей стороны, отвечая на вопрос о целесообразности ведения постоянной инспекции кода, с чистой совестью говорю: “надо, товарищи! “ Поскольку результат от таких проверок с лихвой окупит потраченное время.

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

Из первых рук: мнения разработчиков о Code Review

“Я участвовал в проекте, предусматривавшем перекрестный ревьюинг кода: два разработчика, соответсвенно, просмотр кода друг друга. Скажу честно: удобно, нужно, нетрудозатратно. Если в цифрах, то поначалу ревьюинг обнаруживал примерно 10-15 ошибок, а затем 0-5 ошибки на один и тот же объем кода. А по времени в день весь процесс занимал не более 20 минут, поэтому, как мне кажется, смысл в такой работе есть точно.”

Алексей Романенко, PHP-разработчик

Константин Медведев, Project Manager

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

Николай Джулай, Project Manager

  • Перевод
  • Tutorial

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

Так что у меня случилось откровение: если это работает для кода, то почему не будет работать в романтичных отношениях? Итак, встречайте новую электронную книгу, которая поможет программистам в отношениях со своими возлюбленными (обложка на иллюстрации слева).

Моя революционная книга обучит вас проверенным техникам по выявлению максимального количества недостатков в своём партнёре. Книга не затрагивает следующие области:

Обсуждение проблем с сочувствием и пониманием.
Помощь партнёру в устранении недостатков.

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

Как вам нравится такая книжка? Предполагаю, что она вам не очень по душе.

Так почему мы проводим code review таким образом?

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

Я собираюсь сделать смелое предположение, что вам хотелось бы улучшить code review в настоящем, где вы работаете с людьми, а не с роботами. Даже более смелое предположение, что хорошие отношения с коллегами - это главная цель, а не просто переменная для ускорения исправления ошибок (минимизации параметра cost-per-defect). Как бы изменились ваши методы ревью с учётом этих обстоятельств?

В этой статье обсудим техники, которые предполагают, что code review - не только технический, но и социальный процесс.

Что такое code review?

Термин “code review” может означать разные действия, от простого чтения какого-то кода из-за спины разработчика до совещания на 20 человек, где вы разбираете код строчка за строчкой. Я здесь имею в виду формальную и письменную процедуру, но не отягощённую рядом совещаний для инспекции кода.


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

Ревью начинается, когда автор отправляет список изменений рецензенту. Оно происходит в несколько раундов . Каждый раунд - это один полный цикл приёма-передачи между автором и рецензентом: автор присылает изменения, рецензент отвечает отзывом на эти изменения. Каждая рецензия кода состоит из одного или нескольких раундов.

Ревью завершается, когда рецензент одобряет изменения. Обычно этому сопутствует отправка сообщения LGTM, сокращённой фразы “looks good to me”.

Почему это так трудно?

Если программист присылает вам список изменений, которые он считает потрясающими, а вы отвечаете ему подробным перечислением причин, почему он не прав, такое общение требует определённой деликатности.
«Это одна из причин, почему я не скучаю по IT, ведь программисты - весьма малопривлекательные люди… Например, в авиации те, кто слишком переоценивают свои способности, уже мертвы».

Филип Гринспан, сооснователь ArsDigita, из книги «Основатели за работой ».


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

Как будто и без этого недостаточно сложностей, здесь вами приходится выражать свои мысли в письменной форме, что ещё больше повышает риск недопонимания. Автор не слышит тон вашего голоса, не может воспринять язык тела, поэтому так важно аккуратно подбирать формулировки. Для автора, который занял оборонительную позицию, безобидное примечание вроде «Ты забыл закрыть файловый дескриптор» может звучать как «Не могу поверить, что ты забыл здесь закрыть файловый дискриптор! Ты такой идиот!»

Техники

Отдайте компьютерам скучную часть работы

Когда вас постоянно отвлекают совещания и почтовые письма, трудно выкроить время для вдумчивого анализа кода. Ментальных сил ещё меньше, чем времени. Чтение кода коллеги - когнитивно тяжёлая задача и требует высокого уровня концентрации. Не транжирьте эти ресурсы на задачи, которые может выполнить компьютер, особенно если он выполняет их лучше.

Очевидный пример - ошибки с пробелами в отступах. Сравните, сколько усилий требует выявление такой ошибки от человека, по сравнению с применением автоматического инструмента форматирования:

Правая часть пустая, потому что автор использовал редактор кода, который автоматически форматирует пробелы каждый раз при нажатии кнопки «Сохранить». Ну худой конец, когда автор отправляет свой код на проверку, система непрерывной интеграции сообщает о неправильных пробелах. Автор исправляет проблему ещё до того, как рецензент её заметил.

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

Задача Автоматизированное решение
Проверить билды Travis или CircleCI .
Проверить прохождение автоматических тестов Система непрерывной интеграции, такая как Travis или CircleCI .
Проверить, что отступы и пробелы соответствуют корпоративному стилю. Инструмент форматирования кода, такой как ClangFormat (для C/C++) или gofmt (для Go).
Идентификация неиспользуемых модулей или неиспользуемых переменных Статические анализаторы кода, такие как pyflakes (линтер для Python) или JSLint (линтер для JavaScript).

Автоматизация помогает вам как рецензенту внести более ценный вклад. Если игнорировать целый класс проблем, вроде порядка импорта модулей или соглашения о присвоении имён для файлов исходников, то вы можете сконцентрироваться на более интересных вещах, таких как функциональные ошибки или недостаточная читаемость кода.

Автоматизация выгодна и автору. С её помощью он может обнаружить небрежные ошибки за несколько секунд, а не за несколько часов. Мгновенный фидбек ускоряет обучение и упрощает исправление ошибки, потому что релевантный контекст у автора ещё в рабочей памяти. Кроме того, автор гораздо легче воспринимает сообщение о глупой ошибке от компьютера, а не от вас.

Нужно всем вместе поработать, чтобы внедрить автоматические проверки такого рода в рабочий процесс code review (например, хуки перед коммитами в Git или вебхуки в GitHub). Если процесс ревью требует от автора запускать такие проверки вручную, то вы лишаетесь большей части преимуществ. Автор неизбежно иногда будет забывать о проверке, из-за чего вам придётся возиться с с простыми ошибками, которые могла исправить автоматическая проверка.

Оформите аргументы по стилю в виде руководства по стилю

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

Хорошее руководство по стилю определяет не только внешние элементы оформления вроде соглашения о присвоении имён или правила отступов, но и как использовать функции данного языка программирования. Например, JavaScript и Perl набиты функциональностью - там есть много вариантов реализации одной и той же логики. Руководство по стилю определяет Один Правильный Способ программирования, так что вы больше не увидите, что одна половина вашей команды использует один набор функций языка, а другая половина - совершенно другой набор функций.

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

Вариант 1: адаптация существующего руководства по стилю
Если поискать в интернете, то можно найти опубликованные руководства по стилю - и позаимствовать их для собственного использования. Самые известные - руководства по стилю от Google , но можно найти и другие, если эти не подходят. Адаптируя существующий документ, вы получаете все выгоды от наличия руководства по стилю, не тратя времени на написание такого документа с нуля.

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

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

Я предпочитаю держать наше руководство в формате Markdown в системе контроля версия (например, на GitHub Pages). Так все изменения проходят через стандартную процедуру рецензирования - кто-то должен явно одобрить изменения, а каждый в команде может поднять любую проблему. Вики и Google Docs тоже подходят.

Вариант 3: Гибридный подход
Сочетая варианты 1 и 2, вы можете адаптировать существующее руководство и одновременно вести локальный документ для расширения или перезаписи базы. Хороший пример - руководство Chromium C++. Там в качестве базы взяли руководство по C++ от Google, но сделали собственные изменения и дополнения.

Начинайте ревью немедленно

Расценивайте code reviews как задачу с высоким приоритетом. Когда вы изучаете код или пишете отзыв, не торопитесь, но начинайте делать это немедленно - в идеале, в течение нескольких минут.

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

Если вы начинаете ревью немедленно, то создаёте благоприятный цикл. Весь документооборот становится исключительно функцией размера и сложности списка изменений автора. Это побуждает авторов присылать небольшие, точно сформулированные списки. Их легче и приятнее рассматривать, так что ваши ревью ускорятся, а благоприятный цикл продолжится.

Представьте, что ваш коллега реализовал новую функцию, которая требует изменения 1000 строчек кода. Если он знает, что вы можете сделать ревью списка изменений на 200 строчек примерно за два часа, то разобьёт эту функцию на фрагменты примерно по 200 строчек и закончит ревью всей функции за один-два дня. Но если любые код ревью занимают у вас целый день, независимо от размера, то утверждение функции займёт неделю. Ваш коллега не хочет ждать целую неделю, так что он скорее направит вам более крупные списки, по 500-600 строк каждый. Их труднее рассматрвиать, да и отзывы станут беднее, потому что тяжелее сохранить контекст для изменения на 600 строк, чем на 200.

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

Начните с высокого уровня, и спускайтесь ниже

Чем больше заметок вы пишете в каждом конкретном раунде, тем больше риск, что автор будет чувствовать себя подавленным. Точный лимит зависит от разработчика, но опасная зона обычно начинается в районе 20-50 заметок на один раунд ревью.

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

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

Щедро используйте примеры кода

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

Хороший способ улучшить восприятие код ревью автором - найти возможность подарить ему подарок. А какие подарки любят получать все разработчики? Конечно, примеры кода.

Urls = for path in paths: url = "https://" url += domain url += path urls.append(url)
Ответ в стиле «Можно это упростить с помощью списковых включений?» вызовет раздражение, потому что ему нужно потратить 20 минут, изучая функцию, которую никогда раньше не использовал.

Предлагаю рассмотреть списковое включение такого рода:

Urls = ["https://" + domain + path for path in paths]


Эта техника не ограничивается однострочными изменениями. Я часто создаю собственную ветку кода для демонстрации автору большой концептуальной идеи, такой как разбиение крупной функции или добавление юнит-теста для покрытия дополнительной граничной ситуации.

Применяйте эту технику только для ясных, бесспорных улучшений. В вышеприведённом примере спискового включения немногие разработчики будут спорить с сокращением количества строк кода на 83%. И наоборот, если вы напишете пространный пример для демонстрации изменения, которое «лучше» на ваш личный вкус (например, изменения стиля), то такой пример кода создаст впечатление, что вы настырно проталкиваете свои предпочтения, а не проявляете щедрость.

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

Никогда не говорите «ты»

Это звучит немного странно, но послушайте: никогда не обращайтесь лично к автору в процессе код ревью.

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

Подбирайте слова для отзыва таким образом, чтобы минимизировать риск возникновения оборонительной позы. Чётко обозначьте, что вы критикуете код, а не самого разработчика. Когда автор видит в комментарии личное обращение к нему, это отвлекает его внимание от кода и переводит внимание на личность. Так повышается риск, что он воспримет критику лично.

Например, вот безобидный комментарий:

Ты допустил ошибку в слове «благополучно».

Автор может интерпретировать такое замечание двумя соврешенно разными способами:
  • Интерпретация 1 : Эй, приятель! Ты неправильно написал слово «благополучно». Но я всё равно считаю, что ты умный! Вероятно, это просто опечатка.
  • Интерпретация 2 : Ты допустил ошибку в слове «благополучно», придурок.
Сравните это с замечанием, в котором отсутствует обращение «ты»:
sucessfully -> successfully

Вторая заметка - это просто исправление, а не оценка автора.

К счастью, можно легко переписать свои комментарии, избегая слова «ты».

Вариант 1: Замените «ты» на «мы»

Ты можешь дать этой переменной более наглядное имя, вроде seconds_remaining ?
превращается в:
Мы можем дать этой переменной более наглядное имя, вроде seconds_remaining ?

«Мы» усиливает коллективную ответственность всей команды за код. Автор может перейти в другую компанию, как и вы, но в том или ином виде останется команда, которая отвечает за этот код. Может показаться глупым говорить «мы», когда речь идёт об изменении, которое автор явно должен сделать единолично, но лучше выглядеть глупым, чем обвинителем.

Вариант 2: Удалите из предложения субъект
Другой вариант избежать личного обращения - использовать сокращение, которое исключает из предложения субъект:

Надо подумать о переименовании переменной для более наглядного имени, вроде seconds_remaining .

Того же эффекта можно достичь с помощью пассивного залога. Я обычно в технических текстах избегаю пассивного залога как чумы, но это может быть полезный вариант обхода проблемы с личным обращением:
Эта переменная должна быть переименована для более наглядного имени, вроде seconds_remaining .

Ещё один вариант - перефразировать предложение в виде вопроса, который начинается со слов «Что насчёт...» или «Как насчёт...»:
Что насчёт переименовать эту переменную для более наглядного имени, вроде seconds_remaining .

Оформляйте отзывы как запросы, а не команды

Код ревью требует большего такта и осторожности, чем обычное общение, потому что здесь выше риск, что обсуждение скатится в личный спор. Казалось бы, рецензенты должны проявлять бóльшую вежливость и учтивость в код ревью по сравнению с личным общением. Но я обнаружил страннейшим образом прямо противоположную ситуацию. Многие люди никогда не скажут коллеге, «Дай мне этот степлер, а потом принеси газировки». Но я видел множество случаев, когда рецензенты оформляют отзывы в таком командном стиле, вроде «Перенеси этот класс в отдельный файл».

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

Сравните одно и то же замечание, оформленное двумя разными способами:

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

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

Сравните, насколько воинственной кажется беседа в зависимости от оформления первоначального замечания:

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

Обосновывайте принципами, а не мнениями

Когда составляете замечание для автора, то объясните и предлагаемое изменение, и его причину . Вместо фразы «Нам следует разделить этот класс на два» лучше сказать «Сейчас этот класс отвечает одновременно и за скачивание файла, и за его парсинг. Следует разделить его на класс для скачивания и на класс для парсинга согласно принципу единой ответственности ».

Если обосновывать замечания принципами, то дискуссия принимает конструктивную форму. Когда вы цитируете конкретную причину, вроде «Нужно сделать эту функцию приватной, чтобы минимизировать открытый интерфейс класса», то автор не может просто ответить «Нет, я предпочитаю сделать по-своему». А если он так ответит, то это будет выглядеть глупо, потому что вы показали, как изменение позволяет достичь цели, а он просто заявил о своём предпочтении какому-то способу.

Разработка программного обеспечения - это одновременно искусство и наука. Не всегда можно точно сформулировать, что именно неправильно в данном фрагменте кода с точки зрения устоявшихся принципов. Иногда код просто некрасивый или излишне усложнённый, и сложно точно сформулировать, почему. В таких случаях объясните как можете, но сохраняйте объективность. Если сказать «Мне кажется, это сложно понять», то это хотя бы объективное утверждение, в сравнении с фразой «Это запутанный код», что является оценочным суждением, с которым не каждый может согласиться.

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

Часть 2: скоро

Через пару недель я опубликую вторую часть статьи. Будьте на связи, там рассмотрим дополнительные советы, в том числе:
  • Работа со слишком большими код ревью.
  • Распознавание возможности дать высокую оценку.
  • Соблюдение рамок ревью.
  • Действия в безвыходных ситуациях.
UPD. Опубликована вторая часть статьи. Добавить метки

Купить "Мольба " можно за 250 руб.

О фильмах "Мольба", "Покаяние"
Тенгиз Абуладзе знает скверную механику беззаконной власти, ее закулисье, все эти внезапные разносы подчиненным за правонарушения, все эти ухмылки, с которыми их, черт с ними, прощают (сцена грозы над Доксопуло, который в нерве подслушивает, удастся ли благоволящей ему секретарше утишить гнев Вар-лама), — но он знает про беззаконие и нечто иное и большее.

Беззаконие (так движется мысль в "Покаянии") не только посягает на чьи-то жизни, ломает их и отнимает их; беззаконие есть посягательство на жизнь в высшем смысле этого слова.

Беззаконие проедает бытие. Если в суде оказываются люди в мантиях и в париках, если на всадниках, спешившихся во дворе, где стоит грузовик с арестантами, оказываются латы и кольчуги, если в разоренном храме звучит по радио речь Эйнштейна и по своим делам шмыгает какой-то выходец из Босха в наряде XV века и с тяжелым крысьим хвостом, — это не каприз режиссера и не карнавал персонажей, а знак того, как беззаконие разъело время. На улицах города, взятого Варламом и им терзаемого, вполне могут скакать всадники, которые вырезали Грузию при монголах или при турках; злу открылись щели в проеденном времени; старое зло в щели лезет, всплывает со дна времен, хватает живых.

Фильм "Покаяние" сделан в глубоком сознании того, что преступление смущает мир и заражает его смутой. [...]

Беззаконие наполняет жизнь тем, что не может, не должно ее наполнять, для чего прежде надо вытеснить ее природное, наследственное наполнение. Варлам знает, что делает, когда в старом храме, где еще можно разглядеть на стенной росписи "Изгнание из рая" и "Уверение Фомы", оказываются вбиты какие-то гигантские штыри с подвешенными на них гроздьями поблескивающих медных шаров и тарелок и громоздится чудная аппаратура.

Варлам также знает, что он делает, когда преследует чету Баратели: в союзе Сандро и Нино есть то, на чем мир стоит, держится; в нем наполненность должным. Это люди чести, люди семьи, люди товарищества, люди отечества, на земле которого стоит старый храм. [...]

В систему творческих импульсов Тенгиза Абуладзе входят и живая образность дома, и корневая образность народной памяти, и грозная и гротескная образность "потрясенного естества". Свобода, с которой эти импульсы соединяются, удивительна. Какой-то слой памяти (нашей, зрительской) воспринимает, до чего же "те самые" — эти составленные в пустой комнате праздничные круглые транспарантики с портретом, один из которых отчаянно ломает и топчет Нино; какой-то иной, несравненно более глубокий слой памяти воспринимает традиционность древнего жеста моления, когда бедная женщина тут же склоняется перед самим вошедшим владыкой; но память может и не проснуться, все равно захватит суть: суть волнения и горя, суть верности, суть немилосердия. Среди творческих импульсов, властных над создателями этой картины,— ощущение кровной близости. Не то, чтобы Тенгиз Абуладзе стремился засвидетельствовать: вот как было с нами. Напротив, в "Покаянии" есть разгадывание общей модели, модели разлада мира, в котором обосновывается беззаконие и истребляется то, на чем мир стоит. Но как, разгадывая такую модель, не думать о своем.

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

Вряд ли следует видеть в образности "Покаяния" образность, рождающуюся по законам лирического творчества — самораскрытия, поэтического самоизъявления. Вольность и острота образных сцеплений и ассоциаций в "Покаянии" иные по своей природе. Дар Тенгиза Абуладзе прежде всего в органике и силе воображения, откликающегося на нечто, что вне художника; в органике и силе вживания в чужое, не с ним лично бывшее. Он не изживает в творчестве собственные свои, преследующие его и ищущие воплощения образы и состояния, а вживается, проникаясь тем состраданием и ужасом, которые, по закону трагической поэзии, должны очищать душу. Режиссер "Покаяния" отдает себя боли и муке других так, как актер школы переживания отдает себя боли и муке того другого, кем предстает на сцене. Не свое состояние "до-сценическое", "до-экранное" выражает, а вникает в чужое, проникается чужим.

Творческая память Абуладзе — память, принимающая то, что ей передано, наследующая; в известной мере сыновняя память.

В русском фольклоре есть постоянное обращение — "отецкий сын"; "добрый молодец, отецкий сын". Казалось бы, образ не содержателен — кто ж без отца. Но по достоинству и вежеству в мужчине узнают воспитавшего его мужчину, узнают перенятый и оберегаемый закон. Человек не безроден. В этом-то смысле Тенгиз Абуладзе — "отецкий сын", и "отецким сыном" предстает в его картинах всякий, кто достоин уважения, живет ли он в героическом древнем доме-крепости, как в "Мольбе", или в завешанной холстами квартире, как в "Покаянии". [...]

В природе вещей, как ее понимает Важа Пшавела и как ее понимает Тенгиз Абуладзе, снимая "Мольбу" по его поэмам, есть трагизм; трагизм — природное наполнение жизни. Человек угрожает юной плакальщице, пробирающейся на кладбище; горит сжигаемый односельчанами дом Алуды, и истомленный горной дорогой отверженец с женой и детьми то ли исчезает, то ли приближается к нам в леденящем тумане. В трагедии человек может погибнуть, но не может исказиться, как не искажается и природа вещей. Искаженность караулит где-то рядом — там, где накладывает лапу ухмыляющийся собеседник поэта, его туго знающий свое дело антагонист, возникающий в первых же кадрах "Мольбы" и почесывающий за разговором голое пузо. […]

[...] Шут в фильме "Покаяние", как и в "Мольбе", на святом месте выламывается, ломает комедию. Именно что ломает и выламывается — это его дело. Шут против творца (знающие это дело подтвердят, что и в русской средневековой культуре улыбка есть атрибут неба, ад озвучен хохотом басов. Шутом, чтобы не поминать его истинного имени, называют того же, кого называют нечистым и лукавым). [...]

Не доказательству того, что негодяи — отпетые плуты, и даже не разгадке естества, потрясенного преступлением, служит картина. А уж скорее тому, о чем сказано во фреске "Уверение Фомы", которую разглядывала девочка Кето. Можно задуматься над ее сюжетом: апостол трогает язву, смерть от которой неминуема, и удостоверяется, что перед ним живой; будь стоящий перед апостолом мертв, самая мысль влагать персты в язвы мертвого не могла бы прийти.

Тенгиз Абуладзе своим фильмом удостоверяет, что мы живы. [...]

СОЛОВЬЕВА И. Королевская мысль // ИК. 1987. № 8.

У вашего проекта сменился разработчик и он говорит, что старый код невозможно использовать? Новая команда тратит много времени на решение простых задач? Как только удаётся справиться с одной проблемой, тут же ломается что-то другое? Скорее всего, проблема в качестве кода.

Что такое качественный код

Не существует точного определения этого термина. Как правило, понимание того, как должен выглядеть качественный исходный код, основывается на многолетнем опыте специалиста. Некоторые программисты придерживаются абстрактного принципа KISS, который расшифровывается как Keep It Simple, Stupid! («Делай это проще, тупица!»). Отчасти этот метод проектирования справедлив, так как отражает главное правило хорошего кода — простота и ясность. Однако простоту часто путают с упрощением, поэтому о качестве исходного кода в профессиональной среде судят ещё по нескольким свойствам:

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

Чтобы облегчить понимание кода в профессиональной среде, у каждого языка программирования есть свой Code Style — стандарт оформления. Именно он диктует правила: где ставить пробелы или скобки, как отделять строки или называть переменные. Может показаться, что эти нюансы не так важны, однако их соблюдение значительно облегчает понимание кода для тех, кто видит его впервые.

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

Как повысить качество кода?

Одна из самых популярных и при этом довольно простых в реализации техник носит название Code Review. Её смысл в том, чтобы любые изменения, вносимые программистом, попадали в основное хранилище кода и в релизную версию ПО только после того, как их проверят остальные участники команды.

Этот процесс состоит из нескольких этапов.

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

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

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

Плюсы Code Review

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

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

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

Благодаря Code Review снижается так называемый bus-фактор, или «фактор автобуса». Так называют число, означающее количество участников команды, которых должен сбить автобус, чтобы все знания о проекте были потеряны. К примеру, в проекте занято четыре человека, если два из них по каким-то причинам уйдут, то оставшиеся смогут закончить работу, а если команду покинут трое — последний участник не справится в одиночку.

Минусы Code Review

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

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

Когда использовать Code Review?

Процесс Code Review достаточно прост, а его плюсы заметно преобладают над минусами, но в ряде ситуаций вы легко обойдетесь без него.

К примеру, нет смысла проводить Code Review при разработке прототипа или MVP — минимально жизнеспособного продукта. Главная задача такого проекта — получить от пользователей обратную связь, чтобы построить гипотезы для дальнейшего развития. Структура этих приложений делается максимально простой, и в дальнейшем код всё равно предстоит переписывать кардинальным образом.

Ещё Code Review не нужен в работе над простыми приложениями, которые делаются раз и навсегда. Так что если вы не планируете в будущем изменять или дорабатывать свой проект, можно сэкономить время.

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

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