Представляющий время открытия и закрытия бизнеса


У меня есть класс OpenClose, который просто представляет часах работы Бизнес-время открытия и время закрытия. Он принимает открытия и закрытия в качестве аргументов конструктора, и каждый объект datetime.

Данные поступают из внешнего источника в строку, отформатированную как "чч:мм (утро|вечер)-чч:мм (утро|вечер)"

У меня есть следующая функция, чтобы превратить это в объект OpenClose:

def __get_times(self, hours):
    return OpenClose(*map(lambda x: datetime.strptime(x, "%I:%M %p"), hours.split("-")))

Что вы думаете об этом? Это слишком много для одной линии? Слишком запутанно? Я должен разбить его на несколько строк?

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

format = "%I:%M %p"
open_time, close_time = hours.split("-")
open_time = datetime.strptime(format)
close_time = datetime.strptime(format)
return OpenClose(open_time, close_time)

В качестве альтернативы можно использовать сочетание этих подходов:

format = "%I:%M %p"
hours = hours.split("-")
open_time, close_time = map(lambda x: datetime.strptime(x, format), hours)
return OpenClose(open_time, close_time)

Какой из них самый лучший?



Комментарии
4 ответа

Да, слишком много для одной линии, да, слишком запутанно. Делая карту для Lambda для двух значений, как это глупо, если вы пытаетесь выиграть конкурс запутывания.

Так, средний вариант.

6
ответ дан 30 июня 2011 в 06:06 Источник Поделиться

"Никто не выигрывает на гольф-код". Один вкладыши являются просто ошибкой ждать, что произойдет. Они не создают никакого значения, потому что они могут быть непонятными. Они чрезвычайно трудно поддерживать.

Иногда, один-лайнер (т. е. простой список осмысления) очень понятно и имеет смысл.

В этом случае

open_time = datetime.strptime(open_time,format)
close_time = datetime.strptime(close_time,format)

Не достаточно излишним волнует.

Это фундаментальная ошибка.

OpenClose(*map(lambda x: datetime.strptime(x, "%I:%M %p"), hours.split("-")))

Выше есть ошибка жесткого кодирования строки формата в заявлении.
Строка формата является, скорее всего, что нужно изменить, и должны быть в объекте конфигурации какой-то, так что он может быть найден и легко изменить. Если не в объект конфигурации, то на уровне класса атрибута. Или модуль уровня глобальных.

Вторая самая распространенная мутация идти от того, что у вас есть (что не справиться с ValueError) к этому.

try: 
open_time= datetime.strptime(open_time,format)
except ValueError, e:
log.error( "Open time in %s is invalid: %s", hours, open_time )
return
try:
close_time = datetime.strptime(close_time,format)
except ValueError, e:
log.error( "Close time in %s is invalid: %s, hours, close_time
return

Это не лучше.

OpenClose( *map(lambda x: datetime.strptime(x, format), hours.split("-")) )

И это тоже.

OpenClose( *[datetime.strptime(x, format) for x in hours.split("-")] )

Короче, но не лучше.

5
ответ дан 12 июля 2011 в 10:07 Источник Поделиться

Одна из рекомендаций чистой код: не возвращают выражения, лишь возвращаемых переменных (или констант или объектов). Применяя только что руководство диктует разделение на 2 линии.

Вопрос в задать для любой реализации-это:


  • Это повышает производительность, если да, то как? См http://wiki.python.org/moin/PythonSpeed/PerformanceTips

  • Текущий код не дает улучшения производительности поставить все на одной линии..

  • Поэтому он может быть оптимизирован для лучшей читабельности.. (меньше багов)

  • И она может быть оптимизирована для лучшей ремонтопригодности.. (меньше багов)


    • возвращение() с простой аргумент легко читаемый для программы (никаких странных побочных эффектов легко увидеть) = лучшей читаемости

    • Добавление кода до возврата() (например, обработка исключений или новый возвращаемых значений) может быть сделано путем создания логических выражений или добавления новых побочных эффектов = лучше ремонтопригодность


      • Как правило, человек, следящий за код (добавление нового возвращаемое значение) не могут быть знакомы с алгоритмом или оригинальном исполнении, поэтому существует высокая вероятность ошибки внедряется..



См. также :


Модели Питон - Анекдот Оптимизации
http://www.python.org/doc/essays/list2str.html

2
ответ дан 20 июля 2011 в 08:07 Источник Поделиться

Я пойду против течения и скажу, что это нормально, но тесновато. Есть кусок кода, который вы не хотите повторить, и я согласен с твоим мнением. Но поскольку этот кусок кода-это семантическая единица, оно должно сделать больше видимость. Дать ему имя, для документов.

def __get_times(hours):
def parse_time(s): return datetime.strptime(s, "%I:%M %p")
return OpenClose(*map(parse_time, hours.split("-")))

Там еще немного запаха в том, что OpenClose ожидает ровно два аргумента, но раскол может в принципе произвести любое количество. Кроме того, strptime сам может провалиться, если строка была неправильно отформатирована. Когда вы получаете вокруг, чтобы снабжать чистым отчетов об ошибках, вам необходимо сообщить об ошибке, прежде чем называть OpenClose; это будет легче, если вы получите извлечение из пути в первую очередь.

def __get_times(hours):
def parse_time(s): return datetime.strptime(s, "%I:%M %p")
opening, closing = map(parse_time, hours.split("-"))
return OpenClose(opening, closing)

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

def __get_times(hours):
def parse_time(s): return datetime.strptime(s, "%I:%M %p")
times = map(parse_time, hours.split("-"))
if len(times) <> 2: raise …
return OpenClose(*times)

1
ответ дан 25 июля 2011 в 09:07 Источник Поделиться