HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Representing the opening and closing time for a business

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thebusinesstimeforandopeningclosingrepresenting

Problem

I have an OpenClose class which just represents the hours of operation of a business by the opening and closing time. It takes the opening and closing times as arguments to its constructor, and each is a datetime object.

The data is coming from an external source in a string, formatted like "HH:MM (AM|PM)-HH:MM (AM|PM)"

I have the following function to turn this into an OpenClose object:

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


What do you think of this? Is it too much for one line? Too confusing? Should I split it up into multiple lines?

It sort of bugged me when doing it with multiple lines, but I can certainly see how this would be more readable, despite my hatred of explicitly doing the datetime calculation twice:

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)


An alternative would be to use a combination of these approaches:

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)


Which of these is best?

Solution

Yes, too much for one line, yes, too confusing. Making a map an lambda for two values like that is silly, unless you are trying to win an obfuscation contest.

So, the middle version is best.

Context

StackExchange Code Review Q#3213, answer score: 6

Revisions (0)

No revisions yet.