12
\$\begingroup\$

This is my first TUI program implemented via the curses module for Python.

I use the program to input some data (numbers only) via the terminal, which are then saved to a .txt file.

Any kind of feedback is highly appreciated, especially regarding the way I input data (I basically catch every character that the user inputs, if it's a number it is saved to a list and printed on screen, if it's an arrow key I move between the cells) and the way I handle terminal resizing (I delete everything and print it again).

#!/usr/bin/env python3
# last updated on 2019/10/26

import curses

class Twod2small(object):

    def __init__(self, stdwin):

        curses.init_pair(1, 40, 0) # save green
        curses.init_pair(2, 9, 0) # red
        curses.init_pair(3, 255, 0) # white
        self.green = curses.color_pair(1)
        self.red = curses.color_pair(2)
        self.white = curses.color_pair(3)

        self.stdwin = stdwin

        # set the main window's rows and columns
        self.winheight = 29
        self.winwidth = 120

        # two 2d lists containing the input rows and cols for each part of the main window
        self.inputrow = [[5, 9, 13, 16, 18, 21, 23], [5, 9, 12, 14, 17, 19]]
        self.inputcol = [[36, 50], [int(self.winwidth/2) + 36, int(self.winwidth/2) + 50]]

        # measures is the list in which I store all the data inputed
        self.measures = [[["----", "----"] for i in range(7)], [["----", "----"] for i in range(6)]]
        self.kcell = "----"

        # current input side (ciside) is either 0 or 1, respectively for the left and right
        # part of the screen
        self.ciside = 0

        self.cirow = 0
        self.cicol = 0

        self.initScreen()
        self.inputMeasures()

    def initScreen(self):

        # don't show the screen until the terminal has the minimal dimensions
        while True:
            self.stdwin.erase()
            rows, cols = self.stdwin.getmaxyx()
            if rows < 35 or cols < 134:
                msg = "Make terminal at least 134x35"
                if rows > 3 and cols > len(msg):
                    self.stdwin.addstr(int(rows/2) - 1 + rows%2, int((cols - len(msg))/2), msg)
                ch = self.stdwin.getch()
            else:
                break

        self.stdwin.refresh()

        # draw the command window
        self.commandwin = curses.newwin(3, cols, rows - 3, 0)
        msg = "Press 'S' to save, 'Q' to quit."
        self.commandwin.addstr(1, int((cols - len(msg))/2), msg, curses.A_BOLD)
        self.commandwin.chgat(1, int((cols - len(msg))/2) + 7, 1, self.green|curses.A_BOLD)
        self.commandwin.chgat(1, int((cols - len(msg))/2) + 20, 1, self.red|curses.A_BOLD)
        self.commandwin.refresh()

        # set the y and x coordinates for the upper left corner of the measure window
        uly = int((rows - 2 - self.winheight)/2)
        ulx = int((cols - self.winwidth)/2)

        # create the window and enable the keypad
        self.measurewin = curses.newwin(self.winheight, self.winwidth, uly, ulx)
        self.measurewin.keypad(True)
        self.measurewin.border()

        # print the vertical bar separating the two areas of the window
        for i in range(self.winheight - 6):
            self.measurewin.addch(i + 2, int(self.winwidth/2), curses.ACS_VLINE)

        # print the horizontal bar at the bottom of the window
        for i in range(self.winwidth - 1):
            self.measurewin.addch(self.winheight - 3, i, curses.ACS_HLINE)

        # make the corners seamless
        self.measurewin.addch(self.winheight - 3, 0, curses.ACS_LTEE)
        self.measurewin.addch(self.winheight - 3, self.winwidth - 1, curses.ACS_RTEE)

        # print the windows entry points
        self.measurewin.addstr(2, self.inputcol[0][0] - 3, "1", self.white)
        self.measurewin.addstr(2, self.inputcol[0][1] - 3, "2", self.white)
        self.measurewin.addstr(2, self.inputcol[1][0] - 3, "1", self.white)
        self.measurewin.addstr(2, self.inputcol[1][1] - 3, "2", self.white)

        self.measurewin.addstr(5, 5, "A")
        self.measurewin.addstr(9, 5, "B")
        self.measurewin.addstr(13, 5, "C")
        self.measurewin.addstr(17, 5, "D")
        self.measurewin.addstr(16, 20, "D.I")
        self.measurewin.addstr(18, 20, "D.II")
        self.measurewin.addstr(22, 5, "E")
        self.measurewin.addstr(21, 20, "E.I")
        self.measurewin.addstr(23, 20, "E.II")

        self.measurewin.addstr(5, int(self.winwidth/2) + 5, "F")
        self.measurewin.addstr(9, int(self.winwidth/2) + 5, "G")
        self.measurewin.addstr(13, int(self.winwidth/2) + 5, "H")
        self.measurewin.addstr(12, int(self.winwidth/2) + 20, "H.I")
        self.measurewin.addstr(14, int(self.winwidth/2) + 20, "H.II")
        self.measurewin.addstr(18, int(self.winwidth/2) + 5, "J")
        self.measurewin.addstr(17, int(self.winwidth/2) + 20, "J.I")
        self.measurewin.addstr(19, int(self.winwidth/2) + 20, "J.II")

        # print each value of measures at the proper place
        for i, side in enumerate(self.measures):
            for j, row in enumerate(side):
                for k, measure in enumerate(row):
                    self.measurewin.addstr(self.inputrow[i][j], self.inputcol[i][k] - 4,
                                                                 "{} \"".format(measure))

        self.measurewin.addstr(self.winheight - 2, int(self.winwidth/2) - 2, "{} K".format(self.kcell))
        self.measurewin.refresh()

    def inputMeasures(self):

        # if kcell is True I'm in the 11th cell
        kcell = False

        # I only display the cursor when its counter is a multiple of 2
        cursorcntr = 0

        while True:

            i = self.ciside
            j = self.cirow
            k = self.cicol

            if kcell == False:
                row = self.inputrow[i][j]
                col = self.inputcol[i][k]
            else:
                row = self.winheight - 2
                col = int(self.winwidth/2) + 2

            # If the current cell is empty a blank space is added
            if (((kcell == False and self.measures[i][j][k] == "----")
            or (kcell == True and self.kcell == "----")) and cursorcntr%2 == 0):
                self.measurewin.addstr(row, col - 4, "    ")
                chars = []

            # if it isn't, I save the current characters of the entry of the cell
            # in a list called chars
            else:
                if kcell == False:
                    chars = list(self.measures[i][j][k])
                else:
                    chars = list(self.kcell)

            while True:

                # display the cursor only if cursorcntr is even
                if cursorcntr%2 == 0:
                    curses.curs_set(1)
                ch = self.measurewin.getch(row, col)
                curses.curs_set(0)

                # If the user hits the enter key, the cursor counter's value is flipped and
                # I exit the main loop
                if ch == 10:
                    cursorcntr += 1
                    break

                # I also exit the loop if one of the following conditions if verified
                if ((ch == curses.KEY_UP and j > 0)
                or (ch == curses.KEY_DOWN and kcell == False)
                or (ch == curses.KEY_LEFT and (i != 0 or k != 0) and kcell == False)
                or (ch == curses.KEY_RIGHT and (i != 1 or k != 1) and kcell == False)
                or (ch in [ord("s"), ord("S")])
                or (ch in [ord("q"), ord("Q")])):
                    break

                # If the user hits the backspace key and there are characters to be removed,
                # they are removed
                elif ch == 127 and len(chars) > 0 and cursorcntr%2 == 0:
                    chars.pop(len(chars) - 1)
                    self.measurewin.addstr(row, col - len(chars) - 1, " " + "".join(chars))

                # If the user resizes the screen I call the initScreen method and reprint the
                # whole screen
                elif ch == curses.KEY_RESIZE:
                    self.initScreen()
                    self.measurewin.addstr(row, col - 4, " "*4)
                    if len(chars) > 0:
                        self.measurewin.addstr(row, col - len(chars), "".join(chars))

                # if the key entered is none of the above I try to see if it's a number (or the
                # character '.'). If it is, I add it to the chars list and print it on screen
                else:
                    try:
                        if (chr(ch).isdigit() or ch == ord(".")) and len(chars) < 6 and cursorcntr%2 == 0:
                            chars.append(chr(ch))
                            self.measurewin.addstr(row, col - len(chars), "".join(chars))
                    except ValueError:
                        pass

            # At this point I have exited the main loop and I check whether or not chars is empty or
            if len(chars) > 0:
                if kcell == False:
                    self.measures[i][j][k] = "".join(chars)
                else:
                    self.kcell = "".join(chars)
            else:
                if kcell == False:
                    self.measures[i][j][k] = "----"
                else:
                    self.kcell = "----"
                self.measurewin.addstr(row, col - 4, "----")

            # here I check which key has been entered that caused me to exit the loop, and
            # perform actions accordingly for every option
            if ch == curses.KEY_UP:
                if kcell == True:
                    kcell = False
                else:
                    self.cirow -= 1
                # If I pressed an arrow key the value of the cursor counter is always set to a
                # multiple of two (which means that when an arrow key is entered I will always
                # get a blinking cursos in the destination cell)
                cursorcntr *= 2

            elif ch == curses.KEY_DOWN:
                if (i == 0 and j == 6) or (i == 1 and j == 5):
                    kcell = True
                else:
                    self.cirow += 1
                cursorcntr *= 2

            elif ch == curses.KEY_LEFT:
                self.cicol -= 1
                if i == 1 and k == 0:
                    self.ciside -= 1
                    self.cicol += 2
                cursorcntr *= 2

            elif ch == curses.KEY_RIGHT:
                self.cicol += 1
                if i == 0 and k == 1:
                    self.ciside += 1
                    self.cicol -= 2
                    if self.cirow == 6:
                        self.cirow -= 1
                cursorcntr *= 2

            # check If the user wants to save/quit
            elif ch in [ord("s"), ord("S")]:
                self.exit("save")

            elif ch in [ord("q"), ord("Q")]:
                self.exit("quit")

    def exit(self, saveorquit):

        if saveorquit == "save":
            self.save()

        raise SystemExit

    def save(self):
        pass

