0
\$\begingroup\$

The program asks for a user to set the max number of the range. That number is used to set the max range of a randomly generated number. The program keeps track of the number of guesses. I have taken a couple classes recently on different programming languages. I wrote a Hi/Low program in C a while back and decided to convert it to Python 3. I would appreciate someone looking at the code of the program and give me feedback on what I did good/bad and ways that I could improve.

def tooHigh():
    guess = int(print("Your guess is too high, please try again: "))
    return guess

def tooLow():
    guess = int(print("Your guess is too low, please try again: "))
    return guess

def guessLoop(x):
    guess=x
    counter+=1
    if guess>gennum:
        guess = tooHigh()
    elif guess<gennum:
        guess = tooLow()
    elif guess==gennum:
        print("you guessed it!")
    return guess

import random
range=int(input("Please enter the maximum number of the range you would like to guess within: "))
gennum = random.randrange(range+1)
guess = int(input("Please enter a number within the range: "))
counter =1

if guess==gennum:
    print("Congrats, you guessed right on the first try!")

while not guess==gennum:
    loop(guess)

print("It took you",counter,"guesses to guess correctly.")
\$\endgroup\$
  • \$\begingroup\$ Why do you print the solution before the player make any attempt? Is it because you don't want them to fail so your program doesn't run into 2 NameErrors? \$\endgroup\$ – 409_Conflict Oct 23 '16 at 21:30
  • \$\begingroup\$ that is because I forgot to delete it from the code after testing it. I wanted to make sure I knew what the number was so I could intentionally guess wrong to make sure the count was correct and that the correct functions were being called. What name errors are you referencing? \$\endgroup\$ – platypus87 Oct 23 '16 at 21:31
  • \$\begingroup\$ And so? Did you test it while guessing wrong? Turns out this doesn't work unless you guess right first try. \$\endgroup\$ – 409_Conflict Oct 23 '16 at 21:38
  • \$\begingroup\$ I originally said it was working for me, because it was when I used my IDE. But not so much when I ran it from command line...my apologies \$\endgroup\$ – platypus87 Oct 23 '16 at 21:42
  • \$\begingroup\$ Then if you have a working version, please, post it. This one has at least 4 issues: NameError, UnboundLocalError, TypeError and infinite loop. Culprits being loop, counter, print and... return, I guess… \$\endgroup\$ – 409_Conflict Oct 23 '16 at 21:46
2
\$\begingroup\$

Here are a few comments:

1) There are three different places in the code where you ask the user to enter the next number to guess. This is not very readable and will backfire if and when you want to extend this code. It's better to have something like this:

def getNextGuessFromUser(messageToPrint):
    print messageToPrint + '\n'
    return int(input())

Then remove the tooHigh, tooLow and initial guess calls, and replace them with calls to this function. By the way, I'm not sure how your reading of guesses works with the print call; looks like a defect to me.

2) Unnecessary usage of global variables. guess should be generated for the first time inside guessLoop, and not rely on the outer code to initialize it.

3) Double check of the right guess. First, you do it in guessLoop to print the right message, and then again in the main module. Again, all of this should be happening in guessLoop: something like this

def guessRun(rightAnswer):
   counter=1
   guess=getNextGuessFromUser("Please enter a number within the range: ")

   while guess != rightAnswer:
     if guess>rightAnswer:
        guess = getNextGuessFromUser("<Too high message>")
     elif guess<rightAnswer:
        guess = getNextGuessFromUser("<Too low message>")
     counter+=1    

   print("you guessed it!")
   return counter

4) Consider using PEP-8.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the feedback. I like your ideas and I am going to try to put them into practice in the next edit. \$\endgroup\$ – platypus87 Oct 23 '16 at 21:56
  • \$\begingroup\$ Ok, in that case do you mind accepting the answer. As a side point, you should only post working code here; next time, someone might correctly put your question on hold. \$\endgroup\$ – RomanK Oct 23 '16 at 22:01
  • \$\begingroup\$ Are you sure you’re using Python 3 and PEP 8 correctly? \$\endgroup\$ – 409_Conflict Oct 23 '16 at 22:07
  • \$\begingroup\$ I'm not. I just followed the style of the original code and recommended switching to PEP-8. My goal was not to fix all the issues in the original snippet. \$\endgroup\$ – RomanK Oct 23 '16 at 22:12

Not the answer you're looking for? Browse other questions tagged or ask your own question.