4
\$\begingroup\$

I've completed my first Python project. I created a basic guessing game first, with no extra features other than guessing and random number generation. Afterwards, I've watched python courses on Udemy and slowly I began implementing what I've learned. I have a name system, welcome screen, fully functional betting system, a score system with prizes depending on amount of guesses, etc etc.

I'm super proud of this, and would like it reviewed. Any tips, tricks, etc. is very welcome, same with constructive criticism!

    #pylint:disable=W0621
    #pylint:disable=W0621
    #pylint:disable=W0613
    #pylint:disable=W0312
    #pylint:disable=W0611
from random import randint
import math

######### NUMBER GUESSING GAME ##########

START_BALANCE = 500
POSITIVES = ["yes", "yeah", "y", "yep", "roger", "yea", "positive", "play"]
NEGATIVES = ["no", "nope", "n", "nah", "negative"]
game_state = [True, START_BALANCE]
guess_list = []

choice = ("\nPlay again? Y/N:     ").upper()
userName = input("       Welcome to NumGuess! What is your name?\n")
userName = userName.title()


#### INTRO MENU ####
def menu():

    print(''' \n                        Hello {}!\n
                * The rules are very simple *
--         The AI generates a number from 1 - 100.       --
--    You will have to make a bet and enter your guess.  --
--   You have 10x tries. If you fail, you lose your bet. --
--   The AI will let say if you guessed 'low' or 'high'  --
--    Correct guess = prize. Wrong guess = lost bet.     --

                       - Good Luck! - 
'''.format(userName))

#### MENU SCREEN #### 
def menuPlay():
    try:
        menuPlay = input("              Press any key to start the game.\n")
    except TypeError:
        return menuPlay()
    else:
        if menuPlay.upper() != "":
            return

#### NUMBER GENERATION FUNCTION ####
def xNumbers():
    number = randint(1,100)
    return number

#### TRIES FUNCTION ####
def xTries():
    tries = 10
    return tries

#### BETTING FUNCTION ####
def xBets(balance):
    try:
        print("--------------------------------")
        bet = int(input("Enter your bet:     "))
        if (bet <= balance and bet >= 0):
            return int(bet)
        else:
            print(f"Your bet of {bet}$ has to be less than your balance of {balance}$. Try again.\n")
        return xBets(balance)
    except ValueError:
        return xBets(balance)

#### GUESSING FUNCTION ####
def xGuesses():
    try:
            guess = int(input("Enter your guess:    "))
            if (guess >= 0 and guess <= 100):
                guess_list.append(guess)
                return int(guess)
            else:
                print("Your guess has to be from 0 to 100. Try again.")
                return xGuesses()
    except ValueError:
        xGuesses()

#######################
##### MAIN FUNCTION #####
def main(number, tries, balance, gameWon, guess_list, bet):

    print(f"\nYou have {tries}x tries left.\n")

    scoreFactor = {
    '10':5.0, '9':3.00, '8':2.50, 
    '7':2.00, '6':1.75, '5':1.50, 
    '4':1.35, '3':1.25, '2':1.25, 
    '1':1.25            }

    guess = int(xGuesses())
    gameEnds = False
    if gameWon == True:
        number = int(xNumbers())

    if tries ==0:
        balance -= bet 
        print(f"\nGAME OVER! - YOU ARE OUT OF TRIES!\n- The number was: {number}.\n- Your balance is now: {balance}$")
        print(guess_list)
        gameEnds = True

    elif guess == number:
        gameEnds = True
        prize = bet * scoreFactor[str(tries)]
        prize = math.ceil(prize)
        balance += prize

        print(f"Congratulations, {userName}! 'You win: {prize}$")
        print(f"Your new balance is: {balance}$\n")
        print(guess_list)

    elif guess < number:
        print(f"--------------------------------\nWrong guess\nYour guess is too LOW!\nPrevious guesses: {guess_list}\n--------------------------------")
        tries -= 1
        return main(number, tries, balance, False, guess_list, bet) 

    elif guess > number:
        print(f"--------------------------------\nWrong guess\nYour guess is too HIGH!\nPrevious guesses: {guess_list}\n--------------------------------")
        tries -= 1
        return main(number, tries, balance, False, guess_list, bet)

    if gameEnds is True:
        playerChoice = input(choice).lower()

        if playerChoice in POSITIVES: 
            print(f"New round started!\nYour balance is: {balance}$")
            return [True, balance] 
        elif playerChoice in NEGATIVES:
            print(f"\nThanks for playing, {userName}!\n")
            return [False, balance]