if __name__ == "__main__":
    curses.wrapper(Twod2small)
\$\endgroup\$
  • 1
    \$\begingroup\$ Please provide an example session so that reviewers can understand the program better. \$\endgroup\$ – L. F. Oct 26 at 13:51
  • 2
    \$\begingroup\$ @L.F. I'm not sure what you mean by 'example session', but what I want the program to do is: let me input some numbers in every cell, move through the cells with the arrow keys, save those numbers to a text file if I press 'S' (I omitted the save method in the code I posted as it isn't relevant to the curser part), exit without saving any number if I press 'Q', and it should handle terminal resizing. Nothing more. \$\endgroup\$ – noibe Oct 26 at 14:07
  • 1
    \$\begingroup\$ By example session, @L.F. probably means show all input and output for an example run of the application. \$\endgroup\$ – Reinderien Oct 26 at 18:01
  • \$\begingroup\$ Yeah, just show the contents of the terminal, including the input and output. Preferably demonstrate all the features. An example is worth 1000 words :) \$\endgroup\$ – L. F. Oct 26 at 23:28
7
+50
\$\begingroup\$

Besides of that I support the idea of having constant values like window's height, width and input_row settings as class constants (uppercase names) and referencing negative kcell flag as not kcell instead of kcell == False here's some list of advises in terms of better code organizing, restructuring conditionals and eliminating duplicates:

  • window's half of width int(self.winwidth / 2) is calculated 13 times across 3 methods.
    Instead, we'll apply Extract variable technique, extracting precalculated expression into the instance field self.win_width_half = int(self.winwidth / 2) and referencing it in all places it's needed
    (like self.inputcol = [[36, 50], [self.win_width_half + 36, self.win_width_half + 50]])

  • setting "windows entry points" in initScreen method (should be renamed to init_screen): involves consecutive 20 calls of self.measurewin.addstr(...) function.
    Instead, we can define entry points attributes beforehand and then pass them in simple iteration:

    win_entry_points_attrs = [
        (2, self.inputcol[0][0] - 3, "1", self.white),
        (2, self.inputcol[0][1] - 3, "2", self.white),
        (2, self.inputcol[1][0] - 3, "1", self.white),
        (2, self.inputcol[1][1] - 3, "2", self.white),
    
        (5, 5, "A"),
        (9, 5, "B"),
        (13, 5, "C"),
        (17, 5, "D"),
        (16, 20, "D.I"),
        (18, 20, "D.II"),
        (22, 5, "E"),
        (21, 20, "E.I"),
        (23, 20, "E.II"),
    
        (5, self.win_width_half + 5, "F"),
        (9, self.win_width_half + 5, "G"),
        (13, self.win_width_half + 5, "H"),
        (12, self.win_width_half + 20, "H.I"),
        (14, self.win_width_half + 20, "H.II"),
        (18, self.win_width_half + 5, "J"),
        (17, self.win_width_half + 20, "J.I"),
        (19, self.win_width_half + 20, "J.II"),
    ]
    # print the windows entry points
    for attrs in win_entry_points_attrs:
        self.measurewin.addstr(*attrs) 
    

