4
\$\begingroup\$

I am a newbie in Python and would very much need help in arranging and setting out function. I have programmed a number guessing game. The outlook of the program is very messy but I don't where to fix since my Python knowledge is very limited.

import random


def random_num():
   # bound preferences and number of trials to win
   lower_bound = input("what is your lower bound: ")
   upper_bound = input("What is your upper bound: ")
   trials = input("How many time would you like to try: ")
   ending = "Thanks For Playing"
   # zero and below "trials" input checker
   if int(trials) <= 0 or int(trials) == 0:
       replay = input("You sure you don't want to play(Yes/No) ")
       if replay in ["yes"]:
           return ending
       else:
           return random_num()
   # variables for ending, continue and count(number of turns taken to compares to number of trials)
   ending = "Thanks For Playing"
   continue_game = (
       "That's it. Do you want to (1)continue the game or (2)restart or (3)neither?: "
   )
   count = 0
   # Functions for the purpose of allowing the user to continue in the desire stage when win
   # or inputting wrong input
   def retries_wrong():
       retry = input(
        "Wrong input. Do you want to (1)restart, (2)stop or (3)retry at the same range : "
       ).lower()
       if retry in ["1"]:
           return random_num()
       elif retry in ["3"]:
           return guess()
       else:
           return ending

   def retries_win():
       win = input(continue_game).lower()
       if win in ["1"]:
           guess()
       elif win in ["2"]:
           random_num()
       else:
           return ending

   # The function guess is for user guess with loop until win or out of turns.
   def guess():
       random_number = random.randint(int(lower_bound), int(upper_bound))
       timer = count
       user_input = input(
           "What is the number that I am thinking in the range {} - {} only {} tries:".format(
            lower_bound, upper_bound, trials
        )
    )

       while True:
           try:
               int(user_input)
               answer = int(user_input)
               if timer == int(trials) - 1:
                   return retries_wrong()
               else:
                   if answer == random_number:
                       retries_win()

                   elif answer >= random_number:
                       user_input = input("The number is smaller than that: ")
                   else:
                       user_input = input("The number is bigger than that: ")
                   timer += 1
           except ValueError:
               return retries_wrong()

   return guess()


print(random_num())
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

Your code looks pretty messy partly because you're under utilizing functions.

I like to use these general rules of thumb:

  • Is the method longer than 5 lines? -> Extract some of it.
  • Does the method have more than one level of indention? E.g. nested ifs or loops? -> Extract until it only has one level indention.

Basically, if you can put a comment over a block of code with the intent of what it does, not how it does it, you can extract it to a function with the intent as the name.

This way, you don't have to read the content of a function to understand what it does, only if you want to know how it does that. And if you're reading the function being called, you don't have to inspect its content, because the intent of the function is in its name.

For more information view How do I make my code more readable?

\$\endgroup\$
1
\$\begingroup\$

Function nesting

All of your functions are nested into random_number, which is not a usual way of constructing a structured program. What you probably want to do is instead pass arguments to the functions (through the brackets) of sensible names and use those arguments.

Function names

Your function names are not very intuitive. random_num doesn't return a random number to me like I might expect, instead it is the variables needed to start a game. As such, you may want to rename it start_game or get_options, likewise guess is play_round

Repetition

retries_win and retries_wrong are basically the same function, we can merge them together into one which prints the appropriate message based on a victory argument.

Main

In Python it is common practice to not put scripts on the lowest level, using the paradigm:

if __name___ == "__main__":

if the code is called as python my_program.py, then it will start the game, however, this still allows functions to be imported and used by other programs without disruption. While this isn't important now, it's a good habit to get into.

Constants

You have constants in your functions which are used throughout. You can bring these outside the function. PEP-8, the style-guide for Python recommends these are UPPER_SNAKE case.

Comparisons

You have a conditional which looks like the following:

if int(trials) <= 0 or int(trials) == 0:

likewise in

if answer == random_number:
    retries_win()
elif answer >= random_number:
    user_input = input("The number is smaller than that: ")
else:
    user_input = input("The number is bigger than that: ")

The <=/== already checks if it's equal, we only need to do one. In the latter, although it makes no technical difference, it can be a better statement of intent to use an explicit answer < random_number

You also check against lists of one object to verify whether a string matches. This can just be done with an equality operator.

if win in ["1"]:

becomes

if win == "1":

or using input_int (below), win==1

Confusing or misleading prompts

You have several places in the code where your prompts are confusing or misleading. For confusing:

"That's it. Do you want to (1)continue the game or (2)restart or (3)neither?: "

What does "neither" mean here? Also you can enter 42 and it will act as option 3.

For misleading:

replay = input("You sure you don't want to play(Yes/No) ")

Where the comparison only checks for "yes" (lower case) and anything else is "no"

Types

trials is cast to an int multiple times, and most of the things you are trying to read are ints . We can do this (and other casts) when we read in the user input. Maybe with some kind of function which checks the validity, e.g.:

