7
\$\begingroup\$

I am learning Python at present, from several books. I am new to programming and I need assistance with knowing when my code is written with good form and style - I want to ingrain the habit of writing good clean code, which would be accepted as thus by other programmers. This is a code example from the book Python Programming for the Absolute Beginner by Michael Dawson, which I have altered in several ways - a game which picks a random number and asks the player to guess that number:

# Alex Grattan 
# 8/10/14

# Guess My Number
#
# The computer picks a random number between 1 and 100
# The player tries to guess it and the computer lets
# the player know if the guess is too high, too low
# or right on the money

import random

print("\tWelcome to 'Guess My Number'!")
print("\nI'm thinking of a number between 1 and 100.")
print("Try to guess it in as few attempts as possible.\n")

# set the initial values
the_number = random.randint(1, 100)
guess = int(input("Take a guess: "))
tries = 1

# guessing loop
while guess != the_number:
    if guess > the_number:
        print("Lower...")
    else:
        print("Higher...")

    guess = int(input("Take a guess: "))
    tries += 1
    if tries == 5:
        print "You failed to guess in time!"
        break
    if guess == the_number:
        print("You guessed it! The number was", the_number)
        print("And it only took you", tries, "tries!\n")

raw_input("\n\nPress the enter key to exit.")

The code works, however, I feel it is slightly 'bloated' and messy. How would a more experienced programmer change my code?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ You're asking the user to guess a number within a certain amount of tries while telling him he needs to guess it in as few tries as possible. That's not the same. \$\endgroup\$
    – Mast
    Oct 10, 2014 at 7:26

5 Answers 5

7
\$\begingroup\$

Mixing Python 2 and 3

The program is correct in neither Python 2 nor Python 3.

Your program fails to run at all in Python 3, due to the lack of parentheses when calling

print "You failed to guess in time!"

Also, the raw_input() function does not exist in Python 3.

On the other hand, it's not quite correct under Python 2 either. These print statements

print("You guessed it! The number was", the_number)
print("And it only took you", tries, "tries!\n")

would be interpreted as requests to print tuples… complete with parentheses and commas.

Initialization

The number of allowed guesses, 5, is buried in the middle of your program, and should be defined in an obvious place near the top.

The bounds, 1 and 100, are each hard-coded twice. That is a maintenance pitfall, should you ever need to change the upper bound. Again, these special numbers should be defined just once in an obvious place.

Loop

The "Take a guess" prompting statement is written twice. It should be moved inside the loop, at the top of the loop.

@MasayukiFujita and @MathiasPanda have the right idea, I think, to use the count rather than the correctness as the loop condition. I would go a step further and suggest that counting loops in Python would be more idiomatically written using for counter in range(…).

To distinguish between the winning and losing condition without using a boolean variable or retesting the correctness of the guess, you can take advantage of an obscure Python language feature: an else clause on a loop. The else clause is executed only when the loop terminates "naturally", rather than due to a break.

Newline management

You'll have a better time printing nice output if you make a habit of always putting your newlines at the end of print statements.

Suggested solution

Here is a program that works in both Python 2 and Python 3.

import random

BOUNDS = (1, 100)
TRIES_ALLOWED = 5

the_number = random.randint(*BOUNDS)

print("\tWelcome to 'Guess My Number'!\n")
print("I'm thinking of a number between %d and %d." % BOUNDS)
print("Try to guess it in as few attempts as possible.\n")

for tries in range(TRIES_ALLOWED):
    guess = int(input("Take a guess: "))

    if guess > the_number:
        print("Lower...")
    elif guess < the_number:
        print("Higher...")
    else:
        print("You guessed it! The number was %d" % (the_number))
        print("And it only took you %d tries!\n\n" % (tries + 1))
        break
else:
    print("You failed to guess in time!\n\n")

# Admittedly a contorted way to write a statement that works in both Python 2 and 3...
try:
    input("Press the enter key to exit.")
except:
    pass
\$\endgroup\$
3
  • \$\begingroup\$ You'll allow 5 attempts whereas the OP only allows 4 (might be intentional, might not). \$\endgroup\$
    – Ben
    Oct 9, 2014 at 12:41
  • \$\begingroup\$ @Ben The original code also prompts for five guesses. \$\endgroup\$ Oct 10, 2014 at 20:02
  • \$\begingroup\$ It breaks on the 5th guess before determining whether the guess is correct; I suspect it's wrong, but it'll only allow 4. \$\endgroup\$
    – Ben
    Oct 10, 2014 at 22:40
4
\$\begingroup\$

