3
\$\begingroup\$

I posted part of this module earlier (here). I implemented that feedback and now am looking for another round of gut checking. Questions/Concerns I have:

  • Does the way I've structured my functions make sense? Can they be broken down more/differently?
  • Is there a better way of doing the error handling? I feel like I'm repeating a lot of code, especially in GetNumber, GetNumberInRange, and _AcceptAndValidateNumber
  • Anything else that will help me improve my coding skills

Also, I'm not sure if I qualify as a beginner anymore, but I tagged it as such because I feel I still am.

Thanks in advance!

"""
This module contains tools for getting input from a user.
At any point while getting input, the user may enter "quit", "exit", or
  "leave" to raise a SystemExit exception and quit.
"""

import textwrap as tw
from enum import Enum, auto

_EXIT_WORDS = {"quit", "exit", "leave"}


class OutputMode(Enum):
    """
    Used to determine the output of the GetNumber function
    """
    INT = auto()
    FLOAT = auto()
    NUM = auto()


def GetStringChoice(prompt, **kwoptions):
    """
    Print out the prompt and then return the input as long as it matches one
    of the options (given as key/value pairs)

    Example call:
        >>> prompt = "Who is the strongest Avenger?"
        >>> input_options = {
                "t":"Thor",
                "i":"Iron Man",
                "c":"Captain America",
                "h":"The Hulk"}
        >>> response = GetStringChoice(prompt, **input_options)
        Who is the strongest Avenger?
         - 't' for 'Thor'
         - 'i' for 'Iron Man'
         - 'c' for 'Captain America'
         - 'h' for 'The Hulk'
        h
        >>> response
        'h'
    Invalid results are rejected:
        >>> response = GetStringChoice(prompt, **input_options)
        Who is the strongest Avenger?
        - 't' for 'Thor'
        - 'i' for 'Iron Man'
        - 'c' for 'Captain America'
        - 'h' for 'The Hulk'
        Ant-Man
        That wasn't one of the options.
        Who is the strongest Avenger?
        ...
    """
    formatted_options = _get_formatted_options(**kwoptions)

    print(tw.fill(prompt))
    while True:
        try:
            print(formatted_options)

            user_choice = input()
            if user_choice in kwoptions:
                return user_choice
            elif user_choice in _EXIT_WORDS:
                _SysExitMsg()

            print("That wasn't one of the options.",)
        except TypeError as t:
            raise t
        except SystemExit as s:
            raise s
        except Exception as e:
            raise e


def _get_formatted_options(**kwoptions):
    """Formats a dictionary of options and returns them as a string"""

    OPTION_TEMPLATE = " - '{0:{1}}' for '{2}'"
    # The 1 as the second arg below is filler because format won't allow 0
    # -2 ensures that the subsequent indent lines up with the first char
    STR_PADDING = len(OPTION_TEMPLATE.format("", 1, "")) - 2

    # This is used to adjust the section before the "-" to be as wide as the
    # longest key
    space = max(map(len, kwoptions))
    pad_length = space + STR_PADDING

    prompt_lines = []

    for key in kwoptions:
        # This wraps the text at the max line length and pads the new
        # lines so it looks nice.
        full_option = tw.fill(
            kwoptions[key],
            subsequent_indent=" " * pad_length)

        prompt_lines.append(OPTION_TEMPLATE.format(key, space, full_option))

    return "\n".join(prompt_lines)


def GetYesNo(prompt):
    """
    Calls GetStringChoice and only allows yes or no as response. Return y/n.

    Example:
        >>> response = GetYesNo("Is Footloose still the greatest movie ever?")
        Is Footloose still the greatest movie ever?
        - 'y' for 'yes'
        - 'n' for 'no'
        It never was!
        That wasn't one of the options.
        Is Footloose still the greatest movie ever?
        - 'y' for 'yes'
        - 'n' for 'no'
        n
        >>> response
        'n'

    """
    return GetStringChoice(prompt, y="yes", n="no")


def GetTrueFalse(prompt):
    """
    Calls GetStringChoice and only allows boolean response.
    Return boolean True or False.

    Example:
        >>> GetTrueFalse("True or False: Star-Lord was responsible for"
                         "the team losing on Titan:")
        True or False: Star-Lord was responsible for the team losing on Titan:
        - 't' for 'True'
        - 'f' for 'False'
        f
        False
        >>>
    """
    if GetStringChoice(prompt, t="True", f="False") == "t":
        return True
    return False


def GetNumber(prompt, min_opt=1, max_opt=10, data_type=OutputMode.NUM,
              restrict_range=False):
    """
    Return the user's choice of number.

    If restrict_range=False, don't restrict the range (deafult).
    Otherwise, restrict answer to between min/max_opt.

    Use data_type to determine what type of number to return, passing in an
      OutputMode enum. Examples:
    - ui.OutputMode.NUM: whatever type the user entered (this is the default)
        >>> my_num = GetNumber("Pick a number:")
        Pick a number:
        5.0
        >>> my_num
        5.0
        >>> my_num = GetNumber("Pick a number:")
        Pick a number:
        5
        >>> my_num
        5
    - ui.OutputMode.INT: integers
        >>> my_num = GetNumber("Pick an integer:", 1, 10, ui.OutputMode.INT,
                               restrict_range=False)
        Pick an integer:
        (min = 1, max = 10)
        5.0
        >>> my_num
        5
    - ui.OutputMode.FLOAT: floats
        >>> my_num = GetNumber("Pick an integer:", 1, 10, ui.OutputMode.FLOAT
                               restrict_range=False)
        Pick an integer:
        (min = 1, max = 10)
        5
        >>> my_num
        5.0
    """
    print(tw.fill(prompt))

    if not restrict_range:
        # User is not restricted to the min/max range
        num_choice = _AcceptAndValidateNumber()
    else:
        num_choice = GetNumberInRange(min_opt, max_opt)

    if data_type == OutputMode.NUM:
        return num_choice
    elif data_type == OutputMode.FLOAT:
        return float(num_choice)
    elif data_type == OutputMode.INT:
        return int(num_choice)


def GetNumberInRange(min_opt, max_opt):
    """
    Let the user pick a number
    Return it as whatever data type the user used
    """

    # This could live in a separate func but then it'd have to assign
    # min/max_opt even when nothing changes
    if max_opt < min_opt:
        # Switch the order if the maximum is less than the minimum.
        # This is done for aesthetics
        min_opt, max_opt = max_opt, min_opt

    if max_opt == min_opt:
        # It makes no sense for these to be equal, so raise an error
        raise ValueError("The min and max numbers should not be the same.\n")

    print("(min = {0:,}, max = {1:,})".format(min_opt, max_opt))

    while True:
        try:
            num_choice = _AcceptAndValidateNumber()

            # Check to see if the num_choice is valid in our range
            if eval("{0}<={1}<={2}".format(min_opt, num_choice, max_opt)):
                return num_choice
            print("Please pick a number between {0} and {1}.".format(
                min_opt,
                max_opt))
                # The comma here places the user's response on the same line
        except SystemExit as s:
            raise s
        except Exception as e:
            raise e


def _AcceptAndValidateNumber():
    """
    Accept a user's choice of number, and then return it as a float or int.

    Type is determined by whether the user includes a decimal point.
    """
    while True:
        try:
            num_choice = input()
            if num_choice in _EXIT_WORDS:
                _SysExitMsg()

            # Return the corresponding number type
            if num_choice.find(".") == -1:
                return int(float(num_choice))
            return float(num_choice)
        except ValueError:
            # Don't raise; just force the user back into the loop
            print("Please pick a number.")
        except SystemExit as s:
            raise s
        except Exception as e:
            raise e


def _SysExitMsg(msg="Thanks!"):
    """
    A consistent process for SystemExit when a user enters one of the
    _EXIT_WORDS
    """
    print(msg)
    raise SystemExit  # Raise the SystemExit exception again to exit

I don't currently have unit tests for this module (I'm struggling with testing incorrect answers), so I use these functions as a way of running through the different variations of input this module can receive:

def main():
    """
    A demonstration function.
    """
    _demonstrateGetNumber()
    _demonstrateGetStringChoice()


def _demonstrateGetNumber():
    print("""
    Demonstration of GetNumber()
    """)

    print("Returns {0}\n".format(GetNumber(
        "Step right up and pick a number, any number!")))

    print("Returns {0}\n".format(GetNumber(
        "Only integers this time (decimals will be rounded). "
        "Pick any integer!",
        data_type=OutputMode.INT)))
    print("Returns {0}\n".format(GetNumber(
        prompt="Now only an integer in the range below!",
        data_type=OutputMode.INT,
        restrict_range=True)))
    print("Returns {0}\n".format(GetNumber(
        "Now pick a float! (root beer not allowed)",
        data_type=OutputMode.FLOAT)))
    print("Returns {0}\n".format(GetNumber(
        prompt="And finally, a float in the given range:",
        min_opt=1,
        max_opt=50,
        data_type=OutputMode.FLOAT,
        restrict_range=True)))
    return None


