в Code Reviews — Отдайте предпочтение редакциям, а не корректуре | Уильям Дворжак | Середина
В обзорах кода есть две разные формы обратной связи: проверка и корректура.
Когда я работал в Google, я видел чрезмерный упор на корректуру в каждой команде, в которой я участвовал. Это также очевидно во многих других инженерных культурах.
Я обнаружил, что наиболее описательные названия этих двух понятий пришли из журналистики: редактура и корректура. В культуре разработки программного обеспечения нет действительно хороших терминов. Структура против стиля может быть одним из способов, но я не думаю, что он полностью отражает важные моменты. Я утверждаю, что проверка гораздо важнее, и что многие инженерные культуры придают слишком большое значение корректуре.
Ревизионная обратная связь приходит в совокупности и не относится к одной строке и, как правило, даже не к методу. Просьба о пересмотре — это призыв изменить подход и общую структуру. Это может включать «абстрагирование этого кортежа в его собственный класс», «упрощение этого API до меньшего количества методов» или «удаление этой косвенности, здесь нет необходимости в дополнительном уровне абстракции». Исправление обычно требует, чтобы автор думал о проблеме, не печатая на клавиатуре.
С другой стороны, вычитка обратной связи обычно относится к одной или нескольким строкам. Это включает в себя создание более описательных имен переменных, выделение вспомогательных функций или замену do-while на for. Исправление этих изменений становится утомительным и очень локализованным. Следование руководствам по стилю является большим источником обратной связи по корректуре, поскольку руководства по стилю часто строго ограничивают длину строки и стиль регистра переменных и методов.
Возьмем, к примеру, этот коммит:
Пересмотр обратной связи может включать:
— Извлечь last_users в постоянное хранилище, например, в облачный SQL.
-Добавить путь отказа для вызовов API.
Корректирующая обратная связь может быть:
-Удалите past_users из списка пользователей перед итерацией в строке 44.
-Используйте правильные пробелы между переменными, разделенными запятыми, в строке 24.
-tokens, atoken и atokens (строка 24) могут сбивать с толку. Выбирайте более описательные имена.
Этот пример небольшой, поэтому трудно показать осмысленный отзыв об исправлении.
Пересмотр вашего кода важен по многим причинам. Типы ошибок, которые случаются на высоком уровне, являются более постоянными и дорогостоящими, чем те, которые локализованы в одном методе. Если схема вашей базы данных не соответствует действительности или API работает небрежно, работа по ее исправлению требует огромных усилий. Это может быть исправлено только через несколько недель или месяцев после того, как код, нарушающий правила, появится. К этому моменту на основе неправильных абстракций было построено гораздо больше модулей. Чтобы переделать исходный код, необходим серьезный рефакторинг. Часто с ошибками приходится жить, и пользователь получает неоптимальный опыт. Работа по исправлению каждого слоя стека становится слишком дорогостоящей.
Ошибки в именовании переменных или другие проблемы с корректурой обычно локализуются в одном файле. Это означает, что их легко исправить позже, но, что более важно, они не влияют на систему в целом. Если ошибка имеет значение, ее легко исправить. Если это не имеет значения, оно остается навсегда, и никому до него нет дела. Младшие инженеры могут продумать метод с однобуквенными переменными. Они могут рассуждать о двух циклах for, которые можно было бы объединить в один. Эти проблемы находятся в таком небольшом количестве строк кода, что требуется несколько секунд, чтобы определить, что происходит.
И наоборот, откорректированный код в неправильной абстракции выбрасывается при рефакторинге. Усилия по корректуре потеряны, потому что код теперь удален. Если бы вместо этого он был исправлен, он был бы написан только один раз, а небрежность, которую нужно исправить, можно исправить на досуге. Поскольку инженеры почти всегда предпочитают писать новый код, а не редактировать старый (даже в культуре такой корректуры), важно, чтобы мы стояли на прочном фундаменте хорошо продуманных абстракций.
В Google я наблюдал за самыми старшими инженерами, и их мышление было сосредоточено на понимании больших и чрезмерно сложных систем — поиске способов более правильно абстрагировать их для более чистого мышления младших инженеров. Упрощения больших абстракций служили кормом для продвижения по службе. Это подчеркивает относительный приоритет компании. Однако потребность в этих масштабных рефакторингах могла быть устранена в первую очередь за счет хорошей ревизии кода.
Вы можете увидеть влияние обратной связи на ремонтопригодность кода в нашем примере. Если бы мы исправили только ревизии, у нас была бы более функциональная и надежная кодовая база. Тем не менее, зафиксировав изменения корректуры, мы получили бы немного более читаемый код, что сэкономило бы нам 10 секунд при проверке кода в будущем.
Из-за неаккуратного именования и плохой гигиены кода могут возникнуть тонкие ошибки, поэтому проверка не лишена смысла. Они также не исключают друг друга. Но я думаю, что на пересмотре важнее сосредоточиться.
Мой аргумент здесь похож на пропаганду «поступай правильно» вместо «поступай правильно». Мне кажется, что инженеры на порядки отдают приоритет деталям, упуская при этом возможность создать что-то полезное.
Делайте правильные вещи при просмотре кода. Оставляйте отзывы о ревизии и игнорируйте придирки. В результате у всех нас будет больше полезных продуктов. И если вы не заметили, что слово «необходимый» написано с ошибкой в моем заголовке, я доказал свою точку зрения.
