6
\$\begingroup\$

I've recently created a tomato timer for terminal. It's hosted in github.

My concerns are how Pythonic my code is? Is the way I'm handling state transitions good? Which areas of the text user interface I can improve? Also I've ordered the files in order of importance: first file is core logic, second ui and third utils.

How it looks like

screenshot

Code

pydoro.pydoro_core.tomato.py

This file is responsible for handling states

import itertools
from enum import IntEnum
from timeit import default_timer

from pydoro.pydoro_core import sound
from pydoro.pydoro_core.util import in_app_path

TOMATOES_PER_SET = 4
SECONDS_PER_MIN = 60
WORK_TIME = 25 * SECONDS_PER_MIN
SMALL_BREAK_TIME = 5 * SECONDS_PER_MIN
LONG_BREAK_TIME = 15 * SECONDS_PER_MIN
ALARM_TIME = 20


TOMATO = [
    ("", "\n      "),
    ("#00cc00", "/'\\/`\\"),
    ("", "         "),
    ("", "task1"),
    ("", "\n    "),
    ("#ff0000", ".-"),
    ("", "  "),
    ("#00cc00", "|/"),
    ("", "  "),
    ("#ff0000", "-."),
    ("", "       "),
    ("", "task2"),
    ("", "\n   "),
    ("#ff0000", "/"),
    ("", "          "),
    ("#ff0000", "\\"),
    ("", "      "),
    ("", "task3"),
    ("", "\n  "),
    ("#ff0000", "'"),
    ("", "   "),
    ("bold", "pydoro"),
    ("", "   "),
    ("#ff0000", "\\"),
    ("", "     "),
    ("", "task4"),
    ("", "\n "),
    ("#ff0000", ";"),
    ("", "             "),
    ("#ff0000", "'"),
    ("", "     "),
    ("", "status"),
    ("", "\n "),
    ("#ff0000", ";"),
    ("", "             "),
    ("#ff0000", ";"),
    ("", "     "),
    ("", "time"),
    ("", "\n "),
    ("#ff0000", ":"),
    ("", "          "),
    ("#ff0000", "/"),
    ("", "  "),
    ("#ff0000", "."),
    ("", "\n  "),
    ("#ff0000", "\\"),
    ("", "       "),
    ("#ff0000", ".'"),
    ("", "  "),
    ("#ff0000", "/"),
    ("", "      "),
    ("", "count"),
    ("", "\n    "),
    ("#ff0000", "\\"),
    ("", " "),
    ("#ff0000", "____"),
    ("", " "),
    ("#ff0000", ".'"),
    ("", "        "),
    ("", "sets"),
]

LOCATIONS = {
    "count": 51,
    "sets": 59,
    "status": 31,
    "task1": 3,
    "task2": 11,
    "task3": 17,
    "task4": 25,
    "time": 37,
}

TEXT_LONG_BREAK = r"""
  ___    LONG      _
 | _ )_ _ ___ __ _| |__
 | _ | '_/ -_/ _` | / /
 |___|_| \___\__,_|_\_\
""".strip(
    "\r\n"
)

TEXT_SMALL_BREAK = TEXT_LONG_BREAK.replace("LONG", "SMALL")

TEXT_WORK = r"""
 __      __       _
 \ \    / ___ _ _| |__
  \ \/\/ / _ | '_| / /
   \_/\_/\___|_| |_\_\
""".strip(
    "\r\n"
)


class Tasks(IntEnum):
    WORK = 1
    SMALL_BREAK = 2
    LONG_BREAK = 3
    NO_TASK = 4
    INTERMEDIATE = 5


class TaskStatus(IntEnum):
    NONE = 111
    STARTED = 222
    PAUSED = 555
    LIMBO = 666