menu()
menuPlay()
tries = xTries()
print("Your balance is: "+str(game_state[1])+"$")

while game_state[0]:
    guess_list = []
    game_state = main(0, tries, game_state[1], True, guess_list, xBets(game_state[1]))




## BY KEYANAB
\$\endgroup\$
2
\$\begingroup\$

xTries doesn't need to be a function. All it's doing is returning 10. Just make it a variable defining the starting number of tries:

STARTING_TRIES = 10

. . .

tries = STARTING_TRIES

Just like what you did with START_BALANCE.


POSITIVES, and similar collections should arguably be sets, not lists. Since you're using them to check for membership in the collection using in, sets will be much faster than lists. In your case here, it doesn't matter. It's a good thing to think about though. Change it to:

# Just change to curly braces to make it a set
POSITIVES = {"yes", "yeah", "y", "yep", "roger", "yea", "positive", "play"}

When using lists, in will have to search potentially the entire list to see if the element is in it. With sets though, in only has to search a small fraction of the list. Once you're dealing with large amounts of data, this can make a significant difference.


Your current way with POSITIVES is more complicated than it needs to be anyway. Unless you want input validation to be super strict, it seems like the only requirement for being classified as "positive" or "negative" is that positive inputs begin with 'y' or 'p', and that negative inputs start with 'n'.

You could just make these quick functions:

def is_positive(s):
    # s is truthy if non-empty
    return s and s[0] in {'y', 'p'} 

def is_negative(s):
    return s and s[0] == 'n'

This is a little more forgiving.

If you could guarantee that the input is non-negative, the s and check would be unnecessary. It's needed here though so s[0] doesn't throw on an empty input. You could also using slicing to avoid the exception:

def is_positive(s):
    # s[0:1] also gets the first character, but returns "" if s is empty
    return s[0:1] in {'y', 'p'} 

def is_negative(s):
    return s[0:1] == 'n'

Although I'm not sure if that's clearer.


menuPlay is very jarring to read. The function is called menuPlay, then you create a local variable called menuPlay. You also try to catch a TypeError, although you appear to be using Python 3. I'm not aware of a case where input would throw a TypeError. And if this is actually Python 2, then a much better solution is just to use raw_input instead.

This is a case where a while True loop (or a do...while if Python had them) is handy. I'd just write this as:

def menu_play():
    while True:
        inp = input("...")

        # Return if the input is non-empty
        if inp:
            return

If you think about it though, is this even necessary? The prompt is "Press any key to start the game". The "enter" key is a valid key though, but pressing just that is rejected by your program. It might make more sense to get rid of that function and just write:

menu()

# We don't care about what key was pressed, just that one (+ enter) was
input("Press any key and enter to continue")

tries = STARTING_TRIES

