📝 код-ревью 10: разобрал fixme-комменты из задачи — модель Item, типы, порог low_stock, возврат структурой

Author: esemi

code-review-practice/solutions/10_inventory.md

@@ -4,7 +4,11 @@
 
 1. **`process_order` конкатенирует строку с числом** (строка 39). `"...: " + inventory.total_value()`
    — `total_value()` возвращает число (int/float), `str + float` → `TypeError`, функция
-   всегда падает в конце. Фикс: `+ str(inventory.total_value())` или f-строка.
+   всегда падает в конце. Фикс: `+ str(inventory.total_value())` или f-строка. Но глубже —
+   **возврат человекочитаемой строки вместо структуры** делает результат нетестируемым и
+   непригодным для вызывающего кода: парсить число из строки никто не будет. Возвращать
+   стоит данные (статус + `total_value` числом / dataclass / dict), а форматирование — на
+   границе (в UI/логе).
 
 2. **Частичное списание заказа без атомарности** (строки 36–38). `process_order` идёт по
    позициям и вызывает `remove_stock`, **игнорируя возвращаемый `bool`**. Если по одной
@@ -29,5 +33,25 @@
 8. Конструктор принимает внешний `items` и хранит по ссылке — внешний код может мутировать
    склад мимо методов класса.
 
+9. **Товар — сырой вложенный dict вместо модели** (`{name: {"qty": ..., "price": ...}}`).
+   `self.items[name]["qty"]` раскидан по всем методам: опечатка в ключе не поймается, схема
+   позиции нигде не зафиксирована, `mypy` бессилен. Честный `@dataclass Item(qty: int, price: float)`
+   (или `NamedTuple`) убирает магические строки-ключи и даёт типизацию. Это же снимает
+   вопрос из п.5 (Decimal — поле модели).
+
+10. **Нет аннотаций типов** (все методы, `process_order`). `name: str`, `qty: int`,
+    `threshold: int`, возвращаемые типы — контракт склада должен читаться из сигнатур.
+
+11. **Порог `low_stock`: `<` или `<=`?** (строка 30). `qty < threshold` — граничное значение
+    (`qty == threshold`) считается достаточным. Это семантический выбор («низкий остаток» —
+    строго меньше или не больше?), его надо зафиксировать в докстринге/контракте, а не
+    оставлять читателю гадать.
+
+12. **Контракт `remove_stock` при нехватке** (строки 15–18). Сейчас при недостатке остатка
+    метод просто возвращает `False`, ничего не меняя — это ок. Но стоит продумать краевые
+    случаи явно: что делать с позицией при нулевом остатке (оставлять `qty=0` или удалять),
+    и как отличать «товара нет» от «не хватило» — сейчас первое даёт `KeyError` (п.3),
+    второе — `False`.
+
 **Итого:** `TypeError` в `process_order` (падает всегда) + неатомарное частичное списание
 + игнор возвращаемого `bool`. Мораль: если метод возвращает флаг успеха, вызывающий обязан его проверять.