def input_int(prompt: str) -> Union[int, None]:
    while True:  ### Ask until valid answer is given
        x = input(prompt)
        try:
            if x == "Q":
                return None ### We can handle this case usefully elsewhere
            x_int = int(x)
            break
        except ValueError:
            print(f"Invalid integer {x}, please re-enter or type 'Q' to quit and give up")

    return x_int

F-strings

Modern python uses f-strings (post 3.6) to format strings. We can replace a format statement with:

    "What is the number that I am thinking in the range {} - {} only {} tries:".format(
            lower_bound, upper_bound, trials
        )
    ("What is the number that I am thinking of"
     f"in the range {lower_bound} - {upper_bound} only {trials} tries:")

Recursion to check

At the moment, your code recurses (function calls itself) if you don't enter a valid trials number, this means that we pause running our current function and create another one on the stack. If we enter an invalid number 100 times, we can crash the program.

We can use a while and a break to ask until we're happy with the answer (see under Types).

NOP

The line

int(user_input)

Does nothing and can be removed (especially with our input_int function)

Docstrings, type hints and comments

Use docstrings to say what you expect a function or module (program) to do and what you expect the user to do with it. This also provides basic help to a user.

Advanced

Python allows us to use loops and early exit as control flow through the for-else construct. This is something fairly unique to Python, but can be a useful paradigm. The else block is entered if the loop exits normally rather than through an early break.

Applying these things

Taking your kind of structure, I might write this more like:

"""
My guessing game, try to work out the number that I'm thinking of!
"""


import random
from typing import Union

ENDING = "Thanks For Playing"
WIN_GAME = "That's it."
LOSE_GAME = "Wrong answer."
OPTIONS = "Do you want to (1)continue, (2)stop or (3)retry at the same range: "

NEW = 1
STOP = 2
RETRY = 3


def get_options():
    """ Get game options from the player """
    # bound preferences and number of trials to win
    lower_bound = input_int("what is your lower bound: ")
    upper_bound = input_int("What is your upper bound: ")
    trials = input_int("How many times would you like to try: ")
    # zero and below "trials" input checker
    if trials <= 0:
        replay = input("You sure you don't want to play(Yes/No): ")
        if replay.lower() == "yes":
            return None, None, None

    return lower_bound, upper_bound, trials


def round_finished(victory: bool) -> int:
    """ Print game state message and determine whether user wishes to continue """
    if victory:
        print(WIN_GAME)
    else:
        print(LOSE_GAME)

    # 1, 2, 3 are the only valid options (CONTINUE, STOP, RETRY)
    answer = input_int(OPTIONS, low=1, high=3)
    return answer


# The function guess is for user guess with loop until win or out of turns.
def play_round(lower_bound: int, upper_bound: int, trials: int, retry=None):
    """ Play a round of the guessing game """

    if retry is None:
        random_number = random.randint(lower_bound, upper_bound)
    else:
        random_number = retry

    for round_n in range(trials):
        answer = input_int(
            "What is the number that I am thinking of "
            f"in the range {lower_bound} - {upper_bound} only {trials-round_n} tries remain: ",
            low=lower_bound, high=upper_bound
        )
        if answer is None:
            return False

        if answer == random_number:
            victory = True
            break

        if answer > random_number:
            print("The number is smaller than that.")
        elif answer < random_number:
            print("The number is bigger than that.")
    else:
        print(f"The random number was: {random_number}")
        victory = False
    return victory


def input_int(prompt: str,
              low: Union[None, int] = None, high: Union[None, int] = None) -> Union[None, int]:
    """
    Get an int from the user.
    If it's outside the range (low, high) or otherwise invalid, ask again.
    Also allow the user to give up with "Q".
    """
    while True:
        val = input(prompt)
        try:
            if val == "Q":
                return None
            val_int = int(val)
            if ((low is not None and low > val_int) or
                (high is not None and val_int > high)):  # Check if valid choice
                print(f"Invalid option {val_int}, must be in range ({low},{high})")
            else:
                break
        except ValueError:
            print(f"Invalid integer {val}, please re-enter or type 'Q' to quit and give up")

    return val_int


if __name__ == "__main__":
    # When we start we're playing a new game
    CONTINUE_PLAYING = NEW
    while CONTINUE_PLAYING != STOP:
        if CONTINUE_PLAYING == NEW:  # Get new bounds and attempts
            lb, ub, attempts = get_options()
            if lb is None:
                break
        elif CONTINUE_PLAYING == RETRY:  # Don't need new bounds
            pass
        WIN = play_round(lb, ub, attempts)
        CONTINUE_PLAYING = round_finished(WIN)

There're quite a lot of things here of different levels of complexity and I probably wouldn't write it like this myself, but I just want to give you some idea of things you might want to look into.

\$\endgroup\$

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.