3
\$\begingroup\$

First time posting to Code Review. Just looking to get some advice on how I can write better code.

I found the following exercise for a "Guess my Number" game on DaniWeb, but the linked source code file is no longer active. So while I know my code works, I'm not sure if I've written it in the best way.

"The computer randomly generates a number. The user inputs a number, and the computer will tell you if you are too high, or too low. Then you will get to keep guessing until you guess the number."

(I also added a limited number of tries to the game, so the user can't try 1-10 in order until they guess right.)

from random import randint

### Variable declarations
correct = False
tries = 0
number = randint(1,10)
###

print ("I'm thinking of a number between 1 and 10.")
print ("I'll give you 5 chances to guess it.\n")

# Ask the user for their guess, as long as there are tries remaining
# and they haven't already guessed correctly.
while correct == False and tries < 5:

    guess = raw_input("What number am I thinking of?: ")

    # Make sure user's input is an integer
    try:
        guess = int(guess)

        if guess == number: # Correct answer
            correct = True
            print ("\nYou got it!")

        elif guess > number:    # Guess too high
            if guess > 10:
                tries += 1
                print ("\nWay too high! Try a number 10 or lower.")
            else:
                tries += 1
                print ("\nTry something lower.")

            remaining = 5 - tries
            print ("\nYou have " + str(remaining) + " guesses left.\n")

        else:
            if number > guess > 0:    # Guess too low
                tries += 1
                print ("\nTry something higher.")
            else:
                tries += 1
                print ("\nMake sure you guess a number 1 or higher!")

            remaining = 5 - tries
            print ("\nYou have " + str(remaining) + " guesses left.\n")

    # Input not an integer
    except ValueError:
        print ("\nThat wasn't a number! Try again.")
        print ("(I won't count that attempt.)")

        remaining = 5 - tries
        print ("\nYou have " + str(remaining) + " guesses left.\n")


### Game over! ###

# Ran out of guesses
if tries > 5:
    print ("\nSorry, you ran out of guesses!")
    print ("My number was " + str(number) + ".")

# Exited loop with tries left, must have guessed correctly
else:
    print ("Well done. Thanks for the game.")

I'd love to get some feedback on what I have here. Thanks in advance!

\$\endgroup\$
6
\$\begingroup\$

Magic numbers

1, 10, and 5 are so-called magic numbers in the code: they are duplicated in many places. The numbers themselves don't carry a meaning, but they are special to the program. If you wanted to change the maximum number of tries, you would have to manually search and replace all occurrences of 5. If the number 5 is used for other than the purpose of maximum number of tries, then the change will be especially difficult, as you would have to review each occurrence to verify its purpose.

The solution is simple: give the magic numbers meaningful names, for example:

lower_limit = 1
upper_limit = 10
max_tries = 5

You might even want to make these constants by uppercasing the names.

Copy-paste coding

This piece of code appears multiple times:

remaining = 5 - tries
print("\nYou have " + str(remaining) + " guesses left.\n")

Most probably you copy-pasted it every time you needed. Try to avoid pressing ControlV while programming. Try solution is almost always to create a new function, for example:

def print_remaining_guesses(tries):
    remaining = max_tries - tries
    print("\nYou have " + str(remaining) + " guesses left.\n")

Use "{}".format(...) for formatting

Instead of:

print("\nYou have " + str(remaining) + " guesses left.\n")

The recommended way is:

print("\nYou have {} guesses left.\n".format(remaining))

This is a kind of templating. By removing + varname + from the middle of strings, the text becomes a bit easier to read. Another benefit is that you can drop the str(...) wrapper, format will automatically take care of that for you.

Pull common operations up

Looking at this code:

if guess > 10:
    tries += 1
    print("\nWay too high! Try a number 10 or lower.")
else:
    tries += 1
    print("\nTry something lower.")

The value of tries will be increment no matter what. So you can pull it up, out of the if-else:

tries += 1
if guess > 10:
    print("\nWay too high! Try a number 10 or lower.")
else:
    print("\nTry something lower.")

Keep try blocks small

You have a very large try block here:

try:
    guess = int(guess)

    # ... many lines ...

# Input not an integer
except ValueError:
    # ...

Since the only place where you expect something to go wrong is the guess = int(guess) line, it would be better to wrap only that in the try-except.

Comparison with boolean values

Don't use == or != with boolean values. Instead of:

while correct == False:

The recommended way is:

while not correct:

Pointless comments

The comments on these lines really don't tell anything new:

if guess == number:  # Correct answer
    # ...

elif guess > number:  # Guess too high

Let the code speak for itself. Then you really don't need comments at all.

Formatting

The space between print and ( is redundant, you should remove it:

print ("I'm thinking of a number between 1 and 10.")

Python has an official style guide called PEP8, I suggest to review it and follow it.

\$\endgroup\$
  • 1
    \$\begingroup\$ Nice answer, but I'm kinda disappointed you failed to advise an upgrade to Python 3, or at least 2.7.11. There's no reason to use / learn an obsolete version. \$\endgroup\$ – cat Mar 12 '16 at 15:19
  • \$\begingroup\$ @tac plenty of companies do not use 3+ versions of python... \$\endgroup\$ – enderland Mar 12 '16 at 17:28
  • \$\begingroup\$ @enderland Plenty of companies still use Java 4, C# 3 or freakin' Pascal. That's no reason to not use / learn newer versions of said languages, or to not learn many languages. \$\endgroup\$ – cat Mar 12 '16 at 18:56
  • \$\begingroup\$ @cat There are valid reasons to use python2.7 though, some modules only work in 2.7. Although I agree with you that here this is not the case. \$\endgroup\$ – Ludisposed Jun 27 '17 at 13:35
  • \$\begingroup\$ @Ludisposed Such modules should be rewritten or updated to work in newer versions. If the functionality you need is only available in an obsolete version, and since that's the only reason to use such an obsolete version, implement the functionality yourself and publish it for Python 3. \$\endgroup\$ – cat Jun 27 '17 at 15:36
1
\$\begingroup\$

@janos' answer is obviously great and exhaustive, but the first thing I thought when I read the title is "Why 2.7.9?"

If you, for some reason, like living in the lame, sub-par past, then at least use 2.7.11, which contains many bug and security fixes.

But really, you should go all out and upgrade to the far more modern Python 3 -- it's really quite a terrific language. From what I can tell, this code will run as-is in Python 3, and faster at that.

\$\endgroup\$
  • \$\begingroup\$ Yeah, I strongly agree and to be totally coherent Python 3.5.1. \$\endgroup\$ – JumpAlways Mar 12 '16 at 17:07

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.