0
\$\begingroup\$

This is the program I have written to return the list of keys if the current hour is present within the range provided.

# time_mapping has data in key:value pair 
# where value consists of time range in 24 Hr format.

# For key 'abc', the time range is 6AM (i.e. 6) to 3PM (i.e. 15)
# For key 'def', the time range is 2PM (i.e. 14) to 11PM (i.e. 23)
# For key 'pqr', the time range is 5PM (i.e. 17) to 2AM (i.e. 2)
# For key 'xyz', the time range is 10PM (i.e. 22) to 7AM (i.e. 7)

time_mapping = {'abc': ((6,), (15,)), 'def': ((14,), (23,)), 
                'pqr': ((17, 24), (0, 2)), 'xyz': ((22, 24), (0, 7))}


def get_keys(current_time_in_hr):
    list_of_keys = []
    for key, time_range in time_mapping.items():
        if len(time_range[0]) == 1 and len(time_range[1]) == 1:
            if time_range[0][0] <= int(current_time_in_hr) < time_range[1][0]:
                list_of_keys.append(key)
        elif len(time_range[0]) == 2 and len(time_range[1]) == 2:
            if (time_range[0][0] <= int(current_time_in_hr) < time_range[0][1]) \
                or (time_range[1][0] <= int(current_time_in_hr) < time_range[1][1]):
                list_of_keys.append(key)
    return list_of_keys


if __name__ == "__main__":
    current_hour = datetime.now().strftime("%H") # if 15
    print(get_keys(current_hour)) # Then print ['def', 'pqr']

Basically, if the current hour is 2 PM i.e. 14 then it will return ['abc', 'def'].

Well, the program works but I am not happy the way I have written it. I will be glad if someone can review it and share some pointers with me to improve it.

Thank you.

\$\endgroup\$
5
  • \$\begingroup\$ Please describe time_mapping in detail - what those elements mean and why you need them in real life. \$\endgroup\$
    – Reinderien
    Nov 12, 2023 at 13:13
  • \$\begingroup\$ @Reinderien, sorry I missed that. Basically, time_mapping consists of key:value pair data where value is a time range in 24hr format. So, for example, for key abc, the time range is 6AM(i.e. 6) to 3PM(i.e. 15). I hope this will clear your doubt. \$\endgroup\$ Nov 12, 2023 at 13:30
  • \$\begingroup\$ Not really - what does the "key" represent in real life? Components of a recipe for guacamole? \$\endgroup\$
    – Reinderien
    Nov 12, 2023 at 13:32
  • \$\begingroup\$ @Reinderien, it can be anything, nothing specific or important. \$\endgroup\$ Nov 12, 2023 at 13:35
  • \$\begingroup\$ If it's nothing specific or important, what is the point of this code existing? \$\endgroup\$
    – Reinderien
    Nov 12, 2023 at 13:37

1 Answer 1

1
\$\begingroup\$

time_mapping is data lasagna. Whereas it's good that you've added documentation, it's not enough - the elements are still untyped. The more helpful thing to do is represent it as a simple sequence (immutable tuple) of well-typed, immutable class instances (NamedTuple is easiest). Representing it as a dictionary is not useful because you never actually perform a key lookup.

get_keys should probably not accept a current time in hours as an integer, but instead, a datetime.time. It can be further simplified by acting as an iterator instead of materializing a list.

Don't hard-code for the two cases with either one or two ranges. Instead, just code for an arbitrary number of ranges.

Don't strftime to get the hour from a datetime.

Suggested

import datetime
from typing import NamedTuple, Iterator


class TimePair(NamedTuple):
    key: str
    hour_ranges: tuple[range]

    @classmethod
    def from_hours(cls, key: str, start: int, end: int) -> 'TimePair':
        """
        :param key: Passed verbatim to TimePair.key
        :param start: Starting hour, inclusive.
        :param end: Ending hour, inclusive. If the next day, implies two ranges.
        """
        if start <= end:
            ranges = (range(start, 1 + end), )
        else:
            ranges = (range(start, 24), range(0, 1 + end))
        return cls(key, ranges)

    def __contains__(self, time: datetime.time) -> bool:
        return any(time.hour in trange for trange in self.hour_ranges)


time_mapping = (
    TimePair.from_hours('abc', 6, 15),
    TimePair.from_hours('def', 14, 23),
    TimePair.from_hours('pqr', 17, 2),
    TimePair.from_hours('xyz', 22, 7),
)


def get_keys(current_time: datetime.time) -> Iterator[str]:
    for tmap in time_mapping:
        if current_time in tmap:
            yield tmap.key


def main() -> None:
    time = datetime.datetime.now().time()
    for key in get_keys(time):
        print(key)


if __name__ == '__main__':
    main()

Indexing

Depending on a lot of things, it may be worthwhile collapsing the time mapping to an index sequence. This will accelerate lookup at the cost of a small amount of memory and a small amount of startup time:

import datetime
from collections import defaultdict
from typing import NamedTuple, Iterable


class TimePair(NamedTuple):
    key: str
    hour_ranges: tuple[range]

    @classmethod
    def from_hours(cls, key: str, start: int, end: int) -> 'TimePair':
        """
        :param key: Passed verbatim to TimePair.key
        :param start: Starting hour, inclusive.
        :param end: Ending hour, inclusive. If the next day, implies two ranges.
        """
        if start <= end:
            ranges = (range(start, 1 + end), )
        else:
            ranges = (range(start, 24), range(0, 1 + end))
        return cls(key, ranges)

    def __contains__(self, time: datetime.time) -> bool:
        return any(time.hour in trange for trange in self.hour_ranges)


def make_index(time_mapping: Iterable[TimePair]) -> tuple[str]:
    index = tuple([] for _ in range(24))
    for tmap in time_mapping:
        for trange in tmap.hour_ranges:
            for hour in trange:
                index[hour].append(tmap.key)
    return index


time_mapping = (
    TimePair.from_hours('abc', 6, 15),
    TimePair.from_hours('def', 14, 23),
    TimePair.from_hours('pqr', 17, 2),
    TimePair.from_hours('xyz', 22, 7),
)
time_index = make_index(time_mapping)


def get_keys(current_time: datetime.time) -> list[str]:
    return time_index[current_time.hour]


def main() -> None:
    time = datetime.datetime.now().time()
    for key in get_keys(time):
        print(key)


if __name__ == '__main__':
    main()

time_index looks like

(['pqr', 'xyz'],
 ['pqr', 'xyz'],
 ['pqr', 'xyz'],
 ['xyz'],
 ['xyz'],
 ['xyz'],
 ['abc', 'xyz'],
 ['abc', 'xyz'],
 ['abc'],
 ['abc'],
 ['abc'],
 ['abc'],
 ['abc'],
 ['abc'],
 ['abc', 'def'],
 ['abc', 'def'],
 ['def'],
 ['def', 'pqr'],
 ['def', 'pqr'],
 ['def', 'pqr'],
 ['def', 'pqr'],
 ['def', 'pqr'],
 ['def', 'pqr', 'xyz'],
 ['def', 'pqr', 'xyz'])
\$\endgroup\$
1
  • \$\begingroup\$ Great! You are genius! I really want to learn how to code like this, how to think like this. \$\endgroup\$ Nov 13, 2023 at 7:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.