4
\$\begingroup\$

I've written a simple calculator in Python. I want to show not much effect of the action but the logic behind it, by that I mean console menu implementation in the Menu class. I'm curious about what I can improve in the Menu class, what do you think about it and other helper functions? I'd like to hear also what useful functionality can I add to the Menu library.

Here's code with an example menu implementation in the calculator:

# -*- coding: utf-8 -*- import enum from collections import OrderedDict # I used OrderedDict due to the move_to_end function from dataclasses import dataclass from typing import Any @enum.unique class BannerStyle(enum.IntEnum): FRAME_BOX = enum.auto() HASH = enum.auto() def print_banner(title, style=BannerStyle.FRAME_BOX): match style: case BannerStyle.FRAME_BOX: print('-' * (len(title) + 4)) print(f'| {title:^} |') print('-' * (len(title) + 4)) case BannerStyle.HASH: print('#' * (len(title) + 4)) print(f'# {title:^} #') print('#' * (len(title) + 4)) def read_int(prompt='Enter an integer: ', errmsg='Not an integer.', default=None): while True: try: user_input = input(prompt) if default and not user_input: user_input = default return int(user_input) except ValueError: print(errmsg) def read_float(prompt='Enter an floating point number: ', errmsg='Not an floating point number.', default=None): while True: try: user_input = input(prompt) if default and not user_input: user_input = default return float(user_input) except ValueError: print(errmsg) def print_list(lst): for idx, item in enumerate(lst): print(f'{idx}) {item}') @dataclass class UserChoice: idx: int choice: Any def read_choice(choices, prompt='Your choice (enter an integer): ', errmsg='Invalid choice.', default=None): if default and default not in choices: raise ValueError(f'default value {default} is not present in choices argument') while True: try: print('Available choices:') print_list(choices) user_input = input(prompt) if default and not user_input: user_input = default user_input = int(user_input) if not 0 <= user_input < len(choices): raise ValueError return UserChoice(idx=user_input, choice=choices[user_input]) except (ValueError, IndexError): print(errmsg) class Menu: def __init__(self, title, banner_style=BannerStyle.FRAME_BOX): self.title = title self.active = True self.items = OrderedDict({'Quit': self.quit}) self.banner_style = banner_style def add_item(self, title, func): self.items[title] = func self.items.move_to_end('Quit') def remove_item(self, title): del self.items[title] def add_submenu(self, title, submenu): self.items[title] = submenu.quit @property def _items_titles(self): return list(self.items.keys()) def invoke(self, title): return self.items[title]() def quit(self): self.active = False def loop(self): while self.active: print_banner(self.title) user_input = read_choice(self._items_titles).idx self.invoke(self._items_titles[user_input]) def __repr__(self): return f"MenuItem(title='{self.title}', active={self.active})" def __str__(self): return self.title class CalculatorUI: def add(self): self._read_numbers() print(f'The result is {self.a + self.b}') def subtract(self): self._read_numbers() print(f'The result is {self.a - self.b}') def multiply(self): self._read_numbers() print(f'The result is {self.a * self.b}') def divide(self): self._read_numbers_div() print(f'The result is {self.a / self.b}') def modulo(self): self._read_numbers_div() print(f'The result is {self.a % self.b}') def _read_numbers(self): self.a = read_float('Enter first number: ') self.b = read_float('Enter second number: ') def _read_numbers_div(self): self.a = read_float('Enter first number: ') self.b = read_float('Enter second number: ') while self.b == 0: print('Number cannot be zero.') self.b = read_float('Enter second number: ') calc = CalculatorUI() def test_menu(): menu = Menu(title='Welcome to calculator') menu.add_item('Add', calc.add) menu.add_item('Subtract', calc.subtract) menu.add_item('Multiply', calc.multiply) menu.add_item('Divide', calc.divide) menu.add_item('Modulo', calc.modulo) menu.loop() if __name__ == '__main__': test_menu() 
\$\endgroup\$

    1 Answer 1

    4
    \$\begingroup\$

    In f'| {title:^} |', there's no padding, so does the centre-specifier have any effect? I don't think it does.

    read_int and read_float should be collapsed to one function that accepts a type and a name. Also, the default should not be a string; it should be of the numeral type.

    Your functions are missing PEP484 type hints. It's not enough to hint your class members.

    CalculatorUI is not a well-modelled class. a and b should not be properties, and in fact there's no reason for this to be a class.

    Why does Menu have a remove_item? It's never called, and it probably never should be called. The menu should be an immutable sequence of items. So Menu is also not a well-modelled class either. active should not be a property, and not even a variable of any kind: you should just be able to break out of the loop once it's appropriate.

    It's not a good idea for a choice-from-sequence to return a title string. The choice should instead return a callable or object that can be used directly, without something like invoke needing to exist.

    Make a temporary variable to store your banner character.

    if default and not user_input: is dangerous. What happens on 0? 0 is falsey.

    idx in UserChoice is redundant. One hint as to why this is: you don't use it in print_items.

    Don't overwrite a variable of one type with a value of a second type as you do in user_input = int(user_input).

    For immutable data, NamedTuple is a better choice than @dataclass.

    You capture a "banner style" in your menu constructor but then fail to pass it to the banner rendering method.

    Default-supporting methods could look like

    def read_number(name: str, type_: Type[EntryNumber], default: Optional[EntryNumber] = None) -> EntryNumber: if default is None: prompt = f'Enter {name}: ' else: prompt = f'Enter {name} [{default}]: ' while True: try: user_input = input(prompt) if user_input == '': if default is not None: return default continue return type_(user_input) except ValueError: print(f'Invalid {name}.') def read_choice( choices: Sequence[UserChoice], default: Optional[UserChoice] = None, ) -> UserChoice: if default is not None and default not in choices: raise ValueError(f'default value {default} is not present in choices argument') prompt = '\n'.join(format_items(choices)) while True: try: print(prompt) print('Your choice (enter an integer): ') user_input = input(prompt) if user_input == '': if default is not None: return default continue index = int(user_input) if 0 <= index < len(choices): return choices[index] print('Choice out of range.') except ValueError: print('Invalid integer.') 

    but you never use the default functionality, so delete it.

    It is unusual to have a 0-based user-facing menu selection. Strongly consider using 1-based instead.

    Suggested

    import enum from numbers import Number from typing import Callable, Iterable, NamedTuple, Optional, Sequence, Type, TypeVar @enum.unique class BannerStyle(enum.IntEnum): FRAME_BOX = enum.auto() HASH = enum.auto() EntryNumber = TypeVar('EntryNumber', bound=Number) def read_number(prompt: str, type_: Type[EntryNumber]) -> EntryNumber: while True: try: return type_(input(prompt)) except ValueError: print(f'Invalid number.') class UserChoice(NamedTuple): name: str invoke: Callable[[], Optional[bool]] def __str__(self) -> str: return self.name def format_items(choices: Iterable[UserChoice]) -> Iterable[str]: for idx, item in enumerate(choices, 1): yield f'{idx}) {item}' def read_choice(choices: Sequence[UserChoice]) -> UserChoice: prompt = '\n'.join(( *format_items(choices), 'Your choice (enter an integer): ' )) while True: index = read_number(prompt, int) - 1 if 0 <= index < len(choices): return choices[index] print('Choice out of range.') class Menu(NamedTuple): title: str items: Sequence[UserChoice] banner_style: BannerStyle = BannerStyle.FRAME_BOX def print_banner(self) -> None: match self.banner_style: case BannerStyle.FRAME_BOX: top_char = '-' edge_char = '|' case BannerStyle.HASH: top_char = '#' edge_char = '#' case _: raise ValueError(f'Invalid style {self.banner_style}') top = top_char * (len(self.title) + 4) print(top) print(f'{edge_char} {self.title} {edge_char}') print(top) def loop(self) -> None: while True: self.print_banner() choice = read_choice(self.items) if choice.invoke(): break print() def read_numbers() -> tuple[float, float]: return ( read_number('Enter first number: ', float), read_number('Enter second number: ', float), ) def read_numbers_div() -> tuple[float, float]: a = read_number('Enter first number: ', float) while True: b = read_number('Enter second number (cannot be 0): ', float) if b != 0: return a, b def add() -> None: a, b = read_numbers() print(f'The result is {a + b}') def subtract() -> None: a, b = read_numbers() print(f'The result is {a - b}') def multiply() -> None: a, b = read_numbers() print(f'The result is {a * b}') def divide() -> None: a, b = read_numbers_div() print(f'The result is {a / b}') def modulo() -> None: a, b = read_numbers_div() print(f'The result is {a % b}') def quit_menu() -> bool: return True def test_menu() -> None: menu = Menu( title='Welcome to calculator', items=( UserChoice('Add', add), UserChoice('Subtract', subtract), UserChoice('Multiply', multiply), UserChoice('Divide', divide), UserChoice('Modulo', modulo), UserChoice('Quit', quit_menu), ), ) menu.loop() if __name__ == '__main__': test_menu() 
    \$\endgroup\$
    4
    • \$\begingroup\$Is # -*- coding: utf-8 -*- necessary in the new Python programs?\$\endgroup\$CommentedJan 15, 2023 at 11:27
    • \$\begingroup\$stackoverflow.com/q/14083111/313768\$\endgroup\$CommentedJan 15, 2023 at 13:15
    • \$\begingroup\$Should I use type hints in Django too?\$\endgroup\$CommentedJan 15, 2023 at 20:24
    • \$\begingroup\$yes, definitely. Django is just Python.\$\endgroup\$CommentedJan 15, 2023 at 21:18

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.