2
\$\begingroup\$

My first actual code using GUI. Based off a class I took. Code criticism much appreciated. (I was all over the place with this one)

Resources

Picture:

tomato.png

Sound: https://www.freesoundslibrary.com/success-sound-effect/

Put these in the same directory as main.py

main.py

from tkinter import *
import math
from pygame import mixer
mixer.init()

# ---------------------------- CONSTANTS ------------------------------- #
PINK = "#e2979c"
RED = "#e7305b"
GREEN = "#9bdeac"
YELLOW = "#f7f5dd"
FONT_NAME = "Courier"
WORK_MIN = 25
SHORT_BREAK_MIN = 5
LONG_BREAK_MIN = 20
reps = 1
timer = None
mixer.music.load('success-sound-effect.mp3')

def focus_window():
    window.attributes('-topmost', 1)
    window.focus_force()
    window.attributes('-topmost', 0)

# ---------------------------- TIMER RESET ------------------------------- # 
def reset_timer():
    global timer
    global reps
    window.after_cancel(timer)
    checkmark.config(text='')
    timer_label.config(text='Timer', fg=GREEN)
    canvas.itemconfig(timer_text, text='00:00')
    start_button.config(state="normal")
    reps = 1

# ---------------------------- TIMER MECHANISM ------------------------------- # 

def start_timer():
    global reps

    start_button.config(state="disabled")

    work_sec = WORK_MIN * 60
    short_sec = SHORT_BREAK_MIN * 60
    long_sec = LONG_BREAK_MIN * 60

    if reps % 2 != 0:
        reps += 1
        countdown(work_sec)
        timer_label.config(text='Work', fg=GREEN)
    else:
        if reps == 8:
            reps = 1
            countdown(long_sec)
            timer_label.config(text='Break', fg=RED)
        else:
            reps += 1
            countdown(short_sec)
            timer_label.config(text='Break', fg=PINK)


# ---------------------------- COUNTDOWN MECHANISM ------------------------------- # 
def countdown(count):
    global reps

    count_min = math.floor(count/60)
    count_sec = count % 60

    canvas.itemconfig(timer_text, text=f'{count_min:02}:{count_sec:02}')
    if count > 0:
        global timer
        timer = window.after(1000, countdown, count - 1)
    else:
        if reps % 2 == 0: checkmark.config(text="✔" * int(reps / 2))
        mixer.music.play()
        focus_window()
        start_timer()


# ---------------------------- UI SETUP ------------------------------- #

window = Tk()
window.title('Work Timer')
window.config(padx=100, pady=50, bg=YELLOW)
window.resizable(width=False, height=False)

tomato_img = PhotoImage(file='tomato.png')
canvas = Canvas(width=200, height=224, bg=YELLOW, highlightthickness=0)
canvas.create_image(100, 112, image=tomato_img)
timer_text = canvas.create_text(100, 130, text='00:00', fill='white', font=(FONT_NAME, 35, 'bold'))
canvas.grid(column=1, row=1)

timer_label = Label(text='Timer', bg=YELLOW, fg=GREEN, font=(FONT_NAME, 55, 'bold'))
timer_label.grid(column=1, row=0)

start_button = Button(text='Start', highlightthickness=0, command=start_timer)
start_button.grid(column=0,row=2)

reset_button = Button(text='Reset', highlightthickness=0, command=reset_timer)
reset_button.grid(column=2,row=2)

checkmark = Label(text='',fg=GREEN, bg=YELLOW, font=(FONT_NAME, 25, 'bold'))
checkmark.grid(column=1, row=3)

window.mainloop()
\$\endgroup\$
4
  • \$\begingroup\$ Several times you convert between seconds and minutes. It's not hard to do. But there's an opportunity to import unyts to improve clarity. \$\endgroup\$
    – J_H
    Jul 6, 2023 at 20:49
  • \$\begingroup\$ Consider using enum type for the color constants \$\endgroup\$ Jul 8, 2023 at 11:02
  • \$\begingroup\$ Can you please explain the functioning of reps? This looks to be a very specific sequence that has not been documented. \$\endgroup\$
    – Reinderien
    Jul 11, 2023 at 15:03
  • \$\begingroup\$ reps basically determines how long the pomodoro timer will run. On rep 1 = 25m, rep 2 = 10m, and so on, until we get to rep 8 where we take a long break of 20m and then we start again. Note: I don't really know what said sequences are or where I can find their documentation. \$\endgroup\$
    – Beginner
    Jul 12, 2023 at 5:07

1 Answer 1

3
\$\begingroup\$

Avoid importing * from anything; this produces namespace pollution.

Consider making an Enum to represent your three different work modes.

You should try separating your business logic from your display code. I show a partial implementation of this below. You could also probably move the actual timer expiry to the business logic class, though that's a grey area since you currently pull the timer functionality from tkinter.

Your colour values being constant is fine. Your timer spans should probably be a little bit more configurable, perhaps as parameters to your functions.

You have a bunch of shared state floating around; this makes your code non-reentrant since it's stored in globals. The easiest way to improve this is to move the state to classes.

Avoid initializing things like mixer from the global namespace. If you want to unit-test this or add it to a broader user interface, you'll want to pull symbols from this module selectively and only run the initialization that you want. Move such initialization to a __main__ guard.

Replace int(reps / 2) with floor division //.

