Get random time between two time inputs

Question

This code takes two times as input and then calculates a random time between the two inputs.

def randomTimeRange():
    print('Get a random time between two times')
    start_time_input = '01:25:00'
    end_time_input = '23:30:43'

    start_time = start_time_input.split(':')
    end_time = end_time_input.split(':')

    start_hour = start_time[0]
    start_minute = start_time[1]
    start_seconds = start_time[2]

    end_hour = end_time[0]
    end_minute = end_time[1]
    end_seconds = end_time[2]

    # Get maximum end time for randrange
    if end_hour == '23' and end_minute != '00':
        max_hour = 23 + 1
    else:
        max_hour = start_hour

    if start_minute > end_minute:
        minutes = randrange(int(end_minute), int(start_minute))
    elif start_minute < end_minute:
        minutes = randrange(int(start_minute), int(end_minute))

    if start_hour == end_hour:
        hours = start_hour
    elif start_hour != end_hour:
        hours = randrange(int(start_hour), int(max_hour))

    if str(hours) == str(end_hour):
        minutes = randrange(int(start_minute), int(end_minute))
    else:
        minutes = randrange(0, 59)

    if start_seconds == end_seconds:
        seconds = start_seconds
    elif start_seconds > end_seconds:
        seconds = randrange(int(start_seconds), int(59))
    elif start_seconds < end_seconds:
        seconds = randrange(int(start_seconds), int(end_seconds))

    h = int(hours)
    m = int(minutes)
    s = int(seconds)

    return f"{h:02d}" + ':' + f"{m:02d}" + ':' + f"{s:02d}"

print(randomTimeRange())

I feel like my code is really bad, how can I fix it?

Answer 1

Use the standard library

You use random, why not datetime?

Separate your concerns

Split input, processing and output into different steps.

Generalize your code

Handling datetime.time is cumbersome, since it does not support timestamp conversion. Consider generalizing your code to accept datetime.datetime objects.

Use the if __name__ == '__main__': guard

Put your script code under the infamous __main__ guard, to prevent it from running when imported by other modules.

Suggested change

"""Prompt the user for times and generate a random time between them."""

from datetime import datetime, time, timedelta
from random import randrange


PROMPT = 'Please enter a {} time in ISO format: '


def read_time(prompt: str) -> time:
    """Reads a time object from the user."""

    return time.fromisoformat(input(prompt))


def rand_datetime(start: datetime, end: datetime) -> datetime:
    """Returns a random datetime between start and end."""

    return datetime.fromtimestamp(randrange(
        round(start.timestamp()), round(end.timestamp())
    ))


def rand_time(start: time, end: time) -> time:
    """Returns a random time between start and end."""

    return rand_datetime(
        datetime.combine(dt0 := datetime.fromtimestamp(0), start),
        datetime.combine(
            dt0 if start < end else dt0 + timedelta(days=1),
            end
        )
    ).time()


def main():
    """Runs the script."""

    start = read_time(PROMPT.format('first'))
    end = read_time(PROMPT.format('second'))
    rand = rand_time(start, end)
    print('Your random time:', rand)


if __name__ == '__main__':
    main()

Answer 2

If you're planning on making this into a command-line script, instead of interactive user input I'd recommend using argparse so you can easily invoke your script like $ <script_name> <start_time> <end_time>, e.g. $ ./foo.py 16:00:00 22:00:00. An example of how argparse is used can be found in the example implementation down below.

---

@Richard Neumann already provided a comprehensive review, and I'll echo the advice of leaning more on the standard library datetime for your time parsing and calculation needs.

The main thing I wanted to offer in this review is another way of thinking about this problem, which I find to be pretty intuitive.

Given a start_time and an end_time, we can calculate the duration in seconds between those times. Once we have that, picking a random time between those times is equivalent to picking one of those seconds in the duration between start_time and end_time.

For example, if the start time is 00:01:00 and the end time is 00:02:00, the duration between the two is 3600 seconds. Then we can pick a random number in [0, 3600], and that random number can then be added to the start time to give us our random time.

See the following script for an example implementation. Note that timedelta handles a lot of the heavy lifting for us, including adding/subtracting times, and handling the wrap-around from 23:59:59 to 00:00:00. Which means, for example, doing 01:00:00 (end time) minus 23:00:00 (start time) would correctly yield a duration of 7200 seconds without any special logic required from our side.

#!/usr/bin/env python3