TEXT = {
    TaskStatus.NONE.value: "",
    TaskStatus.STARTED.value: "",
    TaskStatus.PAUSED.value: "PAUSED",
    TaskStatus.LIMBO.value: "",
    Tasks.WORK.value: TEXT_WORK,
    Tasks.SMALL_BREAK.value: TEXT_SMALL_BREAK,
    Tasks.LONG_BREAK.value: TEXT_LONG_BREAK,
    Tasks.NO_TASK.value: "",
    Tasks.INTERMEDIATE.value: "",
}

PROGRESS = ["|#  |", "| # |", "|  #|", "| # |"]


def cur_time():
    return int(default_timer())


def play_alarm():
    # noinspection PyBroadException
    try:
        sound.play(in_app_path("b15.wav"), block=False)
    except Exception:
        pass


class InitialState:
    name = "initial"

    def __init__(self, time_period=0, tomato=None):
        self._time_period = int(time_period)
        self._tomato = tomato
        self._task = Tasks.NO_TASK
        self._status = TaskStatus.NONE
        self._started_at = 0
        self._remainder = 0
        self._progress = itertools.cycle(PROGRESS)

    def start(self):
        play_alarm()
        return WorkingState(tomato=self._tomato)

    def pause(self):
        return self

    def reset(self):
        return self

    @property
    def remainder(self):
        return self._remainder

    @property
    def next_state(self):
        return self

    @property
    def time_period(self):
        return self._time_period

    @property
    def time_remaining(self):
        return "Press [start]"

    @property
    def task(self):
        return self._task

    @property
    def status(self):
        return self._status

    @property
    def done(self):
        return False

    def _format_time(self, remainder):
        minutes, seconds = divmod(int(remainder), SECONDS_PER_MIN)
        if self.status == TaskStatus.STARTED:
            progress = next(self._progress) + " "
        else:
            progress = ""

        return "{}{:00}min {:00}s remaining".format(progress, minutes, seconds)

    def _calc_remainder(self):
        cur = cur_time()
        difference = cur - self._started_at
        remainder = self._remainder - difference
        self._started_at = cur
        if remainder <= 0:
            remainder = 0
        self._remainder = remainder


