5
\$\begingroup\$

I'm sure you've heard this before, I'm a beginner, however I'm trying to break out of the basics and use more intermediate things like functions and such but I'm having a hard time understanding how to properly integrate them into my code. I've created a number guessing game that uses 2 functions, it has difficulty settings, and even a high score. I was hoping somebody could look over it and help me understand how this could've been written better. (The program works entirely, however will throw an error if you try to input incorrect data).

import random

# Default highscores for difficulties
easyScore = 9999999999
mediumScore = 9999999999
hardScore = 9999999999

attempts = [0]

isPlaying = True

# Setting the global variable dif to 1-3 to determine difficulty later on
def difficultysetting(input):
    global dif

    if input.lower() == "e":
        dif = 1
    elif input.lower() == "m":
        dif = 2
    elif input.lower() == "h":
        dif = 3

# Returns the high score of whatever difficulty the user is playing on
def highscore():
    if dif == 1:
        return easyScore
    elif dif == 2:
        return mediumScore
    elif dif == 3:
        return hardScore

while isPlaying:
    print("Difficulties (E)asy, (M)edium, (H)ard")
    difficulty = difficultysetting(input("Choose a difficulty: "))
    guess = -1

    # Checking which difficulty user is playing on and generating a random number based on that
    if dif == 1:
        number = int(random.randint(1, 100))
    elif dif == 2:
        number = int(random.randint(1, 1000))
    elif dif == 3:
        number = int(random.randint(1, 10000))

    # Prompting user which numbers to guess between per difficulty
    while guess != number:
        if dif == 1:
            guess = int(input("Pick a number between 1-100: "))
        elif dif == 2:
            guess = int(input("Pick a number between 1-1000: "))
        elif dif == 3:
            guess = int(input("Pick a number between 1-10000: "))

        # Lets user know if they've guessed to high or too low
        if guess < number:
            print("Higher")
            attempts.append(guess)
        elif guess > number:
            print("Lower")
            attempts.append(guess)
        else:
            print("That's Correct!")
            print("Guesses: ", len(attempts))

    # Checking if new highscore is lower than old highscore and updating the variable if so
    if dif == 1 and len(attempts) < easyScore:
        easyScore = len(attempts)
    elif dif == 2 and len(attempts) < mediumScore:
        mediumScore = len(attempts)
    elif dif == 3 and len(attempts) < hardScore:
        hardScore = len(attempts)

    # reseting list
    attempts.clear()
    attempts.append(0)

    # Asking user if they'd like to play again and ending while loop if no
    print("Highscore: ", str(highscore()))
    playAgain = input("Do you want to play again? (Y/N): ")

    if playAgain.lower() == "n":
        isPlaying = False
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

PEP-8

First off, some minor things your variable names aren't compliant with PEP-8, which recommends using:

  • UPPER_SNAKE for top-level variables
  • PascalCase for classes
  • lower_snake for ordinary functions and variables

It also has recommendations on indent levels, etc. I suggest you look into getting a linter such as flake8 or pylint and run your code through them to standardise it against others' codes.

You also do something called "shadowing" of functions. Which means using the name of something declared elsewhere and giving it a new meaning. Here:

def difficultysetting(input):

