Repository navigation
3. Банад Михаил Вячеславович. 614 - #129
phantom686 wants to merge 26 commits into
Conversation
phantom686
changed the title
EvgrafovMichail
left a comment
EvgrafovMichail
left a comment
There was a problem hiding this comment.
Старайтесь избегать копипасты, храните общие значения и общую логику в единственном экземпляре
| if p[PRODUCT_ID_INDEX] == product_id: | ||
| storage.pop(i) | ||
| return product_id | ||
| return None |
There was a problem hiding this comment.
При отсутствии товара с переданным ID вы просто возвращаете None. По условию необходимо было также вывести сообщение о том, что товара с таким ID найдено не было. Из-за этого некоторые тесты этой функции не пройдены.
| if not name: | ||
| print("product name must not be blank") | ||
| return None | ||
| for p in storage: |
There was a problem hiding this comment.
p - неинформативное имя. В языке нет никакого устоявшегося соглашения о физическом смысле данного имени. Другим разработчикам будет тяжело считать, назначение этой переменной.
Нарушение критерия M2. Минус 0.5 балла
| if cmd == "help" and len(parts) == 1: | ||
| show_help() | ||
| return True | ||
| elif cmd == "exit" and len(parts) == 1: |
There was a problem hiding this comment.
Нарушение критерия T1. Минус 0.25 балла
| return True | ||
| cmd = parts[0] | ||
|
|
||
| if cmd == "help" and len(parts) == 1: |
There was a problem hiding this comment.
Из-за использование if-elif блока для разбора команд у вас большая часть логики дублируется. Для реализации данной функции лучше подошел бы механизма сопоставления шаблонов match-case. Тогда код вышел бы лаконичнее, а логика не дублировалась бы.
Нарушение критерия S1. Минус балл
| print_result(res) | ||
| return True | ||
| elif cmd == "delete" and len(parts) == 2: | ||
| pid = int(parts[1]) |
There was a problem hiding this comment.
Магические константы делают код менее поддерживаемым и читаемым. Новым разработчикам будет непонятно, что значат константы и почему выбраны именно такие значения. А если логика разбора команд изменится, вам придется вручную во всех местах менять одно значение на другое
Нарушение критерия S3. Минус балл
| w_price = max(w_price, len(price_str)) | ||
| w_qty = max(w_qty, len(qty_str)) | ||
|
|
||
| def format_row(c1: str, c2: str, c3: str, c4: str) -> str: |
There was a problem hiding this comment.
Лучше без крайней необходимости вложенные функции не определять
| """ | ||
| # TODO: реализуйте функцию | ||
| return "" | ||
| w_id = len(TABLE_HEADERS[0]) |
There was a problem hiding this comment.
Тоже очень много копипасты. Все это можно было бы шаблонизировать, чтобы сделать код устойчивее к изменениям, и не дублировать одну и ту же логику в нескольких местах
No description provided.