review: добавил TODO

This commit is contained in:
Андрей Скирченко 2025-01-16 18:46:48 +03:00
parent a6a7653ca5
commit d53d4794f5
3 changed files with 52 additions and 15 deletions

View File

@ -336,19 +336,25 @@ class MainWindow(BaseMainWindow):
# TODO: Нарушение принципа единичной ответственности. Почему ИНТЕРФЕЙС занимается сохранением чего-то?
# От него должен только исходить призыв к действию (кнопка или что-то подобное), сама логика же должна
# быть в другом месте.
# TODO: эта реализация работает, но для пользователя будет больно каждый раз писать имена и расширения
# сохраняемых файлов, не кажется? Можно сделать, как показано ниже, тогда файлам будет назначаться имя
# по умолчанию и с расширением. ХЗ правда, как эта форма на винде будет выглядеть - надо проверить.
# ========================================================
# dialog = QFileDialog()
# dialog.setOptions(QFileDialog.DontUseNativeDialog)
# dialog.setFileMode(QFileDialog.AnyFile)
# dialog.setAcceptMode(QFileDialog.AcceptSave)
# dialog.setDirectory(os.getcwd())
# dialog.selectFile("untitled.txt")
# dialog.exec_()
# if dialog.accepted:
# # emit signal to save file
# ...
# ========================================================
filepath, _ = QtWidgets.QFileDialog.getSaveFileName(self, "Save file", "", "Image Files (*.png *.jpeg)")
tab = self.tabWidget.currentWidget()
if tab is not None:
pixmap = QPixmap(tab.size())
tab.render(pixmap)
pixmap.save(filepath)

View File

@ -2,6 +2,9 @@ from src.uml.request_generator import Request
class UMLCreator:
# TODO: в методах, обремененных некой логикой, постоянно встречается проверка режима if not self.theor_mode.
# Есть предложение реализовать паттерн СТРАТЕГИЯ, который в зависимости от режима делал бы что-то
# разными способами. Это бы помогло избавиться от спагетти и обособить логику работы в зависимости от ситуации.
def __init__(self, request_generator: Request, path_to_save: str):
self._request_generator = request_generator
self.path_to_save = path_to_save
@ -21,8 +24,17 @@ class UMLCreator:
for item in items:
real_data.append([item[0]*self.scaler+0.0001, str(key) + '#green'])
real_data.append([item[1]*self.scaler, '{-}'])
if key == 'compression':
# TODO: Дублирование проверки значения ключа. Можно объединить проверки вверху и тут
# в одном match - case выражении:
# match key:
# case "compression":
# ...
# case "opening":
# ...
# case _:
# ...
ideal_data.append([(item[1]-ideal_time)*self.scaler, str(key) + '#yellow'])
ideal_data.append([(item[1]-0.0001)*self.scaler, '{-}'])
else:
@ -30,10 +42,16 @@ class UMLCreator:
ideal_data.append([(item[0]+ideal_time)*self.scaler, '{-}'])
if key == 'opening':
ideal_data.append([item[1]*self.scaler+0.0001, 'coming #yellow'])
# TODO: elif key == 'opening': ideal_time = self._ideal_time[2]. Почему здесь при условии
# значения ключа opening уже self._ideal_time[3]?
ideal_data.append([(item[1]+self._ideal_time[3])*self.scaler, '{-}'])
else:
else:
# TODO: real_data уже определена как [] в самом начале. Если не попадаем в верхнее условие,
# то она таковой и останется. Дублирование.
real_data = []
# TODO: Похоже, что все данные в этом списке - константы. Следовательно, можно проинициализировать этот
# набор данных однократно при инициализации класса, а не делать это время всякий раз при вызове метода.
ideal_data = [
[0.0, 'closure #yellow'],
[self._ideal_time[0] * self.scaler, '{-}'],
@ -45,7 +63,12 @@ class UMLCreator:
[(sum(self._ideal_time[:3]) + self.WeldTime) * self.scaler, 'coming #yellow'],
[(sum(self._ideal_time[:4]) + self.WeldTime) * self.scaler, '{-}'],
]
# TODO: Очень неудачный способ хранения данных разного типа: сложен в восприятии,
# легко ошибиться или запутаться.
# Используй NamedTuple или dataclass: будут определены поля, их типы.
# Взаимодействие также будет гораздо проще (по имени поля, а не индексу).
# TODO: Похоже, что все данные в этом списке - константы. Следовательно, можно проинициализировать этот набор
# данных однократно при инициализации класса, а не делать это время всякий раз при вызове метода.
client_data = [
[0.0 * self.scaler, 'closure'],
[0.165 * self.scaler, '{-}'],
@ -57,7 +80,7 @@ class UMLCreator:
[0.300 * self.scaler, '{-}'],
]
# TODO: Зачем метод возвращает self.bool_dict, если никаких манипуляций с ним не проводит?
return real_data, client_data, ideal_data, self.bool_dict
def _generate_svg(self, real_data, client_data, ideal_data, bool_data) -> None:
@ -70,6 +93,8 @@ class UMLCreator:
for i, [signal, changes] in enumerate(bool_data.items()):
name = 'bool_' + str(i)
# TODO: 3 строки ниже замени на list comprehension
# times = [[str(float(f[0])*scaler), f[1]] for f in changes]
times = []
for f in changes:
times.append([str(float(f[0])*self.scaler), f[1]])
@ -98,7 +123,9 @@ class UMLCreator:
timings_dict: dict,
mode: bool,
name:str):
# TODO: Это надо в конструкторе класса делать!
# TODO: Аргумент mode вводит в заблуждение: ожидаешь увидеть какой-то режим, на деле же это флаг
# теоретического режима. Поправь название аргумента.
# TODO: Инициализацию атрибутов класса в конструкторе надо делать!
self._ideal_time = ideal_time
self.bool_dict = bool_dict
self.float_dict = float_dict

View File

@ -100,6 +100,8 @@ class Request:
def _compileUML(self, name):
# TODO: Единый стиль именования методов! (snake_case / lowercase).
# TODO: Метод и строку собирает, и в файл пишет. Разделить логику формирования строки и записи в файл
# по разным методам.
self._addHeader()
self._addVariables()
@ -108,6 +110,8 @@ class Request:
self._addTimestamp(key, item)
self._endUML()
# TODO: Зачем делать из self.stringUML атрибут класса?
# Используется только тут - достаточно локальной переменной.
self.stringUML = [line + '\n' for line in self.reqArr]
with open(f'{name}.txt', 'w', encoding='utf-8') as file:
file.writelines(self.stringUML)