15
\$\begingroup\$

enter image description here Telling the time based on the "set theory principle", the Mengenlehreuhr consists of 24 lights which are divided into one circular blinking yellow light on top to denote the seconds, two top rows denoting the hours and two bottom rows denoting the minutes.

As a morning exercise I have been writing an implementation of Mengenlehreuhr in Python using curses.

The source of this is as follows:

import curses
from datetime import datetime
from math import floor

class Mengenlehreuhr():
    screen = None
    panel_name = [
        'seconds', 'five_hours', 'hours', 'five_minutes', 'minutes'
    ]

    panes  = {
        'seconds': [None],
        'five_hours': [None, None, None, None],
        'hours': [None, None, None, None],
        'five_minutes': [
            None, None, None, None,
            None, None, None, None,
            None, None, None
        ],
        'minutes': [None, None, None, None]
    }

    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):
        curses.nocbreak()
        self.screen.keypad(0)
        curses.echo()
        curses.endwin()

    def create_clock_face(self):
        offset = 0
        screen_h, screen_w = self.screen.getmaxyx()
        screen_w = int(floor(screen_w / 2))
        for i in range(5):
            j = 0
            k = self.panel_name[i]
            width = 11
            for p in range(len(self.panes[k])):
                if k == 'seconds':
                    x = screen_w - int(floor(width / 2))
                    self.panes[k][p] = self.create_window(x, offset)
                elif k == 'minutes' or k == 'hours' or k == 'five_hours':
                    x = screen_w - int(floor((len(self.panes[k]) * width) / 2))
                    i = (j * 11)
                    self.panes[k][p] = self.create_window(x + i, offset, width=10)
                elif k == 'five_minutes':
                    width = 4
                    x = screen_w - int(floor((len(self.panes[k]) * width) / 2))
                    i = 0 + (j * width)
                    self.panes[k][p] = self.create_window(x + i, offset, width=3)
                j += 1
            offset = offset + 5

    def create_window(self, x, y, height = 4, width = 11):
        win = curses.newwin(height, width, y, x)
        win.box()
        win.refresh()
        return win

    def update(self, time):
        for i in range(5):
            k = self.panel_name[i]
            color_pair = curses.color_pair(1)
            for p in range(len(self.panes[k])):
                if k == 'seconds':
                    color_pair = curses.color_pair(2) if time.second % 2 == 0 else curses.color_pair(3)
                elif k == 'minutes':
                    color_pair = curses.color_pair(3) if p < time.minute % 5 else curses.color_pair(1)
                elif k == 'hours':
                    color_pair = curses.color_pair(2) if p < time.hour % 5 else curses.color_pair(1)
                elif k == 'five_hours':
                    hours = int(floor(time.hour / 5))
                    color_pair = curses.color_pair(2) if p < hours else curses.color_pair(1)
                elif k == 'five_minutes':
                    minutes = int(floor(time.minute / 5))
                    if p in (2, 5, 8) and p < minutes:
                        color_pair = curses.color_pair(2)
                    else:
                        color_pair = curses.color_pair(3) if p < minutes else curses.color_pair(1)
                self.panes[k][p].bkgd(' ', color_pair)
                self.panes[k][p].refresh()

    def run(self):
        self.create_clock_face()
        while True:
            try:
                self.update(datetime.now())
            except KeyboardInterrupt:
                self.close()
                exit(0)

clock = Mengenlehreuhr()
clock.run()

In particular I'd be interested in seeing a better solution for the following part of the update statement which I am sure can be improved:

if p in (2, 5, 8) and p < minutes:
    color_pair = curses.color_pair(2)
else:
    color_pair = curses.color_pair(3) if p < minutes else curses.color_pair(1)

Personally I'd prefer not to have the p in (2, 5, 8) if possible but write this a different way which doesn't involve "magic numbers".

Secondly, is there a way of referencing the default terminal colours rather than having curses.init_pair(1, curses.COLOR_WHITE, curses.COLOR_BLACK) as the "off" lamp?

Finally, there seems to be an inefficiency in the code as the seconds lamp occasionally gets stuck. What can be done to overcome this and make it more accurate?

\$\endgroup\$
  • 4
    \$\begingroup\$ I'd never heard of a Mengenlehreuhr before, now I have an additional task on my to-do list :-) \$\endgroup\$ – Mast Aug 15 '15 at 9:48
  • \$\begingroup\$ I took the liberty to add a picture to your question - I hope you don't mind. \$\endgroup\$ – jacwah Aug 15 '15 at 10:39
  • \$\begingroup\$ I don't mind at all - I guess it adds a bit of context for those who have never seen or heard of this particular clock before :-) \$\endgroup\$ – mplf Aug 15 '15 at 10:43
12
\$\begingroup\$

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 methods you intend to expose in 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 of panes (a quick fix would be to use an OrderedDict)
  • 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 in create_clock_face, and again as the default parameter value in create_window(). j * 11 is because most windows have a width of 10 and a one-column margin. The five_minutes case has width = 4 because the window width is 3, plus a one-column margin. offset = offset + 5 is because the window height is 4, according to the create_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 as n // 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()
\$\endgroup\$
  • \$\begingroup\$ As I said in my comment to Jamie, there is a lot in Python I think I need to learn ;-). I'm guessing the CPU usage might have been the main reason I occasionally saw the seconds panel hang. You're alternative is definitely far cleaner than my original although I still don't like the i in (2, 5, 8) - surely there is a cleaner way of picking out the red-lamps here (or maybe this is me being picky and seeing a problem that doesn't exist). From a mornings exercise to a more elegant solution, I'm happy, I like it. \$\endgroup\$ – mplf Aug 16 '15 at 9:18
  • \$\begingroup\$ You could write (i + 1) % 3 == 0 instead. \$\endgroup\$ – 200_success Aug 16 '15 at 9:21
11
\$\begingroup\$

Some style nitpicks...

  • In your initialization you could replace things like:

    'five_minutes': [
        None, None, None, None,
        None, None, None, None,
        None, None, None
    ],
    

    with the arguably much more readable:

    'five_minutes': [None] * 11,
    
  • You are very often repeating the pattern int(floor(a / b)), where both a and b seem to be guaranteed to always be integers. Python has an integer division operator, // which does exactly that: rounded down integer division. So you can replace all those long lines by 'a // b'.

  • Not affecting your code, but along the same lines, if you ever see your self writing int(ceil(a / b)), consider the alternative (a - 1) // b + 1

  • Another pattern you repeat a lot is:

    curses.color_pair(3) if p < minutes else curses.color_pair(1)
    

    which I think is more readable as:

    curses.color_pair(3 if p < minutes else 1)
    
  • Instead of having a list of dictionary labels, you may consider using the enum module, and have just a list:

    class Panes(IntEnum):
        seconds = 0
        five_hours = 1
        hours = 2
        five_minuts = 3
        minutes = 4
    
    panes = [[None], [None] * 4, [None] * 4, [None] * 11, [None] * 4]
    

    and then access the panes list as:

    panes[Panes.five_minutes]
    

    instead of:

    panes[panes[panel_name[3]]]
    
\$\endgroup\$
  • \$\begingroup\$ Hey if style is the only thing to come back after 24 hours, I'll take it :-). I wasn't aware of the integer divisor (python isn't my first language so I'm still finding things out with it) and those lists in initialisation are ugly I agree. I added an array of dictionary keys to overcome a sorting problem on the dictionary (I originally had panels appearing out of order which was doing no good for telling the time). Changes definitely taken under advisement. \$\endgroup\$ – mplf Aug 16 '15 at 8:40

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.