close()
and run()
are probably the only methodmethods you intend to expose onin Mengenlehreuhr
. The other methods are internal interfaces, so you should name them with a leading underscore by convention.
close()
and run()
are probably the only method you intend to expose on Mengenlehreuhr
. The other methods are internal interfaces, so you should name them with a leading underscore by convention.
close()
and run()
are probably the only methods you intend to expose in Mengenlehreuhr
. The other methods are internal interfaces, so you should name them with a leading underscore by convention.
- the redundancy between
panel_name
and the keys ofpanes
(a quick fix would be to use anOrderedDict
) - the use of string keys at all
- the arrays of
None
to be replaced by curses windows later (I prefer to see them defined all at once, ideally using a list comprehension) - Thethe if-elif chains
- Hardhard-coded magic numbers and dimensions.
range(5)
is because there are five panel types.width = 11
is written once increate_clock_face
, and again as the default parameter value increate_window()
.j * 11
is because most windows have a width of 10 and a one-column margin. Thefive_minutes
case haswidth = 4
because the window width is 3, plus a one-column margin.offset = offset + 5
is because the window height is 4, according to thecreate_window()
function. "Width" sometimes refers to the window width, and sometimes refers to the width and the margin. offset
should be named to make it obvious that it is a y-coordinate.int(floor(n / 2))
would be better written asn // 2
.- redefining variables makes the code harder to follow —
screen_w
gets halved;width
gets overridden.
- the redundancy between
panel_name
and the keys ofpanes
(a quick fix would be to use anOrderedDict
) - the use of string keys at all
- the arrays of
None
to be replaced by curses windows later (I prefer to see them defined all at once, ideally using a list comprehension) - The if-elif chains
- Hard-coded magic numbers and dimensions.
range(5)
is because there are five panel types.width = 11
is written once increate_clock_face
, and again as the default parameter value increate_window()
.j * 11
is because most windows have a width of 10 and a one-column margin. Thefive_minutes
case haswidth = 4
because the window width is 3, plus a one-column margin.offset = offset + 5
is because the window height is 4, according to thecreate_window()
function. "Width" sometimes refers to the window width, and sometimes refers to the width and the margin. offset
should be named to make it obvious that it is a y-coordinate.int(floor(n / 2))
would be better written asn // 2
.- redefining variables makes the code harder to follow —
screen_w
gets halved;width
gets overridden.
- the redundancy between
panel_name
and the keys ofpanes
(a quick fix would be to use anOrderedDict
) - the use of string keys at all
- the arrays of
None
to be replaced by curses windows later (I prefer to see them defined all at once, ideally using a list comprehension) - the if-elif chains
- hard-coded magic numbers and dimensions.
range(5)
is because there are five panel types.width = 11
is written once increate_clock_face
, and again as the default parameter value increate_window()
.j * 11
is because most windows have a width of 10 and a one-column margin. Thefive_minutes
case haswidth = 4
because the window width is 3, plus a one-column margin.offset = offset + 5
is because the window height is 4, according to thecreate_window()
function. "Width" sometimes refers to the window width, and sometimes refers to the width and the margin. offset
should be named to make it obvious that it is a y-coordinate.int(floor(n / 2))
would be better written asn // 2
.- redefining variables makes the code harder to follow —
screen_w
gets halved;width
gets overridden.
Throttling
Your program runs in a tight loop, consuming all the CPU it can get (and battery power, if on a mobile device). For a clock that a user might want to run in the background for a long time, conservation is important. Inserting even just a 0.1 second sleep between updates brings CPU usage from 95% to 1% on my machine.
"Private" methods
close()
and run()
are probably the only method you intend to expose on Mengenlehreuhr
. The other methods are internal interfaces, so you should name them with a leading underscore by convention.
Run loop
You shouldn't need to call exit(0)
if you structure your code properly.
A more appropriate place for cleanup code is in a finally
clause.
Clock face initialization
I refactored this a lot. I didn't like...
- the redundancy between
panel_name
and the keys ofpanes
(a quick fix would be to use anOrderedDict
) - the use of string keys at all
- the arrays of
None
to be replaced by curses windows later (I prefer to see them defined all at once, ideally using a list comprehension) - The if-elif chains
- Hard-coded magic numbers and dimensions.
range(5)
is because there are five panel types.width = 11
is written once increate_clock_face
, and again as the default parameter value increate_window()
.j * 11
is because most windows have a width of 10 and a one-column margin. Thefive_minutes
case haswidth = 4
because the window width is 3, plus a one-column margin.offset = offset + 5
is because the window height is 4, according to thecreate_window()
function. "Width" sometimes refers to the window width, and sometimes refers to the width and the margin. offset
should be named to make it obvious that it is a y-coordinate.int(floor(n / 2))
would be better written asn // 2
.- redefining variables makes the code harder to follow —
screen_w
gets halved;width
gets overridden.
The main improvement, in a nutshell, is to make the code data-directed, so that you declare the five types of panels rather than micromanaging their creation. Then write a simple layout engine to do the dirty work in a generic way.
import curses
from datetime import datetime
from time import sleep
class PanelType():
def __init__(self, n, width, height=4, margin=1, color=lambda i, t: 0):
self.n = n
self.height = height
self.width = width
self.margin = margin
self.color = color
def create_windows(self, y, screen_w):
"""Make a row of n center-justified curses windows."""
screen_mid = screen_w // 2
total_width = self.n * self.width + (self.n - 1) * self.margin
left = screen_mid - total_width // 2
return [
self._create_window(y, left + i * (self.width + self.margin),
self.height, self.width)
for i in range(self.n)
]
@staticmethod
def _create_window(y, x, height, width):
win = curses.newwin(height, width, y, x)
win.box()
win.refresh()
return win
class Mengenlehreuhr():
PANEL_TYPES = [
PanelType( # seconds
n=1, width=10, color=lambda i, t: 3 if t.second % 2 else 2
),
PanelType( # five hours
n=4, width=10, color=lambda i, t: 1 if i >= t.hour // 5 else 2
),
PanelType( # hours
n=4, width=10, color=lambda i, t: 1 if i >= t.hour % 5 else 2
),
PanelType( # five minutes
n=11, width=3, color=lambda i, t: 1 if i >= t.minute // 5 else
2 if i in (2, 5, 8) else 3
),
PanelType( # minutes
n=4, width=10, color=lambda i, t: 1 if i >= t.minute % 5 else 3
),
]
def __init__(self):
self.screen = curses.initscr()
curses.noecho()
curses.cbreak()
self.screen.keypad(1)
curses.start_color()
curses.use_default_colors()
curses.init_pair(1, curses.COLOR_WHITE, curses.COLOR_BLACK)
curses.init_pair(2, curses.COLOR_WHITE, curses.COLOR_RED)
curses.init_pair(3, curses.COLOR_WHITE, curses.COLOR_YELLOW)
curses.curs_set(0)
def close(self):
"""Restore the screen."""
curses.nocbreak()
self.screen.keypad(0)
curses.echo()
curses.endwin()
def _create_clock_face(self):
screen_h, screen_w = self.screen.getmaxyx()
y = 0
self.panels = []
for panel_type in self.PANEL_TYPES:
self.panels.append(panel_type.create_windows(y, screen_w))
y += panel_type.height + panel_type.margin
def _update(self, time):
for panel_type, panel in zip(self.PANEL_TYPES, self.panels):
for i, window in enumerate(panel):
color_pair = curses.color_pair(panel_type.color(i, time))
window.bkgd(' ', color_pair)
window.refresh()
def run(self):
"""Run the clock until a keyboard interrupt."""
self._create_clock_face()
try:
while True:
self._update(datetime.now())
sleep(0.1)
except KeyboardInterrupt:
pass
finally:
self.close()
Mengenlehreuhr().run()