Explicitly calling map with a lambda isn't usually necessary; using a list comprehension is usually more natural/Pythonic. Therefore you'd have this (which is very similar to your third iteration):
def __get_times(self, hours):
open_time, close_time = [datetime.strptime(s, "%I:%M %p") for s in hours.split("-")]
return OpenClose(open_time, close_time)
I didn't use the asterisk because it obfuscates for the reader what the expected call to the
OpenClose constructor is.
In two of your examples, you've reused a variable to hold different value types. While perfectly legal, I think this reduces readability:
open_time, close_time = hours.split("-")
open_time = datetime.strptime(format)
close_time = datetime.strptime(format)
hours = hours.split("-")
I'm interested in what variables are declared and what they are meant to store. So if asked, what does
hours mean, instead of a simple answer you have to say: first it holds a string, then later it holds a two-element list of strings. Similarly, first
open_time holds a string, then later it holds a datetime instance.
On the subject of brevity, I never think of brevity as a first-class virtue, but it definitely plays into readability. I'd rather read something short than something long, but making something too short can of course detract from readability.
But here's where things get dicey. I think we're maybe focusing on the wrong question. One line or five, I think there's something wrong with your program.
Some class has a private method called
__get_times which takes one string argument, doesn't rely on current state, and returns an
OpenClose instance. At a minimum, we shouldn't call it
__get_times because that name implies that we'll get a return of multiple time values, but that's not what we get at all.
But why does the
OpenClose class exist at all? It's just a pair of values; I can't imagine any legitimate behaviors/methods that should be owned by it. If brevity/readability is your goal, then it would certainly be enhanced by getting rid of near-useless classes. Yes, sometimes bundling values together into a structure can reduce mental overhead, but in this case it doesn't seem worth it. Whoever owns an
OpenClose instance should probably just own an
open and a
close value, neither of which should be a new custom type.
And for that matter, whoever owns
__get_times with a single string argument, why do they need a 1 or 2 line method which should probably only be called once?
Then there's the part (and I can't stress this enough) where it doesn't rely on any internal state. At best, this whole thing should be a free-standing function which takes one string argument and returns a pair of time instances (NOT datetimes; you're talking about time of day). It's also more easy to test, tweak, and verify this way.
def parse_open_close_times(range_string, format="%I:%M %p"):
"""Returns a pair of time-of-day instances based on RANGE_STRING."""
return [datetime.strptime(s.strip(), format).time() for s in range_string.split("-")]
In short, brevity/readability is better addressed by not proliferating questionable classes than by playing code golf with methods with odd signatures.