0
\$\begingroup\$

This is just a simple function to convert seconds to days, hours, minutes and seconds for a larger app I'm going to be building. I'm just looking for thoughts on it.

import math


def convert_seconds(x):

    # Day's
    days = x / 86400
    fraction, whole = math.modf(days)
    days = int(whole)

    # Hours
    hours = fraction * 24
    fraction, whole = math.modf(hours)
    hours = int(whole)

    # Minutes
    minutes = fraction * 60
    fraction, whole = math.modf(minutes)
    minutes = int(whole)

    # Seconds
    seconds = fraction * 60
    fraction, whole = math.modf(seconds)
    seconds = int(whole)

    return days, hours, minutes, seconds

if True:
    print(convert_seconds(1043573894573948573))
\$\endgroup\$
3
\$\begingroup\$

Your code is lacking in the regards that you do floating arithmetic when you don't need to, and this potentially introduces smaller errors as you multiply back the different intervals.

With that being said your code looks clean enough, well spaced and good variable names. Only thing missing as such would be a docstring to document the functions return value as the name is somewhat ambiguous.

To avoid the floating arithmetic, and ease the calculation I would flip the order of doing divmod operations, and then we end up with the following code:

def convert_to_d_h_m_s(seconds):
    """Return the tuple of days, hours, minutes and seconds."""

    minutes, seconds = divmod(seconds, 60)
    hours, minutes = divmod(minutes, 60)
    days, hours = divmod(hours, 24)

    return days, hours, minutes, seconds


if __name__ == '__main__':

    for seconds in [ 11, 61, 3601, 3668, 86400, 86401, 90061, 1043573894573948573 ]:
        print("{0} seconds: {1[0]} days, {1[1]} hours, {1[2]} minutes, {1[3]} seconds".format(
              seconds, convert_to_d_h_m_s(seconds)))

Notice how this also uses the if __name__ == '__main__': idiom to pack the test code used. This would allow for the convert function to be imported from another script if imported. The output of this code is:

11 seconds: 0 days, 0 hours, 0 minutes, 11 seconds
61 seconds: 0 days, 0 hours, 1 minutes, 1 seconds
3601 seconds: 0 days, 1 hours, 0 minutes, 1 seconds
3668 seconds: 0 days, 1 hours, 1 minutes, 8 seconds
86400 seconds: 1 days, 0 hours, 0 minutes, 0 seconds
86401 seconds: 1 days, 0 hours, 0 minutes, 1 seconds
90061 seconds: 1 days, 1 hours, 1 minutes, 1 seconds
1043573894573948573 seconds: 12078401557568 days, 20 hours, 22 minutes, 53 seconds
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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