Since you have a fixed timer expiry, your timer error will grow over time. This won't be very noticeable, but it occurs due to jitter from the operating system's thread manager and other assorted overhead. The typical solution is to base the timer expiry on a measured difference between old and new system monotonic time values. I have not demonstrated this but it's straightforward enough.

Suggested

import tkinter as tk
from datetime import timedelta
from enum import Enum
from typing import Optional

from pygame import mixer


def format_timedelta(t: timedelta) -> str:
    min, sec = divmod(t.total_seconds(), 60)
    return f'{min:02.0f}:{sec:02.0f}'


class SegmentMode(Enum):
    WORK = 'Work'
    SHORT_BREAK = 'Short Break'
    LONG_BREAK = 'Long Break'

    @property
    def is_break(self) -> bool:
        return self in {self.LONG_BREAK, self.SHORT_BREAK}


class PomodoroTimer:
    def __init__(
        self,
        work_interval: timedelta = timedelta(seconds=25),  # todo: minutes
        short_break_interval: timedelta = timedelta(seconds=5),
        long_break_interval: timedelta = timedelta(seconds=20),
    ) -> None:
        self.intervals = {
            SegmentMode.WORK: work_interval,
            SegmentMode.SHORT_BREAK: short_break_interval,
            SegmentMode.LONG_BREAK: long_break_interval
        }
        self.reps = 1
        self.mode = SegmentMode.WORK

    def reset(self) -> None:
        self.reps = 1

    @property
    def interval_length(self) -> timedelta:
        return self.intervals[self.mode]

    def next_segment(self) -> None:
        if self.reps % 2 != 0:
            # If the number of timer repeats is odd, start working
            self.reps += 1
            self.mode = SegmentMode.WORK
        elif self.reps == 8:
            # If the number of timer repeats is 8, start a long break
            self.reps = 1
            self.mode = SegmentMode.LONG_BREAK
        else:
            # In all other cases, start a short break
            self.reps += 1
            self.mode = SegmentMode.SHORT_BREAK


class TimerInterface:
    FONT_NAME = 'Courier'
    PINK = '#e2979c'
    RED = '#e7305b'
    GREEN = '#9bdeac'
    YELLOW = '#f7f5dd'
    BACKGROUND = YELLOW

    INTERVAL_COLOURS = {
        SegmentMode.WORK: GREEN,
        SegmentMode.SHORT_BREAK: PINK,
        SegmentMode.LONG_BREAK: RED,
    }

    def __init__(self, timer: PomodoroTimer) -> None:
        mixer.init()
        mixer.music.load('success-sound-effect.mp3')

        self.timer = timer
        self.timer_id: Optional[str] = None

        self.window = tk.Tk()
        self.window.title('Work Timer')
        self.window.config(padx=100, pady=50, bg=self.BACKGROUND)
        self.window.resizable(width=False, height=False)
        self.main_loop = self.window.mainloop

        # A reference needs to be kept or else it will be garbage-collected and fail to display
        self.image = tk.PhotoImage(file='tomato.png')

        self.canvas = tk.Canvas(width=200, height=224, bg=self.BACKGROUND, highlightthickness=0)
        self.canvas.create_image(100, 112, image=self.image)
        self.canvas.grid(column=1, row=1)

        self.timer_text = self.canvas.create_text(
            100, 130,
            text='00:00', fill='white', font=(self.FONT_NAME, 35, 'bold'),
        )

        self.timer_label = tk.Label(
            text='Timer', bg=self.BACKGROUND, fg=self.GREEN,
            font=(self.FONT_NAME, 55, 'bold'),
        )
        self.timer_label.grid(column=1, row=0)

        self.start_button = tk.Button(text='Start', highlightthickness=0, command=self.start_timer)
        self.start_button.grid(column=0, row=2)

        reset_button = tk.Button(text='Reset', highlightthickness=0, command=self.reset_timer)
        reset_button.grid(column=2, row=2)

        self.checkmark = tk.Label(
            fg=self.GREEN, bg=self.BACKGROUND, font=(self.FONT_NAME, 25, 'bold'),
        )
        self.checkmark.grid(column=1, row=3)

    def focus_window(self) -> None:
        self.window.attributes('-topmost', 1)
        self.window.focus_force()
        self.window.attributes('-topmost', 0)

    def reset_timer(self) -> None:
        self.window.after_cancel(self.timer_id)
        self.checkmark.config(text='')
        self.timer_label.config(text='Timer', fg=self.GREEN)
        self.canvas.itemconfig(self.timer_text, text='00:00')
        self.start_button.config(state='normal')
        self.timer.reset()

    def start_timer(self) -> None:
        self.start_button.config(state='disabled')
        self.timer.next_segment()
        self.countdown(self.timer.interval_length)
        self.timer_label.config(text=self.timer.mode.value, fg=self.INTERVAL_COLOURS[self.timer.mode])

    def countdown(self, count: timedelta) -> None:
        self.canvas.itemconfig(self.timer_text, text=format_timedelta(count))

        if count > timedelta():
            self.timer_id = self.window.after(1000, self.countdown, count - timedelta(seconds=1))
        else:
            self.timer_id = None
            if self.timer.mode.is_break:
                self.checkmark.config(text='✔' * (self.timer.reps // 2))
            mixer.music.play()
            self.focus_window()
            self.start_timer()


def main() -> None:
    timer = PomodoroTimer()
    interface = TimerInterface(timer)
    interface.main_loop()


if __name__ == '__main__':
    main()
\$\endgroup\$

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.