Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code review #16

Open
MasterGroosha opened this issue Mar 24, 2022 · 2 comments
Open

Code review #16

MasterGroosha opened this issue Mar 24, 2022 · 2 comments

Comments

@MasterGroosha
Copy link

Теперь по коду. Актуально для состояния репозитория в коммите 1f6d06b70382206bf54c372cc9cc490e261ca918

main.py, L78: нехорошо делать импорты где-то, кроме верхней части модуля.

.gitignore: папка "shit" -- очень смешно :D Если хотите скрыть файл/папку, но не кидать в .gitignore, кидайте в .git/info/exclude, PyCharm умеет это делать.

data_b/parsing/async_parsing.py: надо что-то тут написать, но пока лень :)

data_b/parsing/references_to_tasks.py: импорты отсортированы неправильно (PyCharm -> Double Shift -> Optimize imports), импортируются библиотеки, которых нет в "requirements.txt". Лечится легко: pip freeze > requirements.txt.

data_b/dp_control.py: НИКОГДА НЕ ИСПОЛЬЗУЙТЕ f-strings С SQL-ЗАПРОСАМИ. НИ-КО-ГДА. Неиспользуемые функции должны быть удалены (problem_translate_name(), например). Ну и в целом тут прям напрашивается SQLAlchemy + Alembic для миграций.

handlers/cmd.py: Ценность emojize переоценена. Лучше копировать напрямую эмодзи из TDesktop. Вместо четырех сообщений можно отправить два или три (объединив их), меньше шансов улететь во флудвейт.

# За такое в приличных домах канделябрами бьют.
# Нахрена тут dp_main? Выбросите его.
def register_handlers_start(dp: Dispatcher):
    global dp_main
    dp_main = dp

handlers/admins/send_message_all.py:

# Это нерабочий код о_О
except:
    None

handlers/admins/statistics_info_admins.py: В функции users_new() творится какая-то жесть. Но я там вижу попытку достать что-то из даты в строковом представлении, значит, с 99% шансом это дичь, которую надо выбросить и переписать. Что делает эта функция?

handlers/flashcards/flashcards_managing.py: Функция flashcards_managing_info(). Здесь хорошо бы сделать ранний выход. Например. Было:

if (x):
   ...
   ...
   ...
   ...
   ...
   ...
else:
   ...

Вместо этого можно сделать так:

if not(x):
    ...
    return
...
...
...

Возможно, найду что-то ещё

@AndrewKentavr
Copy link
Owner

Да, с папкой shit получилось неловко. Будем все фиксить

@MasterGroosha
Copy link
Author

handlers/flashcards_training.py: зачем тут заново инициализировать объект бота?

keyboards/default/admin_menu.py: не кажется ли, что вместо трёх функций можно сделать ровно одну, которая будет на вход принимать массив строк? (в соседних файлах аналогично)

logic/tasks_category_logic.py: global -- нафиг, плюс внутри функций никаких импортов быть не должно
L112 - неиспользуемая переменная

math/math_formulas.py: L48-L49 зачем два раза дёргать update, если можно один?
await state.update_data(condition=conditions, answers=answers)

math/mentally_math.py: очень много message.answer() подряд, легко нарваться на flood wait. Если нужна справка, проще сделать документ на https://telegra.ph и кидать ссылку на него один раз.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants