7
\$\begingroup\$

I've started learning to program, and as the first program I wrote on my own I wanted to come up with a number guessing game - the below is what I got.

I'm mainly looking for a review of my Guess function, but I do have a few specific questions:

  1. Does it make sense to have my loop variable as a function parameter?
  2. I'm currently defining some variables (e.g. tries) outside of my function - is this an idiomatic way to do it?
import random
tries = [1]
def Guess(playing):
    number = random.randint(1,100)
    print(number)
    print("We are going to play high and low, the avabile numbers are from 1 to 100 included.")
    while playing == True:
        user_input = input("What is your number? ")
        if int(user_input) == number:
            print("You have won!")
            playing = False
        elif int(user_input) < number:
            print("You need to give a higher number")
            tries.append(1)
        elif int(user_input) > number:
            print("You need to give an lower number")
            tries.append(1)
        else:
            print("You have put an wrong number")
            playing = False
    print("It took you " + str(sum(tries)) + " turns to guess")
    still = input("Do you want to play again?")
    if still == "yes" or still == "YES" or still == "y" or still == "si":
        Guess(playing=True)
    else:
        playing=False

Guess(playing=True)
\$\endgroup\$
0
8
\$\begingroup\$

Overall you've done a good job with your code, there are a couple things I'd look out for :

  • You aren't paying for whitespace! What I mean is that it's important to let your code breathe. Use some empty lines, it'll do wonder for your eyes when dealing with big chunks of code.
  • Why is tries an array? In my opinion, tries should represent the number of tries, so it should be a number!
  • Notice that you have for conditions, ==, <, > and... ? Say I ask you to give me a number that is neither equal, smaller or greater than 3, is there a possible option? What you should do instead is make sure the player input a number! Right now, if I was to answer "I don't know my numbers I'm just a baby" to "We are going to play high and low, the avabile numbers are from 1 to 100 included.", your program will crash! You should look into try/catch blocks to solve this problem, that could be a next thing to learn for you!
  • Say I need three tries on my first playthrough, then I start a new game (using "yes" when prompted) and I'm able to succeed on the first try, your code will tell me I've needed four tries to succeed. Can you find out why and how you can fix that?
  • You could simplify your loop using the break keyword, that's also something you should search for (while we're at it, try to understand the usage of break, return and continue. Those three keywords are pretty useful, although you probably already know about return).
  • What if I'm an english major and I can't stand making grammar mistakes (It's obviously not my case) and I write "Yes", instead of "yes" or "YES", when asked if I want to play again? What you could do is compare the lowered (or uppered) version of the input with what you want to check (ex. still.lower() == "yes").
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for this guidance. I will try now to check all the things that you have listed here, and later I will try to post the changed program. \$\endgroup\$ – Kacper Krawczyk Sep 1 '20 at 14:10
  • 4
    \$\begingroup\$ @KacperKrawczyk please do not edit your question to include updated code - editing a question that has answers may invalidate those answers. If you'd like, please feel free to wait a few days and post a new question to get review on your updated question \$\endgroup\$ – Dan Oberlam Sep 1 '20 at 14:17
  • 2
    \$\begingroup\$ @KacperKrawczyk As much as I appreciate accepting my answer so fast, I'd wait a little bit more :) Posts with no "accepted answers" tend to attract more attention! \$\endgroup\$ – IEatBagels Sep 1 '20 at 14:28
1
\$\begingroup\$

Here are a couple of improvements I think you could make (IEatBabels touched on a lot of these, but I'd like to expand on some areas and add my own phrasing):

  • Why is tries a list? It would easier and make more sense to just let tries = 0 starting out, and then increment tries (tries += 1) each time the user input an incorrect number.
  • Since you don't have any other functions in your program, I wouldn't even bother with making a Guess function. Just write the code directly in the program.
  • You may have just been debugging your program, but you're showing the user the number they're supposed to guess by writing print(number) in Guess!
  • What happens if the user decides to be cheeky and entire some gibberish (i.e. something that's not a number)? Your program is going to raise an exception when it tries to convert the input to a number. What you should do is wrap the code under your loop in a try/except block. That way, whenever the user inputs gibberish, you can prompt them to entire a valid number.
  • I'm not sure why you have the else block after your if/elif statements? If the user input isn't equal to the number, and it isn't less than the number, and it isn't greater than the number, what else could it be? Nothing! So there's really no need for the else statement there. You might've been trying to use the else statement in cases where the user input wasn't valid. This won't quite work though. If that's what you were trying to do, see my last point.
  • Convert the user input to an integer once, and store it in a variable. This saves time and looks cleaner than converting the user input each time you want to test it.
  • I'm not sure why you're using recursion here? Recursion is an excellent tool, but oftentimes while or for loops work much better. I think your program is one of those cases. Just use two loops - one loop for asking the user if they'd like to play again, and one loop for the actual game.
  • I noticed you used flags to break out of our loops. This is a fine method. But I'd prefer here to just use break. break is a statement which tells Python to immediately jump out of the loop it's currently in. It has the same effect as setting flags to True and/or False.
  • You were pretty good about this in your program, but always make sure to use descriptive variable names, and write clear, explicit code. This makes your code clean and self-documenting, and it lets you come back to it months from now and quickly understand what it does and how works.

Here's how I'd re-write your program, with the above suggestions incorporated, and some formatting, logic, and naming improvements (also IEatBagels made an excellent point about whitespace. Make sure you take this to heart!):

import random


print("We are going to play high and low, the avabile numbers are from 1 to 100 included.")

tries = 0
while True:
    number_to_guess = random.randint(1, 100)
    while True:
        try:
            user_input = input("What is your number? ")
            guess = int(user_input)
        except ValueError:
            print("That's not a valid number! Try again.")
        else:
            if guess == number_to_guess:
                print("You have won!")
                break
            elif int(user_input) < number_to_guess:
                print("You need to give a higher number")
                tries += 1
            elif int(user_input) > number_to_guess:
                print("You need to give a lower number")
                tries += 1

    print("It took you " + str(tries) + " turns to guess")
    still = input("Do you want to play again?")

    if not (still == "yes" or still == "YES" or still == "y" or still == "si"):
        break
\$\endgroup\$
4
  • 1
    \$\begingroup\$ If you wanna play again, you will have to geus the same number. The number_to_guess should be below the first while True: \$\endgroup\$ – SirDuckduck Sep 2 '20 at 9:29
  • \$\begingroup\$ @SirDuckduck thanks, good catch! \$\endgroup\$ – Christian Dean Sep 2 '20 at 9:58
  • \$\begingroup\$ Thank you very much for explaining me those things. I have revised all the things that @IEatBagels had mentioned. Now I will take my time to process your stuff. Just by you two guys I have improved and learnt so many new things that i have to admit and appreciate. I am trying to implement all the things and build a hangman game, more clean with those new tricks. \$\endgroup\$ – Kacper Krawczyk Sep 2 '20 at 18:31
  • \$\begingroup\$ That's awesome @KacperKrawczyk. I love hearing that I'm helping others to learn, grow, and better themselves. Best of luck with your hangman project! \$\endgroup\$ – Christian Dean Sep 2 '20 at 23:20
-2
\$\begingroup\$

I tried this. Once look it's helpful or not.

n=20
count=0
while count<5:
    a=int(input("Enter guess number:"))
    count=count+1
    if a==n:
        print("Guess number matched.")
        print("Number of guess took is "+str(count))
        break
    else:
        if a>n:
            print(str(a)+" is greater than guess number.")
        else:
            print(str(a)+" is less than number.")
        print("Remaining chance is "+str(5-count))
        continue
if count==5:
    print('Game Over')
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review. On this site we review working code and provide suggestions for how that code can be improved. Good answers make one or more insightful observations about the original posts code and might not include any code. Alternate code only solutions are considered bad answers because they don't address the original code in the question. Bad answers can be deleted by the community. Please read the Code Review guidelines for answering. \$\endgroup\$ – pacmaninbw Sep 2 '20 at 11:33

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.