Итоговая практика: рефакторинг C-style Python в идиоматичный код

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

def process_records(records, config=None, errors=[]):
    if config is None:
        config = {}
    result = []
    for i in range(len(records)):
        rec = records[i]
        f = open(config.get("log_path", "log.txt"), "a")
        try:
            if rec["value"] < 0:
                errors.append(rec)
                status = -1
            else:
                result.append(rec["value"] * 2)
                status = 0
            f.write(str(status) + "\n")
        finally:
            pass
        f.close()
    return result, errors

Разбор по пунктам, каждый — отсылка к пройденному модулю:

1. errors=[] в сигнатуре — классическая ловушка изменяемого значения по умолчанию. Список создаётся один раз при определении функции и переживает между вызовами: второй вызов process_records увидит «чужие» ошибки от первого. Это разобрано в модуле про функции и изменяемые значения по умолчанию — но здесь ловушка встречается в связке с остальными антипаттернами, что типично для реального код-ревью.
2. for i in range(len(records)) + records[i] — индексный цикл там, где нужна прямая итерация for rec in records. Не только менее читаемо, но и провоцирует ошибки границ, от которых итерация по элементам защищает.
3. status = -1 / status = 0, запись кода в файл — перенос C-идиомы «код возврата вместо исключения» в контекст, где для сигнализации проблемы уместнее исключение (raise ValueError(...) или собственный класс ошибки) либо, если это не ошибка, а бизнес-случай, — просто ветвление без имитации errno.
4. open(...) без with, f.close() в конце тела цикла, finally: pass — ручное управление ресурсом без гарантии закрытия при исключении внутри try. Плюс файл открывается заново на каждой итерации цикла — проблема и производительности, и корректности одновременно.
5. Возврат пары (result, errors) вместо исключения/явной структуры — работает, но смешивает «результат» и «накопленные проблемы» в одном плоском тюпле, что усложняет вызывающий код.

flowchart LR
    A1["for i in range(len(x)): x[i]"] --> B1["for item in x / enumerate(x)"]
    A2["status = -1 / 0, запись кода ошибки"] --> B2["raise SpecificError(...)"]
    A3["errors=[] в сигнатуре функции"] --> B3["errors: list | None = None,<br/>создание внутри тела"]
    A4["open(...) + вручную close()"] --> B4["with open(...) as f: ..."]

Рефакторинг с учётом всего вышеперечисленного:

class NegativeValueError(Exception):
    def __init__(self, record):
        super().__init__(f"negative value in record: {record}")
        self.record = record

def process_records(records, config=None):
    config = config or {}
    log_path = config.get("log_path", "log.txt")
    result = []
    errors = []
    with open(log_path, "a") as f:
        for rec in records:
            if rec["value"] < 0:
                errors.append(rec)
                f.write("error\n")
                continue
            result.append(rec["value"] * 2)
            f.write("ok\n")
    return result, errors

Заметьте: файл открывается один раз на весь вызов (а не на итерацию), with гарантирует закрытие даже при исключении внутри цикла, errors создаётся внутри функции, итерация идёт по значениям без индексов. Класс NegativeValueError определён на случай, если по требованиям это должна быть настоящая исключительная ситуация, а не штатная бизнес-ветка — выбор между raise и обычным if зависит от того, ошибка ли это семантически, а не от привычки всегда сигнализировать через код возврата.

# C-стиль
success = True
for item in items:
    if not validate(item):
        success = False
        break
if not success:
    print("validation failed")
    return None

if not all(validate(item) for item in items):
    raise ValueError("validation failed")

def load_users(paths, cache={}):
    users = []
    for i in range(len(paths)):
        path = paths[i]
        if path in cache:
            users.append(cache[path])
            continue
        f = open(path)
        data = f.read()
        f.close()
        if len(data) == 0:
            users.append(None)
        else:
            u = parse_user(data)
            cache[path] = u
            users.append(u)
    return users