Блог

Я любитель, когда дело доходит до программирования и понимания концепций, поэтому я хотел знать, могу ли я сократить этот код. Я уже слышал о фразе «спагетти-код» раньше, и я изо всех сил стараюсь не попадать на этот путь плохого, беспорядочного кода. Вот мой код:

 if amount[-1] == 's' and amount[:-1].isnumeric():
    amount = int(amount[:-1])
    unit = "seconds"
elif amount[-1] == 'm' and amount[:-1].isnumeric():
    amount = int(amount[:-1])*60
    unit = "minutes"
elif amount[-2:] == 'hr' and amount[:-2].isnumeric():
    amount = int(amount[:-1])*60*60
    unit = "hours"
elif amount[-1] == 'd' and amount[:-1].isnumeric():
    amount = int(amount[:-1])*60*60*24
    unit = "days"
elif reason != None:
    reason = f'{amount} {reason}'
else:
    reason = f'{amount}'
 

Пользователь вводит количество времени (например: 6 часов, 34 минуты, 2 секунды или 1 день) и используется позже в программе. Есть ли хороший способ сократить это?

Мы все были там, я считаю, что одним из хороших способов улучшить это было бы сначала проанализировать проблему и найти закономерность в данных, которые ожидает ваша программа. Один из заметных шаблонов:

• Пользователь всегда вводит строку (потому что «22hr, 2m, 4d» — это буквенно-цифровые значения)
• В допустимых входных данных всегда будут цифры и алфавиты (hr, m, s, d)
• Допустимый ввод всегда будет начинаться с числа

Вот некоторые из шаблонов, которые вы можете вывести из проблемы. Теперь для решения:

• проверка ввода начинается с числа (вы можете выполнить другие проверки вверху)

 if amount[0].isnumeric():
  # remaining logic here
 

Это избавляет от необходимости повторяться при каждом elif

• Затем задайте список словарей разрешенных алфавитных символов, их единицы измерения и их числовое значение

 timeOptions = [
   { 'shortcode': 'm', 'unit': 'minutes', 'value': 60 },
   { 'shortcode': 's', 'unit': 'seconds', 'value': 1 },
   { 'shortcode': 'hr', 'unit': 'hours', 'value': 3600 },
   { 'shortcode': 'd', 'unit': 'days', 'value': 86400 },
]
 

• Затем отфильтруйте числовые значения из строки

 number = ''.join(filter(str.isdigit, amount))
 

• Отфильтруйте значения алфавита из строки

 shortcode = ''.join(filter(str.isalpha, amount))
 

Наконец, в вашем словаре найдите словарную запись, свойство shortcode которой соответствует единице ввода пользователя. Для этого вы можете использовать понимание списка

 timeValue = [x for x in li if x["shortcode"] == shortcode ][0] # refactor to check list length before attempting to access the first item
 

наконец, вы можете делать все, что хотите, с пользовательским вводом и timeValue словарем.

 amount = amount * timeValue["value"]
unit = timeValue["unit"]
 

Вся программа может выглядеть следующим образом:

 if amount[0].isnumeric():
    timeOptions = [
      { 'shortcode': 'm', 'unit': 'minutes', 'value': 60 },
      { 'shortcode': 's', 'unit': 'seconds', 'value': 1 },
      { 'shortcode': 'hr', 'unit': 'hours', 'value': 3600 },
      { 'shortcode': 'd', 'unit': 'days', 'value': 86400 },
    ]
    number = ''.join(filter(str.isdigit, amount))
    shortcode = ''.join(filter(str.isalpha, amount))

    timeValue = [x for x in li if x["shortcode"] == shortcode ]

    if len(timeValue):
      return False
    else:
      timeValue = timeValue[0]

    amount = amount * timeValue["value"]
    unit = timeValue["unit"]
  
 

Это проблема поиска, поэтому лучше относиться к ней как к таковой. Я не совсем уверен, для какой единицы используется, поэтому я оставил ее, хотя функцию можно упростить, если требуется только количество секунд.

С точки зрения «сокращения кода» стабильность и удобочитаемость часто являются наиболее важными факторами. Следовательно, большая выгода получается от разделения этой логики на отдельную функцию.

 import re
from dataclasses import dataclass

@dataclass
class ParseResult:
    seconds:int
    original_unit:str

TIME_UNITS = {
    's': ('seconds',1),
    'm': ('minutes',60),
    'hr': ('hours',60*60),
    'd': ('days',60*60*24)
}

def parse_time(text):
    # match digits and then not digits
    match = re.match(r'(d )(D )',text)
    if not match:
        raise ValueError(f'Invalid input format for "{text}"')
    amount,unit = match.groups()
    if unit not in TIME_UNITS:
        raise ValueError(f'Invalid unit "{unit}"')
    unit_name, seconds_per_unit = TIME_UNITS[unit]
    seconds = seconds_per_unit * int(amount)
    return ParseResult(seconds,unit_name)
 

Наблюдения из вашего кода: у вас есть amount в виде строки. Внутри вашего оператора if вы меняете его на целое число. Вам следует рассмотреть возможность использования другой переменной для хранения значения.

Кроме того, для условий вы можете использовать словарь и сохранять шаблоны. Затем найдите ключ и используйте значения для вычислений.

Я использовал цикл for с несколькими примерами для тестирования кода. Результаты также являются общими.

Вот измененный код, который немного сокращен.

 for amount in ['20s','20m','20hr','20d','20r','20a','20','abc','as','ad','ahr','am']:

    #setup code to initialize some key variables

    amt, unit = 0, ''
    if amount == '20r': reason = None
    else: reason = ''

    #logic for your part of the code

    dx = {'s':('seconds',1),'m':('minutes',60),'d':('days',60*60*24),'hr':('hours',3600)}

    if amount[-1] in dx.keys() and amount[:-1].isnumeric():
        unit = dx[amount[-1]][0]
        amt = int(amount[:-1])* dx[amount[-1]][1]
    elif amount[-2:] in dx.keys() and amount[:-1].isnumeric():
        unit = dx[amount[-2:]][0]
        amt = int(amount[:-2])* dx[amount[-2:]][1]
    elif reason != None:
        reason = f'{amount} {reason}'
    else:
        reason = f'{amount}'

    #logic for your code ends here

    #The rest is to print the results of the logic from above

    print ('amount :',amount, ',amt :',amt, ',unit :',unit, ',reason :',reason)
 

Вывод из этого кода:

 amount : 20s ,amt : 20 ,unit : seconds ,reason : 
amount : 20m ,amt : 1200 ,unit : minutes ,reason : 
amount : 20hr ,amt : 0 ,unit :  ,reason : 20hr 
amount : 20d ,amt : 1728000 ,unit : days ,reason : 
amount : 20r ,amt : 0 ,unit :  ,reason : 20r
amount : 20a ,amt : 0 ,unit :  ,reason : 20a 
amount : 20 ,amt : 0 ,unit :  ,reason : 20 
amount : abc ,amt : 0 ,unit :  ,reason : abc 
amount : as ,amt : 0 ,unit :  ,reason : as 
amount : ad ,amt : 0 ,unit :  ,reason : ad 
amount : ahr ,amt : 0 ,unit :  ,reason : ahr 
amount : am ,amt : 0 ,unit :  ,reason : am