5
\$\begingroup\$

I started writing a small CLI menu library in python. I'm uncertain of the structure and scalability of the current code. For example the Menu.choice method would grow very large if were to add more options for navigation input. Could this be better done with a class to handle the input?

import sys
import os


class Menu(object):
    def __init__(self, *items, **kwargs):
        self.items = []
        self.title = kwargs.get('title', '')
        self.header = kwargs.get('header', '')
        for item in items:
            self.add(item)

    def add(self, *items):
        for item in items:
            self.items.append(item)
            if hasattr(item, 'subMenu'):
                self.addParentToChild(item.subMenu)

    def show(self):
        clear()
        if len(self.header):
            self.showHeader()
        if len(self.title):
            self.showTitle()
        for i, item in enumerate(self.items):
            print('[{}] {}'.format(i + 1, item.title))
        self.choice()

    def choice(self):
        while 1:
            try:
                choice = input('> ')
                if choice is 'q':
                    clear()
                    sys.exit()
                if choice is 'b':
                    if hasattr(self, 'parent'):
                        self.parent.show()
                    else:
                        self.show()
                        continue
                if not choice.isdigit():
                    raise ValueError
                item = self.items[int(choice) - 1]
                if hasattr(item, 'callback'):
                    item.callback()
                if hasattr(item, 'subMenu'):
                    item.subMenu.show()
                if not hasattr(item, 'subMenu') and not hasattr(item, 'callback'):
                    print('{} is an empty menu...'.format(item.title))
            except (ValueError, IndexError) as e:
                self.show()
                continue

    def addParentToChild(self, child):
        child.parent = self

    def showTitle(self):
        print(self.title)

    def showHeader(self):
        print(self.header)


class MenuItem(object):
    def __init__(self, title, subMenu=None, callback=None, tag=''):
        self.title = title
        if subMenu is not None:
            self.subMenu = subMenu
            if len(self.title):
                subMenu.title = self.title
        if callback is not None:
            self.callback = callback
        if tag:
            self.tag = tag


def clear():
    if os.name == 'nt':
        os.system('cls')
    else:
        os.system('clear')

Small example:

if __name__ == '__main__':
    item1 = MenuItem('empty test 1')
    item2 = MenuItem('empty test 2')
    subMenu1 = Menu(item1, item2)
    item3 = MenuItem('sub menu', subMenu=subMenu1)
    testMenu = Menu(item1, item2, item3, title='Main Menu')
    testMenu.show()

Do you have any advice or suggestions about the current structure or anything else that may cause issues later on?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  • Docstrings: You should include a docstring at the beginning of every method/class/module you write. This will help any documentation identify what your code is supposed to do.
  • Naming: Python variables and parameters should be in snake_case, not camelCase.
  • len as condition: You shouldn't use if len(self.title):, instead you should check if it doesn't exist, if not self.title, as it is more compliant with PEP-8. This StackOverflow answer provides more insight.
  • String formatting: This one is a personal preference. I like to use f"" to format my strings because it allows me to directly include variables into the string, without having to chain a .format() onto the end. Again, this is purely personal, but see how you like it.
  • Imports: Another personal preference, I like to organize my imports alphabetically so it's more organized. This is all up to you if you want to keep with this practice, but do make sure to follow PEP-8 Import Guide to make sure you're following standard and accepted practies.
  • Unnecessary methods: show_title and show_header is basically extra and unneeded code. You're essentially redefining the print method. Just print the title/header whenever you need to, instead of calling these methods.

Updated Code

import os
import sys

class Menu:
    """ Menu class for displaying and adding to menu """
    def __init__(self, *items, **kwargs):
        self.items = []
        self.title = kwargs.get('title', '')
        self.header = kwargs.get('header', '')
        for item in items:
            self.add(item)

    def add(self, *items):
        """ Adds each element in `list` to items """
        for item in items:
            self.items.append(item)
            if hasattr(item, 'sub_menu'):
                self.add_parent_to_child(item.sub_menu)

    def show(self):
        """ Displays the items and calls method to collect user input """
        clear()
        if self.header:
            print(self.header)
        if self.title:
            print(self.title)
        for i, item in enumerate(self.items):
            print(f"[{i + 1}] {item.title}")
        self.choice()

    def choice(self):
        """ Collects user input """
        while 1:
            try:
                choice = input('> ')
                if choice == 'q':
                    clear()
                    sys.exit()
                if choice == 'b':
                    if hasattr(self, 'parent'):
                        self.show()
                    else:
                        self.show()
                        continue
                if not choice.isdigit():
                    raise ValueError
                item = self.items[int(choice) - 1]
                if hasattr(item, 'callback'):
                    item.callback()
                if hasattr(item, 'sub_menu'):
                    item.sub_menu.show()
                if not hasattr(item, 'sub_menu') and not hasattr(item, 'callback'):
                    print(f"{item.title} is an empty menu...")
            except (ValueError, IndexError):
                self.show()
                continue

    def add_parent_to_child(self, child):
        """ Adds the passed `child` to the parent of the object """
        child.parent = self


class MenuItem:
    """ Specific class for items in the menu """

    def __init__(self, title, sub_menu=None, callback=None, tag=''):
        self.title = title
        if sub_menu is not None:
            self.sub_menu = sub_menu
            if self.title:
                sub_menu.title = self.title
        if callback is not None:
            self.callback = callback
        if tag:
            self.tag = tag

def clear():
    """ Clears the console screen """
    os.system('cls') if os.name == 'nt' else os.system('clear')

if __name__ == '__main__':
    ITEM_1 = MenuItem('empty test 1')
    ITEM_2 = MenuItem('empty test 2')
    SUBMENU_1 = Menu(ITEM_1, ITEM_2)
    ITEM_3 = MenuItem('sub menu', sub_menu=SUBMENU_1)
    TEST_MENU = Menu(ITEM_1, ITEM_2, ITEM_3, title='Main Menu')
    TEST_MENU.show()
\$\endgroup\$
2
  • \$\begingroup\$ A couple of points: 1. You don't need () in class definition, class Menu will do just fine. 2. I don't think this: if not choice.isdigit(): raise ValueError has any effect at all. But good suggestions \$\endgroup\$ Commented Dec 26, 2020 at 14:22
  • \$\begingroup\$ @GrajdeanuAlex Yeah I should probably update this answer :P \$\endgroup\$
    – Linny
    Commented Dec 26, 2020 at 22:37

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.