Optimizations within inputMeasures method (should be renamed to input_measures):

  • the condition:

    if ((ch == curses.KEY_UP and j > 0)
            or (ch == curses.KEY_DOWN and kcell == False)
            or (ch == curses.KEY_LEFT and (i != 0 or k != 0) and kcell == False)
            or (ch == curses.KEY_RIGHT and (i != 1 or k != 1) and kcell == False)
            or (ch in [ord("s"), ord("S")])
            or (ch in [ord("q"), ord("Q")])):
        break
    

    has a common check kcell == False (should be not kcell) in 3 branches (in the middle) and that's a sign of Consolidate conditional refactoring technique:

    if ((ch == curses.KEY_UP and j > 0)
            or (not kcell and (ch == curses.KEY_DOWN
                                or (ch == curses.KEY_LEFT and (i != 0 or k != 0))
                                or (ch == curses.KEY_RIGHT and (i != 1 or k != 1))))
            or (chr(ch).lower() in ("s", "q"))):
        break
    
  • the variable cursorcntr (cursor counter) deserves for a more meaningful variable name (Rename variable technique) - I would suggest cursor_cnt

  • the last complex if .. elif .. elif conditional of 6 branches at the end of method input_measures seems to be a good candidate for Replace Conditional with Polymorphism technique (OOP approach) but that would require more knowledge and vision about your program conception/settings.
    For now, the first 4 branches of that conditional perform the same action cursor_cnt *= 2 - we can eliminate duplication with additional check.
    It's good to move "save/quit" branches up, as they call self.exit() which will throw raise SystemExit to exit the program.
    Thus the reorganized conditional would look as:

    # set of key codes defined in the parent scope (at least before the loop or as instance variable)
    key_set = set((curses.KEY_UP, curses.KEY_DOWN, curses.KEY_LEFT, curses.KEY_RIGHT))
    ...
    # check If the user wants to save/quit
    inp_char = chr(ch).lower()
    if inp_char == 's':
        self.exit("save")
    elif inp_char == 'q':
        self.exit("quit")
    
    if ch in key_set:
        cursor_cnt *= 2
    
    if ch == curses.KEY_UP:
        if kcell:
            kcell = False
        else:
            self.cirow -= 1
        # If I pressed an arrow key the value of the cursor counter is always set to a
        # multiple of two (which means that when an arrow key is entered I will always
        # get a blinking cursos in the destination cell)
    
    elif ch == curses.KEY_DOWN:
        if (i == 0 and j == 6) or (i == 1 and j == 5):
            kcell = True
        else:
            self.cirow += 1
    
    elif ch == curses.KEY_LEFT:
        self.cicol -= 1
        if i == 1 and k == 0:
            self.ciside -= 1
            self.cicol += 2
    
    elif ch == curses.KEY_RIGHT:
        self.cicol += 1
        if i == 0 and k == 1:
            self.ciside += 1
            self.cicol -= 2
            if self.cirow == 6:
                self.cirow -= 1
    
