как проводить код ревю

Как сделать код-ревью быстрее и эффективнее

Как обычно происходит код-ревью? Вы отправляете пул-реквест, получаете обратную связь, вносите исправления, отправляете фиксы на повторный ревью, затем получаете одобрение, и происходит мерж. Звучит просто, но на деле процесс ревью бывает очень трудоемким.

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

Получается, что чем объемнее пул-реквест, тем меньше пользы будет от его проверки.

Как избежать таких ситуаций? Как сделать пул-реквест проще и понятнее для ревьюера и оптимизировать весь процесс?

Переводим статью нашего бэкенд-разработчика Сергея Жука про то, как устроен процесс код-ревью у команды мобильного приложения Skyeng.

Категории изменений

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

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

Стратегии ревью

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

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

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

При проверке остальных категорий в 99 % случаев мерж происходит сразу же.

Почему нужно разделять изменения по категориям?

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

Что точно не стоит смешивать

Переименование/удаление класса и его рефакторинг. Здесь мы сталкиваемся с Git, который не всегда правильно понимает такие изменения. Я имею в виду масштабные изменения, когда меняется много строк. Когда вы рефакторите класс, а затем перемещаете его куда-то, Git не воспринимает это как перемещение. Вместо этого Git интерпретирует эти изменения как удаление одного класса и создание другого. Это приводит к куче вопросов во время код-ревью. И автора кода спрашивают, почему он написал этот уродливый код, хотя на самом деле этот код был просто перемещен из одного места в другое с небольшими изменениями.

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

Любые механические изменения + любые изменения, произведенные человеком. Под «механическими изменениями» я подразумеваю любое форматирование, выполненное с помощью IDE или генерации кода. Например, мы применяем новый code style и получаем изменений на 3000 строк. И если мы смешаем эти изменения с какими-либо функциональными или любыми другими изменениями, произведенными человеком, мы заставим ревьюера мысленно классифицировать эти изменения и рассуждать: это изменение, произведенное компьютером, — его можно пропустить, а это изменение, сделанное человеком, — его нужно проверить. Так ревьюер тратит очень много дополнительного времени на проверку.

Пример

Вот пул-реквест с функцией метода, который аккуратно закрывает клиентское соединение с Memcached:

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

В результате ревьюер должен просмотреть весь код и

1. Рефакторинг: извлечение класса

Здесь всего два файла. Ревьюер должен проверить только новый дизайн. Если все в порядке — мержим.

2. Следующим шаг — тоже рефакторинг, мы просто перемещаем два класса в новое пространство имен

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

3. Удаление лишних блоков док-блоков

Здесь ничего интересного. Мержим.

4. Сам функционал

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

Заключение

Не создавайте огромных пул-реквестов со смешанными категориями изменений.

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

И всегда выполняйте следующие шаги до отправки пул-реквеста:

От редакции: Сергей много пишет интересного про программирование и PHP, а мы иногда что-то переводим: сервер потокового видео, рендеринг HTML файлов. Смело задавайте ему вопросы в комментариях к этой статье — он ответит!

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

Источник

Code review по-человечески (часть 1)

В последнее время я читал статьи о лучших практиках 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, из книги «Основатели за работой».

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

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

Техники

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

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

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

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

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

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

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

Нужно всем вместе поработать, чтобы внедрить автоматические проверки такого рода в рабочий процесс 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 заметок на один раунд ревью.

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

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

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

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

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

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

Предположим, ваш автор не очень хорошо знаком с функцией списковых включений в Python. Он присылает на рецензию код с такими строчками:

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

Автор будет гораздо счастливее получить такую заметку:

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

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

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

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

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

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

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

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

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

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

Автор может интерпретировать такое замечание двумя соврешенно разными способами:

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

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

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

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

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

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

Ещё один вариант — перефразировать предложение в виде вопроса, который начинается со слов «Что насчёт. » или «Как насчёт. »:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Через пару недель я опубликую вторую часть статьи. Будьте на связи, там рассмотрим дополнительные советы, в том числе:

Источник