Чек-листы качества кода

Код читают в десять раз чаще, чем пишут. Ревьюер — первый из этих читателей, и его непонимание не случайность, а измерение.

#Результат урока

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

#1. Чек-лист — леса, а не здание

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

Правило: чек-лист применяется на четвёртом проходе (сопровождаемость), после того как вы уже ответили себе на вопрос о замысле и корректности. И ни один пункт чек-листа не может быть blocker, если только он не про поведение кода.

Проверьте себя. Возьмите чек-лист своей команды и вычеркните всё, что проверяет линтер. Что осталось?

Частая ошибка. Пройти чек-лист, поставить approve и не заметить, что PR решает не ту задачу.

#2. Что проверяет машина, а что человек

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

Проверяет инструмент	Проверяет человек
Отступы, кавычки, длина строки, порядок импортов	Раскрывает ли имя намерение
Неиспользуемые переменные и импорты	Соответствует ли имя тому, что функция делает на самом деле
Циклическая сложность выше порога	Оправдана ли эта сложность
Отсутствие аннотаций типов	Правильный ли тип выбран
`==` вместо `is` для `None`	Возможен ли здесь `None` вообще

Если в вашем ревью регулярно всплывают комментарии из левой колонки — это баг в настройке проекта, а не в дисциплине автора. Настроенный ruff, eslint и форматтер в pre-commit убирают весь класс таких споров за один вечер.

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

Частая ошибка. Обсуждать форматирование руками «пока не настроили линтер» — это состояние живёт годами.

#3. Имена как диагностика

Плохое имя редко бывает просто плохим именем. Чаще оно — симптом того, что автор сам не до конца понял, что написал.

# ❌ имя не раскрывает намерение
def process(d):
    ...

# ❌ имя врёт: функция ещё и пишет в БД
def get_user(user_id):
    user = db.fetch(user_id)
    user.last_seen = now()
    db.save(user)
    return user

Второй случай важнее первого. get_*, is_*, calculate_* — это обещание: «я ничего не меняю». Функция, которая обещает чтение, а делает запись, ломает ожидания в каждом месте вызова. Это major, а не nit, потому что дело не в эстетике: следующий разработчик вызовет get_user в цикле и получит тысячу записей в БД.

Полезные признаки:

Имя раскрывает намерение: deadline_at, а не d; MAX_RETRY_COUNT, а не 3 в коде.
Булевы значения читаются как утверждение: is_active, has_permission, а не flag или status.
Шумовые слова ничего не добавляют: UserDataManagerInfo — это User.
Имя произносимо: generation_timestamp, а не gen_ts. Непроизносимое имя нельзя обсудить на созвоне.

То же самое в TypeScript, где к именам добавляются типы:

// ❌ тип и имя ничего не обещают
function handle(data: any): any

// ✅ и имя, и сигнатура сообщают контракт
function calculateOrderTotal(order: Order): Money

Проверьте себя. Найдите в своём коде функцию, чьё имя начинается с get, и проверьте, есть ли у неё побочные эффекты.

Частая ошибка. Ставить nit на имя, которое врёт о поведении. Это major.

#4. Функция: одна задача, один уровень абстракции

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

# ❌ в одной функции: SQL, бизнес-правило и форматирование HTTP-ответа
def process_user(user_id):
    row = db.execute("SELECT * FROM users WHERE id = %s", (user_id,)).fetchone()
    if row is None:
        return {"status": 404, "body": "not found"}
    score = row.orders_count * 10 + (50 if row.is_premium else 0)
    return {"status": 200, "body": {"score": score, "tier": "gold" if score > 100 else "basic"}}

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

# ✅ каждый уровень отдельно
def calculate_loyalty_score(user: User) -> int:
    return user.orders_count * 10 + (PREMIUM_BONUS if user.is_premium else 0)


def get_user_profile(user_id: int) -> UserProfile | None:
    user = users_repo.find(user_id)
    if user is None:
        return None
    score = calculate_loyalty_score(user)
    return UserProfile(score=score, tier=tier_for(score))

Ориентиры, а не догмы: до 20–30 строк, до 3–4 аргументов, один уровень вложенности как норма и два как исключение. Когда аргументов становится шесть, это обычно значит, что три из них — на самом деле один объект.

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

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

#5. Комментарии: «почему», а не «что»

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

# ❌ пересказ
# Проверяем, что пользователь не None
if user is not None:
    ...

# ✅ объясняет решение, которого не видно из кода
# Линейный поиск: список гарантированно меньше 10 элементов (ограничение
# тарифа), сортировка обошлась бы дороже самого поиска.
for user in users:
    ...

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

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

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

Частая ошибка. Требовать комментарий там, где достаточно переименовать функцию.

#6. Обработка ошибок в чек-листе качества

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

# ❌ ошибка исчезает бесследно
try:
    process(data)
except:
    pass

