Грейд разработчика приходит с годами, но иногда годы приходят одни: зачем вам код-ревью

Код-ревью (code review или CR) — это проверка кода, написанного разработчиком для решения задачи, перед вливанием в основную ветку проекта. Ревизию кода выполняет не сам автор, а другой разработчик. В идеале, ревьюер должен обладать не только релевантным опытом — владеть нужным стеком, но и быть значительно опытнее и уметь объяснять, что не так, простым и понятным языком.

Результат хорошего код-ревью — это обратная связь, которая помогает разработчику научиться писать код более качественно.

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

На проектах код-ревью может быть устроено по разному:

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

Подходы к код-ревью на проектах:

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

На код-ревью отлавливают следующие виды проблем:

Код тяжело читается, а значит его будет дорого поддерживать и развивать.

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

Некачественный нейминг увеличивает время на чтение и рефакторинг кода. Да и самому автору кода множество условных обозначений придется держать в голове. Лучше называть объекты сразу понятно.

Пример

Вместо абстрактной буквы i удобнее работать с конкретным, именованным объектом.

Название должно быть содержательным, должно соответствовать объекту, описывать его.

Плохо:

capitals = ['Москва', 'Лондон', 'Париж'] for i in capitals: ...

Хорошо:

capitals = ['Москва', 'Лондон', 'Париж'] for capital in capitals: …

Какие бывают проблемы с некачественным неймингом:

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

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

Пример

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

Плохо:

def download_image_and_upload_it_to_telegram(url, tg_token): ...

Здесь одна функция используется для двух действий.

Хорошо:

def download_image(url): ... def upload_image_to_telegram(tg_token): ...

Здесь на одно действие — одна функция.

Пример 2

Функция `calculate_statistics` содержит логику, которую лучше вынести наружу, иначе внутренние задачи выходят за рамки названия, а название функции ни в коем случае не должно обманывать.

Плохо:

def extract_salaries(vacancies): ... def calculate_statistics(vacancies): salaries = extract_salaries(vacancies) average_salary = int(sum(salaries) / len(salaries)) if salaries else 0 ... def main(): ... statistics = ... vacancies = ... statistics[language] = calculate_statistics(vacancies) ...

Хорошо:

def extract_salaries(vacancies): ... def calculate_statistics(vacancies, salaries): average_salary = int(sum(salaries) / len(salaries)) if salaries else 0 ... def main(): ... statistics = ... vacancies = ... salaries = extract_salaries(vacancies) statistics[language] = calculate_statistics(vacancies, salaries) ...

Реализация не учитывает корнер-кейсы и обычные исключения

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

Особые требования к форматированию

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

Неиспользование готового отлаженного функционала

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

Пример

Comprehensions позволяет легко создавать коллекции, но использовать данный функционал рекомендуется только для простых циклов for.

Плохо:

numbers = [...] even_numbers = [] for number in numbers: if number % 2 == 0: even_numbers.append(number)

Хорошо:

numbers = [...] even_numbers = [num for num in numbers if num % 2 == 0]

Проблемы с безопасностью

Какие бывают проблемы: прежде всего речь про работу с файлами, с базой данных, блоками авторизации/регистрации, про хранение и использование чувствительных данных. Здесь опытный разработчик подскажет best practice подходы и укажет на уязвимости. На серьезных проектах отдельный специалист проводит тестирование и анализ кода по требованиям информационной безопасности для соответствующего класса систем.

Пример

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

SECRET_KEY не должен иметь никаких default настроек. Так мы предотвратим риск утечки default значения в случае, если забудем указать SECRET_KEY при запуске приложения.

Плохо:

from environs import Env def main(): env = Env() env.read_env() SECRET_KEY = env.str('SECRET_KEY', 'value_if_secret_key_is_empty')

Хорошо:

from environs import Env def main(): env = Env() env.read_env() SECRET_KEY = env.str('SECRET_KEY')

Несоответствие функциональным требованиям

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

Вылавливание багов и несоответствий поручают автотестам, тестировщикам, продакт-менеджеру, но и ревьюер может заметить проблемы. Например, для работы с базами данных есть негласное правило, что для каждого класса должны быть реализованы CRUD операции (создание, чтение, изменение и удаление объекта).

Неэффективное использование памяти, проблемы с производительностью

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

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

Пример

Для хранения уникальных элементов лучше использовать `set`, а не `list`.

Плохо:

not_unique_numbers = [1, 1, 2, 3, 1, 4, 3, ...] unique_numbers = [5] for number in not_unique_numbers: if number not in unique_numbers: unique_numbers.append(number)

