Не могли бы оценить мой код на Python?

Question

[Валерий]

Не могли бы оценить код моего проекта, который я делал для выпускной квалификационной работы.
Хотелось бы услышать на какие вещи стоит обратить внимание, как улучшить свой код.
GitHub: https://github.com/ApXNTekToP/MerchTelegramBot
Оценить бота можно здесь: https://t.me/testmerchant_bot
Спасибо за внимание!

Comments

[Wispik]

1. Папку venv пушить не надо в репозиторий, смысла в ней нет
2. В чем смысл искать одну и туже строку?

re.findall(r'кактебязовут?' or r'кактебязовут', message.text):

re.findall(r'чемтызанимаешься?' or r'чемтызанимаешься', message.text):

Это я просто мельком глянул, первое что в глаза бросилось.

[Дмитрий]

Данные для подключения базы гвоздями приколочены. Чтоб поменять например порт, придётся лезть в код, и менять не в одном даже месте

[Дмитрий]

Wispik, да и папку идеи можно тоже в гитигнор

[Валерий]

Wispik, Вообще расчет был на то, что пользователь может написать вопрос без знака "?" или с ним. Хотя я уже осознаю что можно было с помощью патерна, искать только буквы, без всяких знаков. Учту на будущее. В ближайшее время займусь рефакторингом. Спасибо!

[Дмитрий]

Валерий, если посмотреть, то в каждом методе, где нужно работать с бд, идёт дублирование кода, это и про настройки подключения, меняется только имя таблицы по сути, вынести это, чтоб не дублировать каждый раз, а получать connection

[Валерий]

Дмитрий, Отлично замечание, спасибо.

[Дмитрий]

Валерий, вот ещё пример дублирования кода
https://github.com/ApXNTekToP/MerchTelegramBot/blo...
Все что ниже меняется только одна строчка, вынести в метод с аргументом
Ну либо проверки выше, а код без дубля ниже

[Влад Григорьев]

Очень много дублирования кода, в постоянном пересоздании конекта тоже смысла мало

quantity = len([i for i, x in enumerate(response)
if x == cart[index]]) - можно просто на фильтр заменить, ну и enumerate - излишний

f-строках приводить дополнительно строки тоже нет необходимости

Переменные из одной буквы не очень в циклах

id_element[0] - лучше вместо обращения по индексу сделать структуру данных с более понятным названием, можно посмотреть в сторону namedtuple

Если создали классы, то и стоит по дробить методы на более маленькие и вынести весь повторяющий код в них

pattern = r'[A-Za-z0-9\.]+[@][A-Za-z0-9]+[.][A-Za-z]{2,3}'
user_response = re.findall(pattern, self.text)

лучше сразу тогда re.compile - использовать

ну и напоследок
re.findall(r'купить' or r'купить', message.text) - во всех таких регулярках будет проверятся, только первое выражение - всегда, 100%

[Влад Григорьев]

ну и замечаний по факту больше, но уже лень еще расписывать

[longclaps]

pattern = r'[A-Za-z0-9\.]+[@][A-Za-z0-9]+[.][A-Za-z]{2,3}'

Смишной pattern. {2,3}, говоришь? Однобуквенные классы тоже доставляют. Пеши ещо.

Answer 1

[Дмитрий]

Посмотрел краем глаза
1. routing.py: замени регулярки на что-нибудь более читабельное, например: if 'some' in text:

2.main.py: class Register: вынеси компиляцию регулярки в начало кода:

import re

MY_RE = re.compile(f'some pattern')

class Register:
user_response = MY_RE.findall(self.text)

3. не оборачивай большие куски кода в try-except

4. Сделай над собой усилие и откажись от global

5. Это что-то страшное:

# Keep main program running while bot runs threaded
if __name__ == "__main__":
    while True:
        try:
            sleep(120)
        except KeyboardInterrupt:
            break

6. Используй f-стринги, вместо format

7. Зачем класс наследует классметод?
class InterfaceInteraction(classmethod):

8. Сделай функции короче, пусть лучше их будет больше, чем такие простыни