class IntermediateState(InitialState):
    name = "waiting"

    def __init__(self, time_period=0, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._task = Tasks.INTERMEDIATE
        self._status = TaskStatus.LIMBO
        self._next_factory = None
        self._last_alarm_time = 0
        self._sound()

    def _sound(self):
        if cur_time() - self._last_alarm_time > ALARM_TIME:
            play_alarm()
            self._last_alarm_time = cur_time()

    def start(self):
        return self._next_factory(tomato=self._tomato)

    @property
    def time_remaining(self):
        self._sound()
        return "Press [start] to continue with " + self._next_factory.name

    @property
    def done(self):
        return False

    @staticmethod
    def transition_to(next_state_factory, tomato):
        state = IntermediateState(tomato=tomato)
        state._next_factory = next_state_factory
        return state


class WorkingState(InitialState):
    name = "work"

    def __init__(self, time_period=WORK_TIME, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._remainder = int(self._time_period)
        self._task = Tasks.WORK
        self._status = TaskStatus.STARTED
        self._started_at = cur_time()

    def start(self):
        return self

    @property
    def time_remaining(self):
        self._calc_remainder()

        return self._format_time(self._remainder)

    @property
    def next_state(self):
        self._tomato.tomatoes += 1
        if self._tomato.tomatoes % TOMATOES_PER_SET == 0:
            return IntermediateState.transition_to(LongBreakState, tomato=self._tomato)
        return IntermediateState.transition_to(SmallBreakState, tomato=self._tomato)

    def pause(self):
        return WorkPausedState.return_to(self._tomato, self)

    def reset(self):
        self._remainder = self.time_period
        return self

    @property
    def done(self):
        return self._remainder <= 0


class WorkPausedState(InitialState):
    name = "work paused"

    def __init__(self, time_period=0, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._prev = None
        self._task = Tasks.WORK
        self._status = TaskStatus.PAUSED

    def start(self):
        self._prev._started_at = cur_time()
        return self._prev

    def reset(self):
        self._prev._remainder = self._prev.time_period
        return self

    @property
    def time_remaining(self):
        return self._format_time(self._prev.remainder)

    @staticmethod
    def return_to(tomato, state):
        cur_state = WorkPausedState(tomato=tomato)
        cur_state._prev = state
        return cur_state

    @property
    def done(self):
        return False


class SmallBreakState(InitialState):
    name = "small break"

    def __init__(self, time_period=SMALL_BREAK_TIME, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._remainder = int(self._time_period)
        self._task = Tasks.SMALL_BREAK
        self._status = TaskStatus.STARTED
        self._started_at = cur_time()

    def start(self):
        return self

    @property
    def time_remaining(self):
        self._calc_remainder()

        return self._format_time(self._remainder)

    @property
    def next_state(self):
        return IntermediateState.transition_to(WorkingState, tomato=self._tomato)

    def pause(self):
        return SmallBreakPausedState.return_to(self._tomato, self)

    def reset(self):
        self._remainder = self.time_period
        return self

    @property
    def done(self):
        return self._remainder <= 0


class SmallBreakPausedState(InitialState):
    name = "small break paused"

    def __init__(self, time_period=0, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._task = Tasks.SMALL_BREAK
        self._status = TaskStatus.PAUSED
        self._prev = None

    def start(self):
        self._prev._started_at = cur_time()
        return self._prev

    @property
    def time_remaining(self):
        return self._format_time(self._prev.remainder)

    @staticmethod
    def return_to(tomato, state):
        cur_state = SmallBreakPausedState(tomato=tomato)
        cur_state._prev = state
        return cur_state

    def reset(self):
        self._prev._remainder = self._prev.time_period
        return self

    @property
    def done(self):
        return False


class LongBreakState(SmallBreakState):
    name = "long break"

    def __init__(self, time_period=LONG_BREAK_TIME, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._task = Tasks.LONG_BREAK
        self._status = TaskStatus.STARTED

    def pause(self):
        return LongBreakPausedState.return_to(self._tomato, self)


class LongBreakPausedState(SmallBreakPausedState):
    name = "long break paused"

    def __init__(self, time_period=0, tomato=None):
        super().__init__(time_period=time_period, tomato=tomato)
        self._task = Tasks.LONG_BREAK
        self._status = TaskStatus.PAUSED

    @staticmethod
    def return_to(tomato, state):
        cur_state = LongBreakPausedState(tomato=tomato)
        cur_state._prev = state
        return cur_state


class Tomato:
    def __init__(self):
        self._state = InitialState(tomato=self)
        self.tomatoes = 0

    def start(self):
        self._state = self._state.start()

    def pause(self):
        self._state = self._state.pause()

    def reset(self):
        self._state = self._state.reset()

    def reset_all(self):
        self._state = InitialState(tomato=self)
        self.tomatoes = 0

    def update(self):
        if self._state.done:
            self._state = self._state.next_state

    def as_formatted_text(self):
        task = TEXT[self._state.task.value]
        task = task.splitlines()
        if not task:
            task = [""] * 4

        sets = self.tomatoes // TOMATOES_PER_SET
        if sets == 1:
            sets = "1 set completed"
        elif sets >= 2:
            sets = str(sets) + " sets completed"
        else:
            sets = ""

        status = TEXT[self._state.status.value]
        time = self._state.time_remaining
        count = "(`) " * (TOMATOES_PER_SET - self.tomatoes % TOMATOES_PER_SET)

        ftext = TOMATO[:]
        for i in range(1, 5):
            ftext[LOCATIONS["task" + str(i)]] = ("", task[i - 1])
        ftext[LOCATIONS["status"]] = ("", status)
        ftext[LOCATIONS["time"]] = ("", time)
        ftext[LOCATIONS["count"]] = ("", count)
        ftext[LOCATIONS["sets"]] = ("", sets)

        return ftext

pydoro.pydoro_tui.py

This is the main executable script file and this is responsible for creating prompt-toolkit layout.

#!/usr/bin/env python
import threading

from prompt_toolkit.application import Application
from prompt_toolkit.application.current import get_app
from prompt_toolkit.key_binding import KeyBindings
from prompt_toolkit.key_binding.bindings.focus import focus_next, focus_previous
from prompt_toolkit.layout import HSplit, Layout, VSplit, FormattedTextControl, Window
from prompt_toolkit.styles import Style
from prompt_toolkit.widgets import Box, Button, Label

from pydoro.pydoro_core.tomato import Tomato
from pydoro.pydoro_core.util import every

tomato = Tomato()


def exit_clicked(_=None):
    get_app().exit()


# All the widgets for the UI.
btn_start = Button("Start", handler=tomato.start)
btn_pause = Button("Pause", handler=tomato.pause)
btn_reset = Button("Reset", handler=tomato.reset)
btn_reset_all = Button("Reset All", handler=tomato.reset_all)
btn_exit = Button("Exit", handler=exit_clicked)

# text_area = TextArea(read_only=True, height=11, focusable=False)
text_area = FormattedTextControl(focusable=False, show_cursor=False)
text_window = Window(
    content=text_area, dont_extend_height=True, height=11, style="bg:#ffffff #000000"
)

root_container = Box(
    HSplit(
        [
            Label(text="Press `Tab` to move the focus."),
            HSplit(
                [
                    VSplit(
                        [btn_start, btn_pause, btn_reset, btn_reset_all, btn_exit],
                        padding=1,
                        style="bg:#cccccc",
                    ),
                    text_window,
                ]
            ),
        ]
    )
)

layout = Layout(container=root_container, focused_element=btn_start)

# Key bindings.
kb = KeyBindings()
kb.add("tab")(focus_next)
kb.add("s-tab")(focus_previous)
kb.add("right")(focus_next)
kb.add("left")(focus_previous)
kb.add("q")(exit_clicked)

# Styling.
style = Style(
    [
        ("left-pane", "bg:#888800 #000000"),
        ("right-pane", "bg:#00aa00 #000000"),
        ("button", "#000000"),
        ("button-arrow", "#000000"),
        ("button focused", "bg:#ff0000"),
        ("red", "#ff0000"),
        ("green", "#00ff00"),
    ]
)

# Build a main application object.
application = Application(layout=layout, key_bindings=kb, style=style, full_screen=True)


def draw():
    tomato.update()
    text_area.text = tomato.as_formatted_text()
    application.invalidate()


def main():
    draw()
    threading.Thread(target=lambda: every(0.4, draw), daemon=True).start()
    application.run()


if __name__ == "__main__":
    main()

pydoro.pydoro_core.util.py

import os
import time


def every(delay, task):
    next_time = time.time() + delay
    while True:
        time.sleep(max(0, next_time - time.time()))
        task()
        next_time += (time.time() - next_time) // delay * delay + delay


def in_app_path(path):
    import sys

    try:
        wd = sys._MEIPASS
        return os.path.abspath(os.path.join(wd, path))
    except AttributeError:
        return _from_resource(path)


def _from_resource(path):
    from pkg_resources import resource_filename

    res_path = resource_filename(__name__, path)
    if not os.path.exists(res_path):
        res_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), path)
    return res_path
\$\endgroup\$
  • \$\begingroup\$ There is a sound.py file in GitHub for handling cross-platform audio but since I didn't write it fully, I've decided not to add it here. But anything even on the GitHub is fair game. Also I'm not a beginner feel free to call out any sloppiness. \$\endgroup\$ – bhathiya-perera Jun 30 at 16:34
3
+50
\$\begingroup\$

Comments are good.

  • Provide a module level doc string describe what the program does or provide a link to wikipedia
  • What are the fields in the tuples in TOMATO (I think RGB color and a display string)
  • Why are some fields in TOMATO empty strings or just spaces?
  • maybe define RED = "#ff0000" and GREEN = "#00cc00" and use RED or GREEN in TOMATO
  • same for LOCATIONS

IntEnum is intended for when you need to be able to use the enums as integers. Your code doesn't use that feature. So consider just using Enum.

Enum

Enum members are hashable and can be used as dictionary keys like so:

TEXT = {
    TaskStatus.NONE: "",
    TaskStatus.STARTED: "",
    ...etc...
}

@property

Although @property is designed so that it can be used to make "read-only" attributes in Python, I don't think that is considered to be Pythonic. It is more Pythonic to just access attributes directly. Properties are more often used when you need to do some calculations when setting or getting the value of an attribute. And you don't need to create properties unless and until you need them.

State machine

Clever use of itertools.cycle() for the progress indicator. But does each state instance need its own?

InitialState.time_remaining() returns "Press [Start]", which doesn't seem like a remaining amount of time.

IntermediateState.start() calls self._next_factory() which is set to None in __init__(). Perhaps it would be good to check that _next_factory was initialized. This is in several places.

I don't see the benefit of the transition_to() method. I would just use a next_state parameter in __init__(). Similarly for return_to().

I found the state transitions hard to follow, so you might want to document that somewhere for future reference.

If you don't want to roll your own, transitions is a nice state machine library.

\$\endgroup\$
  • \$\begingroup\$ I like your answer lot of useful content. Do you think I should perhaps break the tomato module to multiple files? Is tomato a good name for a module? Do you have a better idea? \$\endgroup\$ – bhathiya-perera Jul 9 at 9:18
  • \$\begingroup\$ The code in the tomato module seems related, I don't see anything to split into another module. Maybe the the UI stuff in there could be split out so you use alternate UIs if you continue to develop this. Tomato is a fine name--it means something to you and to someone familiar with the Tomato Timer method. \$\endgroup\$ – RootTwo Jul 9 at 18:50
2
\$\begingroup\$

Thanks for sharing this. There is a lot here, and in general it looks good. Without a LOT more thought I would not be able to provide constructive feedback to the logic here. So I will punt, and fallback to some relatively minor Pythonic (my opinion) comments.

Max is .. really cool

This method does some basic math with timestamps.

def _calc_remainder(self):
    cur = cur_time()
    difference = cur - self._started_at
    remainder = self._remainder - difference
    self._started_at = cur
    if remainder <= 0:
        remainder = 0
    self._remainder = remainder

I would take the 7 lines and reduce to 3 like.

def _calc_remainder(self):
    cur = cur_time()
    self._remainder = max(self._remainder - (cur - self._started_at), 0)
    self._started_at = cur

The big differences are using max() to clip to 0 and removing some of the intermediate calcs.

Be explicit about what you are calculating:

This:

def next_state(self):
    self._tomato.tomatoes += 1
    if self._tomato.tomatoes % TOMATOES_PER_SET == 0:
        return IntermediateState.transition_to(LongBreakState, tomato=self._tomato)
    return IntermediateState.transition_to(SmallBreakState, tomato=self._tomato)

Can be reduced a bit to:

def next_state(self):
    self._tomato.tomatoes += 1
    next_state = LongBreakState if self._tomato.tomatoes % TOMATOES_PER_SET == 0 else SmallBreakState
    return IntermediateState.transition_to(next_state, tomato=self._tomato)

The primary change I would recommend here is the explict calculation of next_state. This construct makes next_state explicit, while the previous requires the reader to figure this out.

Python does not require () around tuples

So this:

ftext = TOMATO[:]
for i in range(1, 5):
    ftext[LOCATIONS["task" + str(i)]] = ("", task[i - 1])
ftext[LOCATIONS["status"]] = ("", status)
....    

Can be reduced to:

ftext = TOMATO[:]
for i in range(1, 5):
    ftext[LOCATIONS["task" + str(i)]] = "", task[i - 1]
ftext[LOCATIONS["status"]] = "", status
....
\$\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.