\$\endgroup\$
8
\$\begingroup\$

I can't run this unfortunately. curses seems to have issues on Windows. I'll just focus mainly on style and design.

There's a few notable things about this chunk:

# I also exit the loop if one of the following conditions if verified
if ((ch == curses.KEY_UP and j > 0)
or (ch == curses.KEY_DOWN and kcell == False)
or (ch == curses.KEY_LEFT and (i != 0 or k != 0) and kcell == False)
or (ch == curses.KEY_RIGHT and (i != 1 or k != 1) and kcell == False)
or (ch in [ord("s"), ord("S")])
or (ch in [ord("q"), ord("Q")])):
    break
  • == False should really just be not instead
  • You need more indentation. It's confusing to see the ors aligned with the ifs. I'd indent it at least all the way up to align with the opening brace.
  • Those two ch in checks at the bottom could be cleaned up.

I'd write this closer to:

if ((ch == curses.KEY_UP and j > 0)
        or (ch == curses.KEY_DOWN and not kcell)
        or (ch == curses.KEY_LEFT and (i != 0 or k != 0) and not kcell)
        or (ch == curses.KEY_RIGHT and (i != 1 or k != 1) and not kcell)
        or (chr(ch).lower() in {"s", "q"})):
    break

I think you could probably factor out the not kcell check too, but my tired brain can't think of a good way at the moment.