# ❌ слишком широко: поймает и KeyboardInterrupt, и опечатку в имени атрибута
try:
    result = api_call()
except Exception:
    return None

# ✅ конкретно, с контекстом и решением на каждый случай
try:
    result = api_call()
except requests.Timeout:
    logger.warning("payment API timeout, retrying", extra={"order_id": order.id})
    return retry_api_call()
except requests.HTTPError as exc:
    logger.error("payment API error", extra={"status": exc.response.status_code})
    raise

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

Проверьте себя. Поищите в проекте except Exception и посмотрите, у скольких из них есть обоснование в комментарии.

Частая ошибка. Пропускать except Exception: pass как стилевую мелочь. Это blocker: он превращает падение в тихую порчу данных.

#7. Разбор: PR «Валидация промокода»

Дифф на 22 строки, линтер зелёный — то есть все замечания ниже человеческие.

+def check(code, u):
+    # получаем промокод
+    p = db.execute("SELECT * FROM promo WHERE code = %s", (code,)).fetchone()
+    if p:
+        if p.expires_at > datetime.now():
+            if p.used_count < p.max_uses:
+                if u.orders_count > 0 or p.for_new_users == False:
+                    p.used_count = p.used_count + 1
+                    db.save(p)
+                    return True
+    return False

Что видит ревьюер. Формально код работает. Но check ничего не сообщает о том, что проверяет, u не сообщает ничего вообще, четыре уровня вложенности прячут условие допуска, а имя check обещает проверку — при этом функция инкрементирует счётчик и пишет в БД. Возвращаемый False не различает «промокод не найден», «истёк» и «лимит исчерпан», хотя пользователю нужно показать разные сообщения.

Комментарии в PR:

promo.py:1 · major check обещает проверку, но внутри инкремент used_count и запись в БД. Имя должно отражать эффект: redeem_promo_code. Иначе кто-нибудь вызовет её, чтобы просто показать скидку в корзине, и спишет применение.

promo.py:9 · major True/False схлопывает четыре разные причины отказа в одну. Фронту нужно показать «промокод истёк» и «только для новых клиентов» по-разному. Верните результат-объект или поднимите типизированное исключение.

promo.py:1 · minor u → user, p → promo, code оставить. Однобуквенные имена здесь ничего не экономят.

promo.py:4 · minor Четыре вложенных if разворачиваются в четыре ранних выхода — заодно каждый получит свою причину отказа для предыдущего комментария.

promo.py:7 · nit p.for_new_users == False → not promo.for_new_users.

promo.py:2 · nit Комментарий «получаем промокод» пересказывает следующую строку, можно удалить.

Чем закончилось. После правки:

def redeem_promo_code(code: str, user: User) -> PromoResult:
    promo = promo_repo.find_by_code(code)
    if promo is None:
        return PromoResult.not_found()
    if promo.expires_at <= datetime.now(tz=UTC):
        return PromoResult.expired()
    if promo.used_count >= promo.max_uses:
        return PromoResult.limit_reached()
    if promo.for_new_users and user.orders_count > 0:
        return PromoResult.new_users_only()

    promo.used_count += 1
    promo_repo.save(promo)
    return PromoResult.ok(promo.discount)

Обратите внимание: ни одно замечание не было blocker — код работал. Но два major изменили сигнатуру функции, и именно они дали основной эффект. Если бы ревьюер начал с nit про == False, разговор ушёл бы в мелочи.

#8. Рабочий чек-лист

Для четвёртого прохода, когда замысел и корректность уже проверены:

□ ИМЕНА       раскрывают намерение и не врут о побочных эффектах
□ ФУНКЦИИ     одна задача, один уровень абстракции, ≤ 3–4 аргументов
□ ВЛОЖЕННОСТЬ разворачивается в ранние выходы
□ КОММЕНТАРИИ объясняют «почему»; мёртвый код удалён
□ ОШИБКИ      конкретные исключения, контекст в логе, ресурсы освобождаются
□ ЗАВИСИМОСТИ явные, без глобального состояния и циклических импортов
□ ТИПЫ        аннотации есть и осмысленны (не `any`)
□ ТЕСТЫ       покрывают новое поведение, а не только счастливый путь
□ ДОКИ        обновлены, если изменился контракт

Всё, что не попало в этот список, но регулярно всплывает в ваших ревью, — кандидат на правило линтера.

#Что дальше

Тренировка на фрагментах с несколькими проблемами сразу: Code Review Python, Code Review React. Настройка автоматических проверок подробно разобрана в теме Автоматизация курса Pro.

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

Проверяет инструмент

Проверяет человек

Отступы, кавычки, длина строки, порядок импортов

Раскрывает ли имя намерение

Неиспользуемые переменные и импорты

Соответствует ли имя тому, что функция делает на самом деле

Циклическая сложность выше порога

Оправдана ли эта сложность

Отсутствие аннотаций типов

Правильный ли тип выбран

== вместо is для None

Возможен ли здесь None вообще