Хорошо:

not_unique_numbers = [1, 1, 2, 3, 1, 4, 3, ...] unique_numbers = {5} unique_numbers = set(not_unique_numbers).union(unique_numbers)

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

Пример

Плохо:

def calculate_fibonacci(num): if num <= 1: return num return calculate_fibonacci(num - 1) + calculate_fibonacci(num - 2) print(calculate_fibonacci(100))

Хорошо:

import functools @functools.lru_cache(maxsize=None) def calculate_fibonacci(num): if num <= 1: return num return calculate_fibonacci(num - 1) + calculate_fibonacci(num - 2) print(calculate_fibonacci(100))

Отсутствие оптимизации запросов может привести не просто к большому, а к чрезмерному количеству запросов к БД.

Пример

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

Плохо:

authors = Author.objects.all() for author in authors: books = author.books.all() print(f'Автором {author.name} написано книг: {books.count()} шт.')

Хорошо:

def display_authors_and_books_optimized(): authors = Author.objects.prefetch_related('books').all() for author in authors: print(f'Автором {author.name} написано книг: {len(author.books.all())} шт.')

При работе с большими файлами или данными чтение всего содержимого разом может привести к большим затратам памяти.

Пример

Загрузить все строки в список — так себе идея. Лучше обрабатывать файл построчно.

Плохо:

with open("large_file.txt", "r") as file: lines = file.readlines() for line in lines: do_something(line)

Хорошо:

with open("large_file.txt", "r") as file: for line in file: do_something(line)

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

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

Мы используем код-ревью в ходе обучения на курсах Devman. Это ключевое звено методики. Код-ревью помогает сократить время обучения профессии в разы и поднимает планку качества кода наших выпускников до уровня джун+/миддл. За 1,5 года с нуля на выходе получаем специалистов, которые могут решать задачи самостоятельно и писать читаемый и стабильный код, документацию.

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

Код-ревью от действующего разработчика

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

Чистота кода

В Python есть обязательный стандарт PEP8, в котором рассмотрены правила форматирования и наименования объектов.

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

Улучшения Девмана

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

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

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

Контекст важен

Ревьюер оценивает код с учетом конкретной ситуации. ООП незаменимо для Django ORM, но использование его для пары запросов через API усложняет поддержку и чтение кода. Не надо забивать гвозди микроскопом!

Накопление опыта

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

Пропустить нельзя

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

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

Мы собрали отзывы наших разработчиков о значении код-ревью:

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

Код ревью позволяет любому разработчику не только совершенствовать свои хард-скилы, но и решить еще 2 задачи: снизить риски провалов на проде и достичь более ценного результата за меньший промежуток времени. Хорошее ревью отличается подачей замечаний и предложений, Мало указать место, где имеются недочеты. Хорошо, когда ревьювер поясняет "почему" и предлагает, даже и кратко, какую-либо более современную и отвечающую текущей ситуации замену. Однако, излишнее увлечение даже с опытным ревьювером может сыграть и злую шутку — это может быть как задержка выдачи результата заказчику, так и выгорание разработчика. Здесь тоже имеют место быть правила "хорошего — понемногу" и "совершенству нет предела". В любом случае к результатам ревью надо относиться внимательно, воспринимать их не как попытку опытного наставника завалить молодого разработчика лишней ненужной работой, подстрекнуть его, а как совместные усилия, направленные на разработку качественного, поддерживаемого программного продукта, необходимого заказчику.
Юрий Олейник, мидл-разработчик

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

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

Что делать, если в команде код-ревью не проводят, но есть желание улучшить свои навыки? Или просто хочется получить независимую оценку своего кода?
Можно попробовать код-ревью в наших бесплатных уроках, которые есть в мини-курсах Девман:

«API веб-сервисов» — на этом мини-курсе можно научиться использовать API различных веб-сервисов: вытягивать данные, обрабатывать их и публиковать в интернете. На первом уроке «Получите погоду из терминала» нужно написать API-запрос к сайту погоды на завтра.
«Знакомство с Django: ORM» — при прохождении мини-курса вы поучаствуете в разработке сайтов — сами реализуете всё что относится к базам данных. На первом уроке вы напишете мини-сайт для охранника банка.

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

Грейд разработчика приходит с годами, но иногда годы приходят одни: зачем вам код-ревью

Зачем проекту нужно код-ревью

Зачем код-ревью разработчику

Код-ревью как часть методики обучения

Как устроено код-ревью в Devman

Опыт студентов

Где получить код-ревью