you use the name input which is an internal Python function (and indeed one you've used), but override it with the variable passed into the function. This means that if we wanted to use input in the function, we couldn't. In this case it's not catastrophic, but doing it is bad practice and confusing.

In this case we might want to change it to choice, for example. Now you should note that there exists a function called choice in the random library. However, because we're importing it with its name, in our local namespace, it is called random.choice, so we aren't shadowing it. This is also why importing * can be a bad idea, we might use names which conflict with some functions defined in libraries we've imported.

Handle user input smartly

At the moment, you are putting a lot of trust in the user to enter sensible values to each of your inputs. Realistically, we want to be prepared to handle whatever the user throws at us. Consider your guess:

guess = int(input("Pick a number between 1-100: "))

What happens if a user provides "a" as their guess?

At the moment you crash out with:

ValueError: invalid literal for int() with base 10: 'a'

We can be smarter than this. Consider:

while True:
    guess = input("Pick a number between 1-100: ")
    try:
        guess = int(guess)
    except ValueError:
        print(f"Invalid guess ({guess}), must be valid number")
        continue  # Go back to start of loop and get a new guess
    if 1 <= guess <= 100:
        break
    else:  # Go back to start of loop and get a new guess
        print(f"Invalid guess ({guess}), must be between 1 and 100")

This can even be stuck in a function so it doesn't clutter up our main code.

Functions (and how to use them)

Functions in Python (and most languages) take arguments and return some answers. The idea being that they can be used in multiple different places and be useful. A common structure of a function looks like:

def function_name(argument):
    """ Describe function, what it does
    Describe function arguments
    Describe what function returns
    """
    do stuff to argument(s)
    
    return result

N.B. This is not always the case, there are some cases where you might have no arguments or want a function which doesn't return anything, e.g. to change the input argument or change global settings.

So, taking your difficulty selection as an example:

# It is sometimes recommended to name functions as verbs where appropriate
def get_difficulty(choice: str) -> int:  # Type-hints give information to a user about how your code is meant to be used
    """ Returns difficulty to 1-3 depending on user choice, otherwise raises a ValueError
    :param choice: string containing user-entered choice
    :returns: 1,2 or 3 representing difficulty
    :rtype: int 
    """
    choice = choice.lower()  # Changing the argument here doesn't change the value outside the function as `lower` returns a new string
    if choice == "e":
        difficulty = 1
    elif choice == "m":
        difficulty = 2
    elif choice == "h":
        difficulty = 3
    else:  # Consider what happens if a user enters e.g. "really hard"
        raise ValueError(f"Invalid difficulty, must be one of e,m or h. Received: {choice}")
    return difficulty

Using the right variable for the job

You have a lot of if blocks which all just check against difficulty, e.g. to get which score, to get which max number, how many attempts, etc. This might be best done using arrays/tuples and we simply index into the array with difficulty. N.B.: This does mean the difficulties become 0, 1, 2 as arrays are indexed from 0

I.e.

SCORES = [9999999999] * 3
DIFFICULTY_RANGES = [100, 1000, 10000]

...
while isPlaying:
    ...
    # You could set a variable to store DIFFICULTY_RANGES[difficulty] when you get difficulty, so you don't have to use the full name every time 
    number = random.randint(1, DIFFICULTY_RANGES[difficulty])

    while guess != number:
        guess = int(input(f"Pick a number between 1-{DIFFICULTY_RANGES[difficulty]}: "))
        ...

    if len(attempts) < SCORES[difficulty]:
        SCORES[difficulty] = len(attempts)

Also, your attempts is a list of prior guesses, but as it is, you don't use it as a list, you just check against its length. This means that it could probably just be an integer which counts up each time.

Summary

Putting all this together along with a few other minor changes we end up with:

"""
Play number guessing game
"""

import random

# Max number for difficulty
DIFFICULTY_RANGES = [100, 1000, 10000]


def get_difficulty() -> int:
    """ Returns difficulty to 0-2 depending on user choice

    :returns: 0, 1 or 2 representing difficulty
    :rtype: int
    """
    difficulty = -1
    while difficulty < 0:
        choice = input("Choose a difficulty, (E)asy, (M)edium, (H)ard: ").lower()

        if choice == "e":
            difficulty = 0
        elif choice == "m":
            difficulty = 1
        elif choice == "h":
            difficulty = 2
        else:
            print(f"Invalid difficulty choice ({choice}) must be one of e, m or h")

    return difficulty


def get_guess(max_number: int) -> int:
    """ Get valid user guess, i.e. is a number and in range

    :returns: User guess 
    :rtype: int
    """
    while True:
        guess = input(f"Pick a number between 1-{max_number}: ")
        try:
            guess = int(guess)
        except ValueError:
            print(f"Invalid guess ({guess}), must be valid number")
            continue

        if 1 <= guess <= max_number:
            return guess

        print(f"Invalid guess ({guess}), must be between 1 and {max_number}")


def play_game(max_number: int) -> int:
    """ Play a round of the guessing game

    :param max_number: Maximum value in range to guess
    :returns: Number of attempts
    :rtype: int
    """

    number = random.randint(1, max_number)
    guess = -1
    attempts = 0

    # Prompting user which numbers to guess between per difficulty
    while guess != number:
        guess = get_guess(max_number)

        # Lets user know if they've guessed to high or too low
        if guess < number:
            print("Higher")
            attempts += 1
        elif guess > number:
            print("Lower")
            attempts += 1
        else:
            print("That's Correct!")
            print("Guesses: ", attempts)

    return attempts


def main():
    # Default highscores for difficulties
    scores = [9999999999] * 3

    is_playing = True

    while is_playing:

        difficulty = get_difficulty()

        max_number = DIFFICULTY_RANGES[difficulty]

        attempts = play_game(max_number)

        # Checking if new highscore is lower than old highscore and updating the variable if so
        if attempts < scores[difficulty]:
            print("New high score!")
            scores[difficulty] = attempts

        print(f"Current Highscore: {scores[difficulty]}")

        # Asking user if they'd like to play again and ending while loop if no
        while True:
            play_again = input("Do you want to play again? (Y/N): ").lower()

            if play_again == "n":
                is_playing = False
                break

            if play_again == "y":
                break

            print(f"Invalid option ({play_again}), must be Y/N")


if __name__ == '__main__':   # Main guard means that code will only run if called as `python guessing_game.py` and not when imported as a package
    main()

There's certainly some more room for expansion, e.g. saving and loading high scores from a file, if you want some extra challenge.

\$\endgroup\$
4
  • \$\begingroup\$ The suggested implementation violates PEP8 by using SHOULD_CASE variables. It also invites a risk of shadowing variables by having them in the global namespace (under the if name guard = still global namespace). A simple fix is to move all the code under the if name guard into a function called main(), call that, and fix the variable names. I urge you fix that. Other than this, very nice answer, keep it up! \$\endgroup\$
    – janos
    Sep 22 at 16:29
  • \$\begingroup\$ I just want to say, and I know I'm late to say this, but thank you! To be honest a lot of the concepts you're using here are way above my skill level and I've sat here and read over and over again trying to understand a little bit but I'm struggling. However I will come back to this from time to time to value my progress! \$\endgroup\$ Sep 23 at 8:40
  • \$\begingroup\$ @TrevnJones The important thing to take away is the bit about functions. Pass arguments in, get results out. Try to avoid globals. \$\endgroup\$ Sep 23 at 9:21
  • \$\begingroup\$ There's a bug in your solution. difficulty_ranges should be DIFFICULTY_RANGES \$\endgroup\$
    – sarema
    Sep 23 at 11:08
5
\$\begingroup\$

Adding to @DeathIncarnate's great answer:

Don't use global variables

It's fine to access global constants, but manipulating global variables such as dif is bad practice. If you need to store state, use classes.

Encapsulate game information in classes

In your original code, information about the high scores, min and max boundaries for each difficulty level are scattered throughout your code. Encapsulate them in an appropriate class.

If you do so, you can also easily map the respective user shortcut to the respective game mode.

Unused code

You store the highscores during runtime, but you never read them to the user. You can do this before each new round. Remember, that without an external file storage, they will be lost as soon as the program terminates.

Use existing library code

Knowing where to look for existing code can be tricky and in my example may even be controversial. But I happen to know, that configparser.ConfigParser has a boolean parser, that we can reuse to evaluate yes/no input from the user.

Suggested

#! /usr/bin/env python3
"""Number guessing game."""

from configparser import ConfigParser
from dataclasses import dataclass
from random import randint
from sys import stderr


BOOLEAN_MAP = ConfigParser.BOOLEAN_STATES


@dataclass
class Game:
    """Game settings."""

    max: int
    min: int = 1
    highscore: int | None = None

    @property
    def guess_prompt(self) -> str:
        """Returns the prompt for the guess input."""
        return f'Pick a number between {self.min}-{self.max}: '

    def random_choice(self) -> int:
        """Returns a random choice from the numbers range."""
        return randint(self.min, self.max)

    def read_guess(self) -> int:
        """Reads an integer guess."""
        while True:
            try:
                guess = int(input(self.guess_prompt).strip())
            except ValueError:
                print('Not an integer.', file=stderr)
                continue

            if self.min <= guess <= self.max:
                return guess

            print('Guess out of range.', file=stderr)

    def update_highscore(self, score: int) -> None:
        """Update the highscore."""
        if self.highscore is None or score < self.highscore:
            self.highscore = score


DIFFICULTIES = {
    'e': Game(100),
    'm': Game(1000),
    'h': Game(10000)
}


def read_bool(prompt: str) -> bool:
    """Read a boolean value."""

    while True:
        try:
            return BOOLEAN_MAP[input(prompt).strip().lower()]
        except KeyError:
            print(f'Please enter one of: {list(BOOLEAN_MAP.keys())}',
                  file=stderr
            )


def read_game(prompt: str) -> Game:
    """Read a game type from the user."""

    while True:
        try:
            return DIFFICULTIES[input(prompt).strip().lower()]
        except KeyError:
            print('Invalid difficulty!', file=stderr)


def next_round() -> bool:
    """Play the next round.
    Return True iff another round is desired.
    """

    game = read_game(
        'Difficulties (E)asy, (M)edium, (H)ard\n'
        'Choose a difficulty: '
    )
    target_value = game.random_choice()
    guesses = []
    print('Current highscore is:', game.highscore)

    while (guess := game.read_guess()) != target_value:
        print('Higher!' if guess < target_value else 'Lower!', file=stderr)
        guesses.append(guess)

    print("That's Correct!")
    game.update_highscore(attempts := len(guesses))
    print('Guesses: ', attempts)
    return read_bool('Do you want to play again? (yes/no): ')


def main() -> None:
    """Play the game."""

    try:
        while next_round():
            pass
    except (EOFError, KeyboardInterrupt):
        print('\nBye!')


if __name__ == '__main__':
    main()
\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your response! I said this in the comment above, but man some of the expressions and things you're using here are making my brain hurt trying to figure it all out haha, but I got exactly what I was hoping for! I don't quite understand what the -> thing is or what it means, why "def main() -> None:" points an error to the word none, or how "name == 'main'" understands you're talking about class file name \$\endgroup\$ Sep 23 at 8:43
  • 1
    \$\begingroup\$ Those are (return) type hints. The other one is the main guard. \$\endgroup\$ Sep 23 at 8:45
  • \$\begingroup\$ Reading up on it now, thank you for the link! And thank you for your time and detailed explanations here :) \$\endgroup\$ Sep 23 at 8:48

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.