import argparse
import random
from datetime import datetime, time, timedelta


def to_timedelta(t: time) -> timedelta:
    return timedelta(hours=t.hour, minutes=t.minute, seconds=t.second)


def to_time(seconds: int) -> time:
    return (datetime.min + timedelta(seconds=seconds)).time()


def random_time(start_time: time, end_time: time) -> time:
    start = to_timedelta(start_time)
    end = to_timedelta(end_time)
    duration = (end - start).seconds
    random_offset = random.randint(0, duration)

    return to_time((start + timedelta(seconds=random_offset)).seconds)


if __name__ == "__main__":
    parser = argparse.ArgumentParser(
        description="Print a random time between two given times."
    )
    parser.add_argument(
        "start_time", help="start time (hh:mm:ss)", type=time.fromisoformat
    )
    parser.add_argument(
        "end_time", help="end time (hh:mm:ss)", type=time.fromisoformat
    )
    args = parser.parse_args()

    print(random_time(args.start_time, args.end_time))

Answer 3

Always remember to import modules at start of the script

You code doesn't even run, randrange isn't defined, it will always throws an NameError and terminate the execution abruptly, and whatever code comes after it won't run.

You should add this line at the start of the script:

from random import randrange

Always remember to import at the start of scripts, otherwise the names won't get defined and your script won't function properly.

Make your function accept arguments

Make your function accept arguments, make your function accept arguments, make your function accept arguments...

Functions are blocks of code that transforms inputs using predefined procedures into certain outputs; functions make these procedures easier to reuse. Your function doesn't accept any argument at all, and start_time_input and end_time_input definitely shouldn't be fixed...

And they should be randomized, as your function name suggests...

Use more functions

Instead of one gigantic function, you should divide the steps into parts and write a smaller function for each part. In this way you will have huge improvements in readability and it will make debugging, testing, profiling etc. a LOT easier.

Always validate user input

Your code contains no input validation because your code accepts no user input... Well your code definitely should accept user input and no input validation will very possibly break the code. Always expect the users to be absolutely computer illiterate and prepare for all possibilities of user input.

Use simpler logic

Seriously, your script is extremely inefficient and poorly writtenl the logic presented by your code could be expressed in far fewer lines...

First, time in the format of hh:mm:ss is a 3-digit base-60 number in the range [0, 86400) with a colon ':' as delimiter of the digit fields and the digits are expressed in decimal.

The hour is a number in [0, 23], while minute and second are both in [0, 59], and the hour has a place value of 3600 (60²), the minute 60 and the second 1...

In short, to compare them, just convert them to integers and compare the integers:

h, m, s = t.split(':')
seconds = h * 3600 + m * 60 + s

To format an int into time stamp:

h, r = divmod(n, 3600)
m, s = divmod(r, 60)
time = f'{h:02d}:{m:02d}:{s:02d}'

And if the start is greater than end, just add 86400 to the end, and always mod 86400 the choice before format, problem solved...

Suggested code:

import re
import sys
from random import randrange

timeformat = '^(2[0-3]|[01]?\d):([0-5]?\d):([0-5]?\d)$'

def parse_time(t: str) -> int:
    if not isinstance(t, str):
        raise TypeError('Argument must be str')
    if re.match(timeformat, t):
        h, m, s = t.split(':')
        return h * 3600 + m * 60 + s
    else:
        raise ValueError('Inputted string is not in a correct format')

def time_string(n: int) -> str:
    if not isinstance(n, int):
        raise TypeError('Argument must be int')
    if 0 <= n < 86400:
        h, r = divmod(n, 3600)
        m, s = divmod(r, 60)
        return f'{h:02d}:{m:02d}:{s:02d}'
    else:
        raise ValueError('Inputted integer is out of range')

def randtime(start=None, end=None) -> str:
    if not start:
        start = randrange(86400)
    if not end:
        end = randrange(86400)
    
    if not isinstance(start, int):
        start = parse_time(start)
    if not isinstance(end, int):
        end = parse_time(end)
    
    if start > end:
        end += 86400
    
    chosen = randrange(start, end+1)
    return time_string(chosen%86400)

if __name__ == '__main__':
    print(randtime(*[int(s) if s.isdigit() else s for s in sys.argv[1:]]))

Answer 4

minutes = comes under two if-else. Which means, you are calculating and assigning to minutes two times, seems to be not a good idea. Think about how you can assign value to minutes one time.