Python uses snake_case, not camelCase (unless you're working with other code that already uses camelCase). You use it in a few places, but are inconsistent. Just remember, Python uses snake_case (a_b).


bet <= balance and bet >= 0

can be more clearly written as

0 <= bet <= balance

Python, unlike most languages, allows for "chaining" of the comparison operators.


Your design of game_state isn't optimal. Say you leave this code for awhile, and come back. Are you going to be able to accurately remember what game_state[1] represents?

I would use a simple dictionary:

game_state = {"continue?":True, "balance":START_BALANCE}

. . .

print("Your balance is: "+str(game_state["balance"])+"$")

. . .

while game_state["continue?"]:

Which I feel reads better. Using Strings here has the disadvantage though that a typo in the String key when accessing game_state will cause a KeyError to be thrown at runtime. game_state[2] will still cause an exception in your code, but it's arguably easier to typo a String than it is a single digit number.

You could also make this a full class, then your IDE could assist using auto-completion. You're likely only ever going to need a single instance of the class though (with the current design), so I'm not sure it's worth it here.

Your design still has problems though:

  • Really, grouping these two bits of data into a state isn't massively advantageous. You have them grouped inside a global variable, so instead of having two smaller global variables, you have one larger one. Grouping them slightly complicates accessing/reading (since you need to index the global using either a numeric or String key). As long as these are global variables, I don't see much point in grouping them together. If you made them locals that are being passed around, it might make a little more sense, but even then...

  • Does it make much sense to store in the state whether or not a player wants to continue? You only use the first boolean part of the state in two places: returning it from main, and checking for it at while game_state[0]:. It makes sense as a return value from main*, but why is it part of the state? It's only ever needed at the call-site of main. Note how you don't pass their decisions between recursive calls. There doesn't seem to be any reason to store it. I'd make the entire state just the balance.

main can still return whether or not the player wants to continue though:

if playerChoice in POSITIVES: 
    print(f"New round started!\nYour balance is: {balance}$")
    return True, balance # A tuple instead, although a list would still work

elif playerChoice in NEGATIVES:
    print(f"\nThanks for playing, {userName}!\n")
    return False, balance

Then, do something like:

keep_playing = True
while keep_playing:
    guess_list = []
    keep_playing, balance = main(0, tries, global_balance, True, guess_list, xBets(global_balance))

* Arguably, main should be responsible for the looping, and everything you currently have in main should go into a function called play_round or something. In most designs, the main is the central function that ties everything together, and in many languages, it's the obligatory entry point to the program that the user never manually calls. In my opinion, in Python, main should really only be called from an "import guard". It's more inline with the conventions of other languages, and just makes more sense.


As the other answer notes, you're using far too much recursion here. I like recursion, I think it gets too much flak. You're using it here though for cases where simple iteration would be simpler. The major downside of recursion in a language like Python where recursion isn't optimized away is that it can lead to Stack Overflows. With how your code is now, if your user fails a validation check too many times (in say, xBets), you'll recurse too many times, and get a Stack Overflow. I'd write xBets as:

def ask_for_bet(current_balance):
   while True:
       try:
           print("--------------------------------")
           bet = int(input("Enter your bet:     "))
           if (0 <= bet <= current_balance):
               return bet # Bet is already an int, no need for "int" again here

           else:
               print(f"Your bet of {bet}$ has to be less than your balance of {current_balance}$. Try again.")
               # Let it loop again instead of recursing

       except ValueError:
           print("Please enter a valid number")

If you like recursion and want to be ""allowed"" or even encouraged to use it, I'd look into functional languages. Recursion is the main (or arguably the only) way to loop in Haskell, and many other functional languages allow for optimization that prevent Stack Overflows. Scala supports optimization and a @tail-call annotation that warns you if it can't be optimized. Clojure has the recur special form that emulates Tail-Call Optimization. Python's a great language, but there may be others out there that are more in-line with how you approach problems.



Those are the major things I saw. This isn't awful code, but there are a few things that can be improved. Good luck!

\$\endgroup\$
3
\$\begingroup\$

Good job on your first Python project! :)

But there are improvements to be made,

  • Instead of the block-comments use docstrings

    #### INTRO MENU ####
    def menu():
    

    You could do

    def menu():
        "This function will print the intro banner"
    
    • Avoid working in the global namespace

    When you work in the global namespace, it becomes really hard to track that one bug.

    Because when something changes, it's hard to see what part of the program changes that variable

  • Instead of recursive functions start writing iterative functions

    Python doesn't really suit itself for recursion

    There is an recursion limit, say your function will be called a 1000 times, it will break, you can see this behavior with

    >>> print(sys.getrecursionlimit())
    1000
    
  • Don't Repeat yourself

    Notice how the function where you get an input from the user are mostly similar. You can refactor those into one (iterative) function

    def get_user_input(lower_bound, upper_bound, message):
        while True:
            try:
                guess = int(input(message)
                if lower_bound <= guess <= upper_bound:
                    return guess
                print(f"Your guess has to be from {lower_bound} to {upper_bound}. Try again.")
            except ValueError:
                pass
    
  • if gameWon == True: the == ... part is redundent

    Just if True: will suffice

  • You only have to check for positives

    When you ask the user for a restart, a negative is the same as False. So there is no need to check for it. Just check for a positive, else game_ended

  • I advice to check PEP8

    The Python style guide with many good point regarding style

\$\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.