Как улучшить код?

Question

[Onigire]

Здравствуйте, пишу бота который в ответ на цифру - кидает ссылку, зависящую от этой цифры.

@bot.message_handler(content_types=['text'])
def get_name(message):
    user_message = message.text
    if not first_time and user_message.isdigit() and 0 < int(user_message) < len(series)+1:
        with open('series.txt', 'r') as series_names:
            for name in series_names:
                value = name.split(" ")
                if int(user_message) == int(value[0]):
                    link = f'jut.su/one-piece/episode-{user_message}.html'
                    bot.send_message(message.chat.id, f'{name}\n{link}')

Общий вид текстового документа...

1 серия: Я - Луффи! Я стану Королём Пиратов!
2 серия: Появление великого фехтовальщика! Охотник на пиратов Ророноа Зоро
20 серия: Прославленный кок! Санджи из плавучего ресторана
933 серия: Гюкимару! Дуэль Зоро на воровском мосту

Все работает правильно, но я уверен что можно написать лучше и короче. Скажите как и где вы бы написали иначе.

Answer 1

[Василий Банников]

Просто сделай словарь, где ключом будет номер серии, а значением - все данные о ней, включая название и прочее.
Если словарь получится слишком большим, то сохрани данные в бд. (но в этом нет смысла, тк инфа о всех сериях one piece должна спокойно уместиться в 1мб, а если добавить описания каждой серии и кадры - тогда да, будет много)
Не гоже постоянно в цикле проходить по всему списку и парсить строки.

for name in series_names:
                value = name.split(" ")

Делать if int(user_message) == int(value[0]) тоже нет смысла - можно просто сравнивать строки.
И тем более нет смысла на каждый запрос читать весь файл with open('series.txt', 'r') as series_names:

Ссылки следует начинать с https:// - так телега сразу сделает её кликабельной. Ссылки в принципе тоже можно кэшировать, вместо со всеми остальными данными - она не меняется + есть шанс, что когда-то ссылка изменится и уже не будет номером серии.

link = f'jut.su/one-piece/episode-{user_message}.html'

Тоесть в итоге будет как-то так:

# Где-то в main
global one_piece_episodes
one_piece_episodes = {}
with open('series.txt', 'r') as episodes_names:
  for episode_name in episodes_names:
    value = name.split(' ')
    key = value[0]
    name = value[1]
    link = f'https://jut.su/one-piece/episode-{key}.html'
    one_piece_episodes[key] = { "number": key, "name": name, "link": link }

А потом уже при обработке сообщений:

@bot.message_handler(content_types=['text'])
def get_name(message):
    episode = one_piece_episodes.get(message.text)
    if episode != None:
        bot.send_message(message.chat.id, f'{episode["name"]}\n{episode["link"]}')

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

PS: Хабр Q&A не является сервисом для проведения код-ревью, и за подобные "вопросы" можно словить бан по причине 3.2, 3.7, 5.12

Comments

[Onigire]

Я понял, больше не буду

[Onigire]

Спасибо большое ❤ я учту все вышеупомянутое)

if int(user_message) == int(value[0])
Я сначала просто не понял как можно проверять не всю строку а срез, ведь серии могут быть 1,2,3-значным.
Но пока читал я догадался:

for name in series_names:
    if user_message == name[:len(user_message)]:
    break

Но опять же как вы сказали это все так же неефективно и лучше использовать словари.