There is another approach though that lets you skip all the ors: any. any is like a function version of or (and all is like and). You could change this to:

if any([ch == curses.KEY_UP and j > 0,
        ch == curses.KEY_LEFT and (i != 0 or k != 0) and not kcell,
        ch == curses.KEY_RIGHT and (i != 1 or k != 1) and not kcell,
        chr(ch).lower() in {"s", "q"}]):
    break

Normally, I'd say that this is an abuse of any, but chaining ors over multiple lines like you had isn't ideal either.


In terms of design decisions, does this really need to be a class? Honestly, I'd probably store the necessary state using a dataclass or NamedTuple (or a couple distinct states), then just pass around and alter state(s) as needed. Making self a grab-bag of everything you may need seem messy to me.

For example, winwidth and winheight appear to be constants. They never change throughout your program, so they should treated as constants. I'd have them outside at the top as:

WINDOW_WIDTH = 120
WINDOW_HEIGHT = 29

And do ciside, cicol and cirow need to be attributes of the class as well? It seems like they're only used in inputMeasures, so why aren't they just variables local to that function? By having them "global" within the object, you're forcing your reader to keep them in the back of their mind in case those states are needed elsewhere in the object.


Finally, in terms of naming, you're violating PEP8. Variable and function names should be in snake_case unless you have a good reason, and class names should be UpperCamelCase.

\$\endgroup\$
  • \$\begingroup\$ @D.BenKnoble Corrected. Thank you. \$\endgroup\$ – Carcigenicate Oct 27 at 15:58
  • \$\begingroup\$ Virtualbox runs on Windows. It is very easy to run a Linux virtual machine and use the program. \$\endgroup\$ – Gribouillis Oct 31 at 16:28

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.