Conversation
AlexPrib
left a comment
AlexPrib
left a comment
There was a problem hiding this comment.
В конце ревью хочу пройтись по каждому файлу и написать общие предложения по улучшению кода.
- В файле main.py следовало бы сделать классы наследниками от общего предка.
- В файле point1.py следует сделать отдельные функции чтения и записи, и избавиться от захардкорженных путей.
- В файле point2.py так же, как в point1.py нарушается SRP.
- В файле point3.py повторяется функция name_for_file. Так же в файле есть бесполезная функция, и нарушается SRP
- В файле point4.py есть нарушения SRP и есть проблемы с логикой в коде функций
|
|
||
| class Iter: | ||
| def __init__(self): | ||
|
|
There was a problem hiding this comment.
Незначимая пустая строка
| self.file_name = "dataset.csv" | ||
|
|
||
| def __next__(self) -> tuple: | ||
|
|
There was a problem hiding this comment.
Незначимая пустая строка
| if os.path.exists(self.file_name): | ||
| with open(self.file_name, "r", encoding="utf-8") as csvfile: | ||
| arr = list(csv.reader(csvfile, delimiter=",")) | ||
|
|
There was a problem hiding this comment.
Незначимая пустая строка
|
|
||
| if self.counter == len(arr): | ||
| raise StopIteration | ||
|
|
There was a problem hiding this comment.
Незначимая пустая строка
|
|
||
| class YXiter: | ||
| def __init__(self): | ||
|
|
There was a problem hiding this comment.
Незначимая пустая строка
| """ | ||
|
|
||
| if os.path.exists(file_name): | ||
| with open(file_name, 'r', encoding='utf-8') as csvfile: |
There was a problem hiding this comment.
Считывание стоит вынести в отдельную функцию
|
|
||
| def __init__(self): | ||
| self.counter = 0 | ||
| self.file_name = 'dataset.csv' |
There was a problem hiding this comment.
Это поле класса должно быть передаваемым параметром
| def __next__(self) -> tuple: | ||
|
|
||
| if os.path.exists(self.file_name): | ||
| with open(self.file_name, 'r', encoding='utf-8') as csvfile: |
There was a problem hiding this comment.
Считывание с файла нужно вынести в отдельный метод
| import os | ||
|
|
||
|
|
||
| def name_for_file(first_part: str, second_part: str) -> str: |
There was a problem hiding this comment.
Функцию следовало назвать make_name_for_file, либо create_name_for_file
| return int(day) | ||
|
|
||
|
|
||
| def name_for_file(first_part: str, second_part: str) -> str: |
There was a problem hiding this comment.
Функцию следовало назвать make_name_for_file, либо create_name_for_file
alxmcs
left a comment
alxmcs
left a comment
There was a problem hiding this comment.
Хотя 90% комментариев это "незначимая пустая строка", в ревью есть указание на некоторые ошибки в построении/проектировании приложения, так что, думаю, ревью можно зачесть.
2 participants
No description provided.