1
\$\begingroup\$

I created this countdown timer as a tool to count down the remaining time on a test a few months ago. However, I looked at the code today and it looks like it needs some improvement, but I'm not sure what to fix.

Some problems I've already identified:

  • It makes a call to a macOS-specific command: say.

  • It uses the bell character to make noise (which generally seems like a nuisance).

import os
import time
import sys
class cl: # Thank you SO for these colors
    HEADER = '\033[95m'
    OKBLUE = '\033[94m'
    OKGREEN = '\033[92m'
    WARNING = '\033[93m'
    FAIL = '\033[91m'
    ENDC = '\033[0m'
    BOLD = '\033[1m'
    UNDERLINE = '\033[4m'
def countdown(secs, graceperiod):
    os.system("clear")
    for i in range (secs, -graceperiod, -1):
        if (i > 0):
            print cl.BOLD + cl.UNDERLINE + "%s:%02d" % (str((i)/60),(i)%60) + cl.ENDC
        elif (i == 0):
            print "STARTING GRACE PERIOD"
        else:
            print "EXTRA TIME: %s:%02d" % (str((-i)/60),(-i)%60)
        time.sleep(1)
        os.system("clear")
    print "\a"
    time.sleep(0.25)
    print "\a"
    time.sleep(0.25)
    print "\a"
    time.sleep(0.25)
    print "\a"
    time.sleep(0.25)
    os.system("say \"Time up! Put down your pencil!\"")
    print "\aTime up!"

raw_input("Ready? Press [Enter] to start. ")
timemins = 2
gracemins = 0
try:
    countdown(timemins,gracemins)
except KeyboardInterrupt:
    print cl.BOLD + cl.FAIL + "\nExiting Countdown" + cl.ENDC
    sys.exit(0)
\$\endgroup\$
3
\$\begingroup\$

Your code has a massive bug.

What do you expect the output of the following to be?

import time
clock = time.clock # Reduce errors introduced by me 
start = clock()
for i in range(100):
    print(clock() - start)
    time.sleep(0.1)

So 100 * 0.1 = 10, so it should be about 10 seconds. I ran this and the last print was 10.616061702852994. This means that you get 6% more time using your program than you would otherwise.

This is a problem. To fix it you should change the amount to sleep by subtracting from it the time it took to run the print. And so you may get:

start = next_t = clock()
for _ in r:
    yield clock() - start
    next_t += d
    delay_t = next_t - clock()
    if delay_t < 0:
        continue
    sleep(delay_t)

This is better, as the print shouldn't exceed the delay it's good enough. But it's not as accurate as I'd like. While 1.101270229875297 may seem pretty accurate you can change it so that it is instead 1.100000042666685 accurate. By changing sleep to only do say 90% of the delay, and you create a while loop to do the rest you can get much more accuracy. This may not be needed, but knowing that the code is pretty accurate is nice.

And so I'd use:

def delay(d, total_time):
    r = range(int(total_time * (1 / d)))
    clock = time.clock
    sleep = time.sleep

    t = start = next_t = clock()
    for _ in r:
        yield t - start
        next_t += d
        t = clock()
        delay_t = (next_t - t) * 0.9
        if delay_t < 0:
            continue

        sleep(delay_t)
        while True:
            if t >= next_t:
                break
            t = clock()

This means that you'll have to change your for loop slightly. You will need to remove the time.sleep, and the output is the equivalent of using range(secs + graceperiod). And so if you put i = int(round(sec - i)) at the beginning of your for loop, it's almost identical.

And so to improve your code further I'd:

  • Remove the brackets around your if statements.
  • Use str.format, and change i = -i in your else.
  • Stop using os.system('clear'), and instead use \r and print '...',.
  • Change your four print '\a''s to be in a for loop.
  • Change cl to Color.
  • Add a main. And use the if __name__ == '__main__': guard.

resulting in:

import os
import time


class Color:
    HEADER = '\033[95m'
    OKBLUE = '\033[94m'
    OKGREEN = '\033[92m'
    WARNING = '\033[93m'
    FAIL = '\033[91m'
    ENDC = '\033[0m'
    BOLD = '\033[1m'
    UNDERLINE = '\033[4m'


def delay(d, total_time):
    r = range(int(total_time * (1 / d)))
    clock = time.clock
    sleep = time.sleep

    t = start = next_t = clock()
    for _ in r:
        yield next_t - start
        next_t += d
        t = clock()
        delay_t = (next_t - t) * 0.9
        if delay_t < 0:
            continue

        sleep(delay_t)
        while True:
            if t >= next_t:
                break
            t = clock()


def countdown(secs, grace):
    prev = ''
    for diff in delay(1, secs + grace):
        i = int(round(secs - diff))
        if i > 0:
            message = '{0.BOLD}{0.UNDERLINE}{1}:{2:>02}{0.ENDC}'
        elif i == 0:
            message = 'STARTING GRACE PERIOD'
        else:
            message = 'EXTRA TIME: {1}:{2:>02}'
            i = -i
        this = message.format(Color, i / 60, i % 60)
        print prev + '\r' + this,
        prev = '\r' + ' ' * len(this)

    for diff in delay(0.25, 1):
        print '\a',
    print '\a\nTime up!'

    os.system('say "Time up! Put down your pencil!"')


def main(secs, grace):
    try:
        raw_input('Ready? Press [Enter] to start. ')
        countdown(secs, grace)
    except EOFError:
        pass
    except KeyboardInterrupt:
        print Color.BOLD + Color.FAIL + '\nExiting Countdown' + Color.ENDC


if __name__ == '__main__':
    main(10, 10)
\$\endgroup\$
  • \$\begingroup\$ Thanks for the tip. I've always wondered why the timing is really off sometimes (I check using the Unix command time), but I have never been able to understand how to correct for these errors. And thanks for the style tips–I need to really look at PEP 8 as suggested by another person on this forum. \$\endgroup\$ – Aditya R Jun 29 '16 at 18:39
2
\$\begingroup\$

Use a loop

print "\a"
time.sleep(0.25)
print "\a"
time.sleep(0.25)
print "\a"
time.sleep(0.25)
print "\a"
time.sleep(0.25)

This is the same actions repeated 4 times:

for _ in range(4):
    print "\a"
    time.sleep(0.25)

This reduces length and repetition.

Making it obvious when a sound is played

In fact it is not obvious that print "\a" makes a noise, I suggest writing it into a function:

def make_noise():
     print "\a"

Now just call make_noise for self-documenting code.

\$\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.