def _demonstrateGetStringChoice():
    print("""
    Demonstration of GetStringChoice()
    """)

    print("Returns {0}\n".format(GetStringChoice(
        "What does your mother smell of?", e="elderberries", h="hamster")))

    print("Returns {0}\n".format(GetYesNo(
        "That was just a little Python humor. Did you enjoy it?")))

    print("Returns {0}\n".format(GetTrueFalse(
        "Is it true that an African swallow could carry a coconut?")))

    return None
\$\endgroup\$
2
\$\begingroup\$

I think this is a bit overkill. You basically have three requirements, either the user can choose from an iterable of allowed choices, has to enter something that can be interpreted as some specific type or be in some range.

EXIT = {"quit", "exit", "leave"}

def ask_user(message, type_=str, validator=None, invalid="Not valid"):
    if validator is None:
        validator = lambda x: True
    while True:
        user_input = input(message)
        if user_input in EXIT:
            raise SystemExit
        try:
            x = type_(user_input)
            if validator(x):
                return x
            else:
                if isinstance(invalid, Exception):
                    raise invalid
                else:
                    print(invalid)
        except (ValueError, TypeError):
            print("Please pass a", type_)

Your other functions are just special cases of this, no need to repeat everything:

def get_choice(message, choices):
    if isinstance(choices, dict):
        message += "\n" + "\n".join(f"{k}) {v}" for k, v in choices.items()) + "\n"
    else:
        message += "\n" + "\n".join(choices) + "\n"
        choices = set(choices)
    validator = lambda x: x in choices
    user_input = ask_user(message, validator=validator)
    if isinstance(choices, dict):
        return choices[user_input]
    else:
        return user_input

def get_yes_no(message):
    return get_choice(message, ["yes", "no"])

def get_true_false(message):
    return get_choice(message, ["t", "f"]) == "t"

def get_number(message, start=None, stop=None, data_type=float, invalid=None):
    if start is not None and stop is not None:
        validator = lambda x: start <= x < stop
    elif start is not None:
        validator = lambda x: start <= x
    elif stop is not None:
        validator = lambda x: x < stop
    else:
        validator = None
    if invalid is None:
        invalid = "Please pick a number."
    return ask_user(message, data_type, validator, invalid)

def get_number_in_range(message, start, stop, data_type=int):
    message += f"\n(min = {start}, max = {stop - 1})"
    invalid = f"Please pick a number between {start} and {stop - 1}."
    return get_number(message, start, stop, data_type, invalid)

Note that Python has an official style-guide, PEP8, which recommends using lower_case for variables and functions and only use PascalCase for classes.

In addition, functions return None by default, you don't need to explicitly return it. (An argument can be made if you have different possible returns and in the case you do reach the end of the function you need it for clarity. But that is not the case here.)

It does not make sense to except an exception and then just re-raise it without doing anything with it. At the minimum you would want to add some more information for the user.

Silently truncating user input from an entered float to an int seems like a bad idea to me.

Your tests read a bit like reverse polish notation, there is no harm done if you save the result of the user input in a variable first.

In Python 3.6, a new way to format strings was introduced, the f-string. But even before you did not need the positional index in str.format, by default it will align to the order of the passed input.

def _demonstrate_get_number():
    print("""
    Demonstration of get_number and get_number_in_range
    """)

    user_input = get_number("Step right up and pick a number, any number!")
    print(f"Returns {user_input}\n")

    user_input = get_number("Only integers this time. Pick any integer!", data_type=int)
    print(f"Returns {user_input}\n")

    user_input = get_number_in_range("Now only an integer in the range below!", 1, 11)
    print(f"Returns {user_input}\n")

    user_input = get_number("Now pick a float! (root beer not allowed)")
    print(f"Returns {user_input}\n")

    user_input = get_number_in_range("And finally, a float in the given range:",
                                     1, 51, data_type=float)
    print(f"Returns {user_input}\n")


def _demonstrate_get_string_choice():
    print("""
    Demonstration of get_choice()
    """)

    user_input = get_choice("What does your mother smell of?", ["elderberries", "hamster"])
    print(f"Returns {user_input}\n")

    user_input = get_choice("MCU or DCEU?", {"m": "MCU", "d": "DCEU"})
    print(f"Returns {user_input}\n")

    user_input = get_yes_no("That was just a little humor. Did you enjoy it?")
    print(f"Returns {user_input}\n")

    user_input = get_true_false("Is it true that an African swallow could carry a coconut?")
    print(f"Returns {user_input}\n")

if __name__ == "__main__":
    _demonstrate_get_number()
    _demonstrate_get_string_choice()
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for the feedback! It's very helpful. I'm still learning to see more streamlined solutions to problems instead of big chunks that I solve independently. \$\endgroup\$ – DukeSilver Apr 18 at 14:26

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.