Обнаружение багов, edge cases, обработка исключений, null-проверки
Ревьюер не запускает код. Его инструмент — подстановка входных данных, на которых автор не думал.
Вы освоите три техники, которые превращают чтение диффа в систематический поиск: мысленное выполнение с конкретными значениями, анализ границ и проверку инвариантов. И научитесь отличать баг, который надо блокировать, от гипотезы, которую надо оформить как вопрос.
Просто читать код бесполезно — он написан человеком, который был уверен в своей правоте, и при обычном чтении вы повторяете ход его мысли. Работает другое: взять конкретное значение и провести его через код руками.
Обязательный набор для любой функции:
process([]) # пусто
process([x]) # ровно один элемент
process(None) # отсутствие
process([...]) # нормальный случай
process(huge) # очень многоИ столько же для чисел и строк:
| Тип | Что подставлять |
|---|---|
| Число | 0, -1, 1, максимум типа, дробное там, где ждали целое |
| Строка | "", один символ, очень длинная, юникод и эмодзи, пробелы по краям |
| Коллекция | [], один элемент, дубликаты, None внутри |
| Дата | начало и конец диапазона, 29 февраля, переход на летнее время, другая таймзона |
| Деньги | 0, отрицательное, значение с тремя знаками после запятой |
# ❌ падает на пустом списке
def calculate_average(numbers):
return sum(numbers) / len(numbers)
# ✅
def calculate_average(numbers: list[float]) -> float:
if not numbers:
return 0.0
return sum(numbers) / len(numbers)Проверьте себя. Возьмите любую функцию из открытого PR и прогоните по ней все пять входов из списка выше.
Частая ошибка. Проверять только тот сценарий, который описан в тикете, — его автор точно проверил сам.
Off-by-one живёт в сравнениях и в индексах, и оба случая ловятся подстановкой ровно граничного значения.
def is_adult(age):
return age > 18 # 18-летний совершеннолетний → должно быть >=
def get_last(items):
return items[len(items)] # IndexError: последний индекс len - 1
def calculate_discount(price, discount):
return price - discount * price # при discount=0.2 верно; при 1.2 — отрицательная ценаВопрос к каждому сравнению: что происходит ровно на границе. Вопрос к каждому индексу: что при длине 0 и при длине 1.
Отдельная ловушка — приоритет операторов:
# ❌ "pending" — непустая строка, то есть всегда истинна: условие всегда True
if status == "active" or "pending":
...
# ✅
if status in ("active", "pending"):
...
# ❌ читается не так, как выглядит: is_admin OR (is_moderator AND is_active)
if user.is_admin or user.is_moderator and user.is_active:
...
# ✅ явные скобки
if (user.is_admin or user.is_moderator) and user.is_active:
...Проверьте себя. Найдите в проекте условие с and и or без скобок и проверьте, совпадает ли фактическая группировка с задуманной.
Частая ошибка. Верить отступам и переносам строк: они показывают намерение автора, а не приоритет операторов.
Инвариант — утверждение, которое обязано быть истинным до и после операции. Найдите его — и половина багов станет видна без подстановки значений.
def transfer(from_account, to_account, amount):
from_account.balance -= amount
to_account.balance += amountИнвариант: сумма балансов не меняется. Он нарушается, если между двумя строками произойдёт сбой, — деньги исчезнут. Ревьюеру нужно спросить про транзакцию. Второй инвариант: баланс не отрицателен — проверки нет вообще. Третий: amount > 0 — иначе перевод отрицательной суммы работает как кража в обратную сторону.
Типичные инварианты, о которых забывают: сумма частей равна целому, счётчик не уходит в минус, статус меняется только по разрешённым переходам, кэш и источник данных не расходятся, у каждой записи есть владелец.
Проверьте себя. Для последней написанной вами функции сформулируйте инвариант одним предложением и проверьте, есть ли на него тест.
Частая ошибка. Проверять шаги по отдельности, не проверив утверждение, которое должно выполняться после всех шагов вместе.
Часть багов не про логику, а про то, как язык устроен. Их надо знать наизусть — ревьюер обязан замечать их мгновенно.
Изменяемое значение по умолчанию (Python). Вычисляется один раз при определении функции, а не при каждом вызове:
def add_item(item, items=[]): # ❌ один список на все вызовы
items.append(item)
return items
def add_item(item, items=None): # ✅
if items is None:
items = []
items.append(item)
return itemsЗамыкание в цикле. Захватывается переменная, а не её значение:
functions = [lambda: i for i in range(5)]
[f() for f in functions] # ❌ [4, 4, 4, 4, 4]
functions = [lambda x=i: x for i in range(5)]
[f() for f in functions] # ✅ [0, 1, 2, 3, 4]В JavaScript тот же класс ошибок жил в var и лечится let, а сегодня чаще проявляется в React через устаревшее замыкание:
// ❌ count навсегда остался тем, каким был на первом рендере
useEffect(() => {
const id = setInterval(() => setCount(count + 1), 1000);
return () => clearInterval(id);
}, []);
// ✅ обновление от предыдущего значения
useEffect(() => {
const id = setInterval(() => setCount(prev => prev + 1), 1000);
return () => clearInterval(id);
}, []);Сравнение дробных чисел. 0.1 + 0.2 == 0.3 ложно и в Python, и в JS — двоичное представление не хранит эти значения точно. Для денег используйте целые копейки или Decimal, для сравнений — math.isclose.
Поверхностное копирование. list.copy(), {...obj} и dict(d) копируют один уровень:
original = [[1, 2], [3, 4]]
shallow = original.copy()
shallow[0][0] = 99 # original тоже изменилсяИзменение коллекции во время обхода. Пропускает элементы, потому что сдвигаются индексы:
for item in items:
if item % 2 == 0:
items.remove(item) # ❌
items = [item for item in items if item % 2] # ✅Проверьте себя. Проверьте, ловит ли ваш линтер изменяемые значения по умолчанию. Если да — эти замечания вообще не должны доходить до ревью.
Частая ошибка. Считать такие ловушки экзотикой для собеседований. Они попадают в прод чаще всего именно потому, что выглядят как рабочий код.
Отдельный раздел, потому что это самый дорогой класс ошибок: сбой не исчезает, он превращается в неверные данные, и обнаружится через недели.
# ❌ конфиг не прочитался — работаем с пустым и не знаем об этом
def load_config():
try:
return json.load(open("config.json"))
except:
return {}Три отдельные проблемы: голый except ловит в том числе KeyboardInterrupt; ошибка нигде не зафиксирована; вместо конфигурации возвращается пустой словарь, и приложение стартует в неопределённом состоянии.
Что спрашивать у автора: какое конкретно исключение здесь ожидается, что записывается в лог и почему возвращаемое значение — правильное поведение, а не маскировка.
Проверьте себя. Найдите в проекте except без указания типа и посчитайте, сколько из них молчат.
Частая ошибка. Отмечать это как minor. Молчаливое проглатывание исключения — blocker.
Задача: после оплаты заказа начислять пользователю бонусные баллы. Дифф на 25 строк, тесты в PR есть — один, на счастливый путь.
+def award_bonus(order_id):
+ order = db.get_order(order_id)
+ user = db.get_user(order.user_id)
+
+ bonus = int(order.total * 0.05)
+ if order.promo_code:
+ bonus = bonus * 2
+
+ user.bonus_balance = user.bonus_balance + bonus
+ db.save(user)
+
+ history = db.get_bonus_history(user.id)
+ avg = sum(h.amount for h in history) / len(history)
+ logger.info(f"awarded {bonus}, avg {avg}")
+
+ try:
+ send_bonus_email(user.email, bonus)
+ except:
+ passПрогоним техники по очереди.
Подстановка значений. order_id несуществующего заказа → order равен None → AttributeError на следующей строке. Заказ на 15 рублей → int(15 * 0.05) = 0, начислили ноль баллов и всё равно записали в историю и отправили письмо «вам начислено 0 баллов».
Границы. Возврат заказа: order.total отрицательный → бонус отрицательный → баланс уменьшился. Функция называется «начислить», а умеет списывать.
Инварианты. Нет защиты от повторного вызова: если платёжный вебхук придёт дважды — а он придёт, — бонусы начислятся дважды. Инвариант «за один заказ бонус начисляется один раз» не обеспечен ничем.
Языковые ловушки. len(history) равен нулю для первого в жизни пользователя начисления, если история пишется после — ZeroDivisionError. И весь блок со средним значением нужен только для строчки в логе.
Комментарии в PR:
bonus.py:1· blocker Нет защиты от повторного начисления. Платёжный вебхук доставляется минимум один раз, а не ровно один раз, — при ретрае пользователь получит бонус дважды. Нужен уникальный ключ поorder_idв таблице начислений или проверка «уже начисляли».
bonus.py:5· blocker При возвратеorder.totalотрицательный, и функция спишет баллы. Либо явный ранний выход дляtotal <= 0, либо возвраты обрабатываются отдельным сценарием — но тогда это надо написать в коде, а не подразумевать.
bonus.py:13· blockerlen(history)= 0 у первого начисления →ZeroDivisionError. Причём падение произойдёт уже послеdb.save(user): баллы начислены, а транзакция оборвалась. Среднее нужно только для лога — предлагаю просто убрать.
bonus.py:2· majordb.get_orderдля несуществующего id вернётNone, дальшеAttributeError. Нужен явный ранний выход.
bonus.py:18· major Голыйexcept: passвокруг отправки письма. Понимаю замысел — сбой почты не должен ломать начисление, — но так вы не узнаете, что почта не уходит уже неделю. Ловите конкретное исключение и логируйте.
bonus.py:5· questionint()округляет вниз: на заказе 15 ₽ бонус будет 0, и пользователь получит письмо про ноль баллов. Это осознанно? Если да — стоит не слать письмо при нулевом начислении.
bonus.py:8· nituser.bonus_balance = user.bonus_balance + bonus→+=. Не блокирую.
Чем закончилось. Автор добавил уникальный индекс по order_id в таблице начислений, ранний выход для None и неположительной суммы, убрал вычисление среднего, обернул отправку письма в except SMTPException с логированием и перенёс её за границу транзакции. Появились три теста: повторный вызов, возврат, заказ на маленькую сумму.
Отметьте, что определило список замечаний: не чтение кода, а прогон конкретных значений. Ни один blocker не был виден «на глаз».
ВХОД пусто · один элемент · None · очень много · невалидное значение
ГРАНИЦЫ на границе сравнения · нулевая длина · максимум типа · отрицательное
ИНВАРИАНТЫ что обязано быть истинным после операции — и обеспечено ли это
ПОВТОР что будет, если вызвать дважды (ретрай, двойной клик, вебхук)
СБОЙ что останется в БД, если упасть посередине
ИСКЛЮЧЕНИЯ конкретный тип · попадает в лог · ресурсы освобождаютсяТренировка ровно на этом навыке: Code Review Python → Баги и Исключения, для фронтенда — Code Review React → Управление состоянием. Конкурентные баги, которые не ловятся чтением одного файла, разбираются в теме Асинхронность и конкурентность курса Pro.
Ключевая мысль: если вы не подставили в код ни одного конкретного значения, вы его не проверили, а прочитали.
Далее: Проверка безопасности