Skip to main content
mention the termcolor package
Source Link
J_H
  • 35.2k
  • 3
  • 32
  • 129

ANSI colors are nice, but they're not always appropriate. Consider depending on the termcolor package. Then a user could export TERM=dumb to suppress those escape codes.


ANSI colors are nice, but they're not always appropriate. Consider depending on the termcolor package. Then a user could export TERM=dumb to suppress those escape codes.

added 331 characters in body
Source Link
J_H
  • 35.2k
  • 3
  • 32
  • 129

Typically a @staticmethod will be a _private() helper for some public method that accepts a self parameter.

A class full of only static methods is a class that should not exist. It maintains no state variables, and defends no class invariant. Prefer to put such static methods at top-level of some suitable module.

Typically a @staticmethod will be a _private() helper for some public method that accepts a self parameter.

A class full of only static methods is a class that should not exist. It maintains no state variables, and defends no class invariant. Prefer to put such static methods at top-level of some suitable module.

Source Link
J_H
  • 35.2k
  • 3
  • 32
  • 129

modules, not classes

I like that you're organizing things. But this is not Java.


class Styling:
    GREEN = '\033[92m'
    YELLOW = '\033[33m'
    RED = '\033[91m'
    ENDC = '\033[0m'
    BOLD = '\033[1m'
    ENDBOLD = '\033[0m'

class CLIHandler:
    @staticmethod
    def display_message( ... ): ...
    
    @staticmethod
    def display_topics( ... ): ...

Use modules to organize such things. Use a class for something that holds state, over which you're trying maintain a class invariant.

So the color constants might be at top-level within a styling.py module, which you then import as needed. Similarly for the display functions, in a cli_handler.py module.

nested printing

The app code doesn't necessarily have to know about those color constants at all. Consider creating a def green(msg: str) -> str: helper, and similarly a bold(msg) helper. Then the helper can ensure ENDC and ENDBOLD always appear in the proper place. Furthermore the nesting works out nicely when we compose them, e.g. print("Hello", bold(red("world")), "!")

CLS

There's no need to fork off a child process here.

    @staticmethod
    def clear_screen():
        os.system('clear')

The ANSI escape codes let you print CSI 2 J to accomplish that. Just define another string constant.

name mangling

I don't understand what's going on here.

    def __correct_answers_count(self):
        ...

    def __load_random_question(self):
        ...

Why are you requesting name mangling for inherited classes? I don't see any use of inheritance, and there's no design notes in the comments explaining the requirement. Mangling is usually not what you want. Recommend you just remove it. Prefer single _ underscore for a _private name, e.g. _correct_answers_count.

extra class

The Topic and Question classes appear to be well motivated. Each would benefit from the addition of a """docstring""".

This class seems less well motivated:

class Answer:
    def __init__(self, value, correct):
        self.value = value
        self.correct = correct
        self.selected = False

    def select(self):
        self.selected = True

Consider using a @dataclass decorator to save a little bit of constructor boilerplate.

The selected attribute seems odd. In many setups I would expect to see a selected_answer_index which lives outside of this class and satisfies the need.

But suppose we do really need the attribute. Don't write a def select() setter routine. We're all grownups here, and the attribute is public. Calling code should just reach in and set the flag. Python packages worry about SemVer and breaking changes, but they have a quite different set of concerns around that compared with java / maven binary compatibility concerns.

When you're tempted to write a getter or setter routine, think twice and resist the urge to write one. The caller should probably just be accessing the public attribute. Linters will flag any caller attempts to access _private attributes.

There are use cases where it makes sense, but they are relatively uncommon. We might want to maintain / enforce a class invariant such as "mass shall be non-negative", or log that a value was updated. And often we'll use a @property decorator when defining getters / setters.