Код-ревью: две истории про Фёдора и Лёху

ноябрь 3 , 2018

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

Я не буду строить теории и учить, как нужно. А просто расскажу две истории. Очень разные истории.

Довелось однажды работать там, где код-ревью активно практиковалось. Вот я делаю задачу, отдаю ее на ревью, а сам перехожу к другим. Иногда смотрю, что написали мне коллеги. Соглашаюсь, если замечание дельное. Уточняю, если что-то не понимаю. Обсуждаю, если предложение спорно. Короче, как у всех. Пока код-ревью не стал заниматься


Фёдор

Фёдор хороший программист. Грамотный. Знает паттерны, алгоритмы, секреты оптимизаций кода и красивых конструкций. Прочитал немеряно книг умных людей. Он пишет отличный код и требует того же от своих коллег.

Уже догадываетесь, к чему я клоню?

Фёдор готов пропускать в мастер только идеальный код. Он не жалеет сил, чтобы донести до вас, как делать ПРАВИЛЬНО. Умный человек на проекте, такой как Фёдор - это очень круто. Но вот умный ИДЕЙНЫЙ это вопрос. Давайте посмотрим.

1. Фёдор защищает названия переменных и функций, предложенные им. Он знает, что в программировании есть две сложности: названия переменных и еще какая-то херня, я не помню.
count, amount, countCars, amountCars или amountOfCars - идеальное название будет обсуждаться минут 15, пока ты не сдашься и не согласишься на его версию.

2. Извечный вопрос, сколько строк в функции? Должна она влезать на экран или нет? А если должна, то в какой именно экран?

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

4. Написал не оптимальный алгоритм обработки коллекции из 20 объектов? Вот тебе книга по алгоритмам, читай. Прочитаешь, исправишь, тогда и продолжим.

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

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

1. Чаще всего Фёдор прав. Да, как бы ты к нему не относился. Но Фёдор предложит лучшие названия переменных, лучше разнесет код по модулям и найдет лучший алгоритм.

2. Ты узнаешь что-то полезное, но тебе приходится тратить очень много времени. Иногда ты узнаешь новое со слов Фёдора, но чаще гуглишь или вообще конкретно изучаешь новую тему под правки на код-ревью.

3. На тебя давят. Сверху начальство, сбоку коллеги. Когда ты сдашь эту задачу? Кто следующую будет делать? Что значит код-ревью не закрыто? Так закрой. Исправь ошибки и заливай в мастер.

4. И ты разрываешься между желанием сделать правильно и завершить задачу. К сожалению, у тебя не столько мозгов, как у Фёдора, чтобы сделать идеально с первого раза. И не столько времени. Стоп! Опять время.

У тебя нет столько, сука, времени, как у Фёдора!

Ты делал задачу 2 дня, а Фёдор ревьюил ее сколько? 3-4 часа, так? Да, точно, он сам на стендапе сказал. Половину рабочего дня потратил на твое код-ревью... А ты ведь не один такой уникальный, вас десяток программистов, и Фёдор шефствует над каждым. А когда ты последний раз видел его код в мастере? Хм, ну в мастере не помню, но вот есть в отдельной ветке, отличный код. Все разложено по полочкам, декомпозировано, задокументировано. Покрыто тестами, все классно. Почему он не в мастере? А потому что этот код уже никому на хер не нужен. Потому что погромисты-костоломы втихую уже накодили, что нужно. Не идеально, но приемлемо. Если решить пришлось совсем уж безбожным костылем, то завели задачу на рефакторинг. И стали работать дальше.

В подходе Фёдора есть и плюсы, и минусы. Поделитесь своим мнением, оставьте комментарий ниже. А я пока расскажу вторую историю.


Лёха

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

Пишу им, когда посмотрите? Отвечает Лёха, мол, подойди сюда. Иду. Говорит, садись рядом, смотри.

1. Вот здесь лучше вынести эту строку в конфиг, она пригодится в будущем.
2. Это coffeescript, здесь и здесь лишние скобки, без них будет короче и красивее.
3. В lodash давно есть _.sum, ни к чему эти понты с _.reduce.
4. Вот здесь не очевидно, почему ты исключил из массива это число. Можно вынести в константу с говорящим названием.

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

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

Код-ревью заняло минут 15. Еще минут 40 исправления. Мерджреквест приняли, залили в мастер, фича ушла на прод.

Но уже это не так важно. Важно то, что прошло несколько лет, а я до сих пор помню и использую почти все, о чем говорил мне тогда Лёха.

Вот и вся история.


Постскриптум

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

Может быть, Лёха - это просто повзрослевший Фёдор. А может, просто отчаявшийся пробиться до пустых голов коллег. Или не было времени со мной возиться. Или что-то еще. А что вы об этом думаете? Какой подход вам ближе и почему?

Что еще почитать

Заходите в группу в контакте - https://vk.com/webdevkin
Анонсы статей, обсуждения интернет-магазинов, vue, фронтенда, php, гита.
Истории из жизни айти и обсуждение кода.
Как Вам статья? Оцените!