You can kill off the if/else print in the while loop by using a one liner:

print "Lower..." if guess > the_number else "Higher"

Can also get rid of the last raw_input, as it is just annoying:

raw_input("\n\nPress the enter key to exit.")

Can also get rid of the three print calls in the top and replace it with 1 multiline print:

print("""\tWelcome to 'Guess My Number'!
I'm thinking of a number between 1 and 100.
Try to guess it in as few attempts as possible.\n""")

which leaves us with:

import random

print("""\tWelcome to 'Guess My Number'!
I'm thinking of a number between 1 and 100.
Try to guess it in as few attempts as possible.\n""")

# set the initial values
the_number = random.randint(1, 100)
guess = int(input("Take a guess: "))
tries = 1

# guessing loop
while guess != the_number:
    print "Lower..." if guess > the_number else "Higher"

    guess = int(input("Take a guess: "))
    tries += 1

    if tries == 5:
        print "You failed to guess in time!"
        break

    if guess == the_number:
        print("You guessed it! The number was", the_number)
        print("And it only took you", tries, "tries!\n")
        break
\$\endgroup\$
3
\$\begingroup\$

I am not an experienced programmer so someone will probably have a better solution.. However, this is how I would have written this game. I simply separated the 'tries' loop from the printing of the results.

import random

print("\tWelcome to 'Guess My Number'!")
print("\nI'm thinking of a number between 1 and 100.")
print("Try to guess it in as few attempts as possible.\n")

# set the initial values
the_number = random.randint(1, 100)
tries = 0
maxTries=5
win = False

# guessing loop
while tries < maxTries:
    tries +=1
    guess = int(input("Take a guess: "))
    if guess == the_number:
        win = True
        break
    elif guess > the_number:
        print("Lower...")
    else:
        print("Higher...")

if win:
    print("You guessed it! The number was", the_number)
    print("And it only took you", tries, "tries!\n")
else:
    print "You failed to guess in time!"
\$\endgroup\$
1
  • \$\begingroup\$ I am glad to see differing ways of writing something. It can only improve my skills. Thanks. \$\endgroup\$ Oct 9, 2014 at 3:04
3
\$\begingroup\$

It is a very simple code, so there isn't much room for improvements. If I had to change it though, I would change the while part to:

while tries < 5:
    guess = int(input("Take a guess: "))
    tries += 1

    if guess > the_number:
        print("Lower...")
    elif guess < the_number:
        print("Higher...") #Your original code would show correct answer as "Higher..."
    elif guess == the_number:
        break

if tries == 5:
    print "You failed to guess in time!"
if guess == the_number:
    print("You guessed it! The number was", the_number)
    print("And it only took you", tries, "tries!\n")

input("\n\nPress the enter key to exit.")

Again, since the code is so simple, there isn't much room for improvements.

\$\endgroup\$
3
  • \$\begingroup\$ I am glad to see differing ways of writing something. It can only improve my skills. Thanks. \$\endgroup\$ Oct 9, 2014 at 3:03
  • 2
    \$\begingroup\$ There's a bug where if you guess correctly on the fifth turn, it will print both You failed to guess in time! and You guessed it!. \$\endgroup\$
    – mjolka
    Oct 9, 2014 at 3:13
  • \$\begingroup\$ You shouldn't get to the fifth try because the while loop stops at try 4. \$\endgroup\$ Oct 10, 2014 at 13:41
2
\$\begingroup\$

You should catch the ValueError, when the user enters a value which cannot be converted to an integer.

try:
    guess = int(input("Take a guess: "))
except ValueError:
    print("Please, enter a number value!)
    continue

It is better to keep that part inside the while loop and catch the error only in one place as you did (See @jsanc623's code, where the error can occur twice.)

import random

print("\tWelcome to 'Guess My Number'!")
print("\nI'm thinking of a number between 1 and 100.")
print("Try to guess it in as few attempts as possible.\n")

# set the initial values
the_number = random.randint(1, 100)
guess = 0
tries = 0
max_tries = 5

# guessing loop
print(the_number)
while guess != the_number:
    if tries == max_tries:
        print("You failed to guess in time!")
        break

    try:
        guess = int(input("Take a guess: "))
    except ValueError:
        print("Please enter a numeric value")
        continue

    tries += 1

    if guess == the_number:
        print("You guessed it! The number was", the_number)
        print("And it only took you", tries, "tries!\n")
        break

    print("Lower...") if guess > the_number else print("Higher...")

input("\n\nPress the enter key to exit.")
\$\endgroup\$

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