Guessing Game in Python

Question

Now that I've got this working, any improvements that I can make to better my code review would be helpful.

import random
import math
import console
console.set_font('Arial-BoldMT',15)
print'Welcome to the guessing game!\nA number will be randomly chosen from 0 to 1000.\nThe player will make a guess, and then the computer will guess. Who ever is closest wins that round!\nFirst to 5 wins!'
rounds = 0
run_again = 'y'
player_wins = 0
computer_wins = 0
draws = 0
while run_again == 'y':
 number = random.randint(0,1000)
 player_number = input('\nPlayer: ')
 computer_number = random.randint(0,1000)
 print 'Computer:', computer_number
 print 'Number:', number
 player_score = math.fabs(player_number - number)
 computer_score = math.fabs(computer_number - number)
 if player_score>=computer_score:
 computer_wins+=1
 console.set_color(1.00, 0.00, 0.00)
 print 'You lost that round'
 console.set_color(0.00, 0.00, 0.00)
 if player_score<=computer_score:
 player_wins+=1
 console.set_color(0.00, 0.00, 1.00)
 print 'You won that round'
 console.set_color(0.00, 0.00, 0.00)
 if player_score==computer_score: 
 draws+=1
 console.set_color(0.00, 1.00, 0.00)
 print 'That round was a tie'
 console.set_color(0.00, 0.00, 0.00)
 rounds +=1
 if rounds == 5:
 if player_wins > computer_wins:
 console.set_color(0.00, 0.00, 1.00)
 print '\nYOU WON THE GAME'
 console.set_color(0.00, 0.00, 0.00)
 break
 elif computer_wins > player_wins:
 console.set_color(1.00, 0.00, 0.00)
 print '\nYOU LOST THE GAME'
 console.set_color(0.00, 0.00, 0.00)
 break
 else:
 print "Wrong counts"

• flakes: I would recommend diving into the world of functions. It will help you better organise your sections, and be easier to debug codereview.stackexchange.com/questions/46482/…

Answer 1

I think the biggest thing here is that you need to create some functions and encapsulate stuff.

Big Points

1. Use reusable functions, especially when you find yourself copy-and-pasting code.
2. Don't hardcode anything; use constants rather than "magic numbers".
3. Don't use global variables.
4. Wrap it in a main().

Specifics

The minimum and maximum can be constants.

MINIMUM = 0
MAXIMUM = 1000

... Then, all you have to do is change these variables if you want to modify the game in the future:

number = random.randint(MINIMUM, MAXIMUM)
player_guess = input('\nPlayer: ')
computer_guess = random.randint(MINIMUM, MAXIMUM)

Break up the big multi-line message at the beginning rather than having it stretch on for 200 characters. (Purists will still say that the implemenation below has the second line too long, as the Python standard is to have no line exceeding 80 characters, but you get the idea.)

print 'Welcome to the guessing game!'
print 'A number will be randomly chosen from ' + str(MINIMUM) + ' to ' + str(MAXIMUM)
print 'The player will make a guess, and then the computer will guess.'
print 'Whoever is closest wins that round!'
print 'First to 5 wins!'

You can have run_again directly store whether or not to run again (i.e., a boolean value) rather than a character. This lets you check directly against it:

run_again = True
// ... then, elsewhere ...
if run_again:
 //do stuff

You can encapsulate the entire "guessing round" of the game into its own function which then returns whether or not the player won. This will let you have a bit more control over your code structure:

def perform_guessing(number):
 player_guess = input('\nPlayer: ')
 computer_guess = random.randint(MINIMUM, MAXIMUM)
 print 'Computer: ' + str(computer_guess)
 print 'Number: ' + str(number)
 player_score = math.fabs(player_guess - number)
 computer_score = math.fabs(computer_guess - number)
 return player_score < computer_score

This will now return True if the player has won the round, so we can use it as such:

while run_again:
 if perform_guessing(random.randint(MINIMUM, MAXIMUM)):
 player_wins += 1
 console.set_color(0.00, 0.00, 1.00)
 print 'You won that round'
 console.set_color(0.00, 0.00, 0.00)
 else:
 computer_wins += 1
 console.set_color(1.00, 0.00, 0.00)
 print 'You lost that round'
 console.set_color(0.00, 0.00, 0.00)

(Note that in your code, it's actually impossible for a draw to occur, since you check both of the winning conditions with <= and >=, meaning that the == condition will be caught in one of those. This may be a bug in your program. :) )

I might even make a color_print function to avoid some copy-and-pasting:

def color_print(text, r, g, b):
 console.set_color(r, g, b)
 print text
 console.set_color(0.00, 0.00, 0.00)

This would reduce our code above to:

while run_again:
 if perform_guessing(random.randint(MINIMUM, MAXIMUM)):
 player_wins += 1
 color_print('You won that round!', 0.0, 0.0, 1.0)
 else:
 computer_wins += 1
 color_print('You lost that round.', 1.0, 0.0, 0.0)

(I don't actually know what the three decimal values represent when you're setting the console color. If they're not RGB, you can rename them accordingly. But you get the idea.)

Finally, it's standard to encapsulate your functionality into a def main() and call it like so:

if __name__ == '__main__':
 main()

This lets you load you import things from this file into another file later without it actually executing.

---

Putting it all together, we get something that looks like this. If it were me, I'd probably further specify the color_print function so that you could pass in something like Color.RED or Color.YELLOW rather than the three decimals, but that can be for a future release, haha. There's some further encapsulation you could do to make the code still more elegant and readable, but I leave that as an exercise to the reader.

import random
import math
import console
MINIMUM = 0
MAXIMUM = 1000
FONT = 'Arial-BoldMT'
FONT_SIZE = 15
def perform_guessing(number):
 player_guess = input('\nPlayer: ')
 computer_guess = random.randint(MINIMUM, MAXIMUM)
 print 'Computer: ' + str(computer_guess)
 print 'Number: ' + str(number)
 player_score = math.fabs(player_guess - number)
 computer_score = math.fabs(computer_guess - number)
 return player_score < computer_score
def color_print(text, r, g, b):
 console.set_color(r, g, b)
 print text
 console.set_color(0.00, 0.00, 0.00)
def main():
 console.set_font(FONT, FONT_SIZE)
 print 'Welcome to the guessing game!'
 print 'A number will be randomly chosen from ' + str(MINIMUM) + ' to ' + str(MAXIMUM)
 print 'The player will make a guess, and then the computer will guess.'
 print 'Whoever is closest wins that round!'
 print 'First to 5 wins!'
 run_again = True
 player_wins = 0
 computer_wins = 0
 rounds = 0
 while run_again:
 rounds += 1
 if perform_guessing(random.randint(MINIMUM, MAXIMUM)):
 player_wins += 1
 color_print('You won that round!', 0.0, 0.0, 1.0)
 else:
 computer_wins += 1
 color_print('You lost that round.', 1.0, 0.0, 0.0)
 if rounds == 5:
 if player_wins > computer_wins:
 color_print('YOU WON THE GAME!', 0.0, 0.0, 1.0)
 else:
 color_print('YOU LOST THE GAME', 1.0, 0.0, 0.0)
 rounds = 0
 player_wins = 0
 computer_wins = 0
if __name__ == '__main__':
 main()

• David Harkness: Do you really need to cast those constants to strings? Doesn't adding to a string do that already? Alteratively, you could use string interpolation: print '... from %s to %s' % (MINIMUM, MAXIMUM)
• asteri: @DavidHarkness Pretty sure it doesn't automatically cast them to strings. I have Python 3 installed and it doesn't cast implicitly under that. Don't have Python 2.7 to test on.
• David Harkness: Bummer. The string interpolation should, though. The %s should be %d to handle integers. It's been some years since I last wrote Python.
• Veedrac: Normally people would write print '... from', MINIMUM, 'to', MAXIMUM for this if they wanted to avoid % or .format.

Answer 2

Continuing from where Jeff Gohlke left off:

def perform_guessing(number):
 player_guess = input('\nPlayer: ')

You're on Python 2, so this should be int(raw_input(...))

 computer_guess = random.randint(MINIMUM, MAXIMUM)
 print 'Computer: ' + str(computer_guess)
 print 'Number: ' + str(number)

These can be formatted like so:

 print 'Computer:', computer_guess
 print 'Number:', number

Then

 player_score = math.fabs(player_guess - number)
 computer_score = math.fabs(computer_guess - number)

There's no reason to use math.fabs over abs here.

 player_score = abs(player_guess - number)
 computer_score = abs(computer_guess - number)

Then, later

 print 'A number will be randomly chosen from ' + str(MINIMUM) + ' to ' + str(MAXIMUM)

Again, print 'A number will be randomly chosen from', MINIMUM, 'to', MAXIMUM.

[...]

Then your loop.

 run_again = True

You never use this... Remove it.

 player_wins = 0
 computer_wins = 0

That's fine

 rounds = 0
 while run_again:
 rounds += 1

You want 5 rounds, so make this a loop:

 for round in range(5):

Note that I removed the plurality because it sounds wrong. Some people would be more worried about shaddowing the built-in round, but I don't agree in this case.

And then just move this block to after the loop:

 if rounds == 5:
 [...]

to just

 if player_wins > computer_wins:
 color_print('YOU WON THE GAME!', 0.0, 0.0, 1.0)
 else:
 color_print('YOU LOST THE GAME', 1.0, 0.0, 0.0)

And this

 rounds = 0
 player_wins = 0
 computer_wins = 0

is pointless, so remove it.

This change allows you to add short-circuit logic:

 if player_wins >= 3 or computer_wins >= 3:
 break

Finally you have:

import random
import console
MINIMUM = 0
MAXIMUM = 1000
FONT = 'Arial-BoldMT'
FONT_SIZE = 15
def perform_guessing(number):
 player_guess = int(raw_input('\nPlayer: '))
 computer_guess = random.randint(MINIMUM, MAXIMUM)
 print 'Computer:', computer_guess
 print 'Number:', number
 player_score = abs(player_guess - number)
 computer_score = abs(computer_guess - number)
 return player_score < computer_score
def color_print(text, r, g, b):
 console.set_color(r, g, b)
 print text
 console.set_color(0.00, 0.00, 0.00)
def main():
 console.set_font(FONT, FONT_SIZE)
 print 'Welcome to the guessing game!'
 print 'A number will be randomly chosen from', MINIMUM, 'to', MAXIMUM
 print 'The player will make a guess, and then the computer will guess.'
 print 'Whoever is closest wins that round!'
 print 'First to 5 wins!'
 player_wins = 0
 computer_wins = 0
 for rounds in range(5):
 if perform_guessing(random.randint(MINIMUM, MAXIMUM)):
 player_wins += 1
 color_print('You won that round!', 0.0, 0.0, 1.0)
 else:
 computer_wins += 1
 color_print('You lost that round.', 1.0, 0.0, 0.0)
 if player_wins >= 3 or computer_wins >= 3:
 break
 if player_wins > computer_wins:
 color_print('YOU WON THE GAME!', 0.0, 0.0, 1.0)
 else:
 color_print('YOU LOST THE GAME', 1.0, 0.0, 0.0)
if __name__ == '__main__':
 main()

Answer 3

1. player_score>=computer_score makes the computer win even when they score the same. A comparison should be > instead of >= (same for the next clause).

2. The game, as coded, may terminate early (before reaching 5 rounds). For me, it contradicts the requirements.

3. User input shall be validated (is it really a number?) somehow.

4. I don't know if you covered functions already. If so, factor out all the copy-pastes into functions.

Answer 4

You're on Python 2, so this should be int(raw_input(...))

Or even

player_guess = None
while player_guess is None:
 try:
 player_guess = int(raw_input(...))
 except ValueError:
 print("Sorry, '%s' cannot be converted to int. Please try again.")

Answer 5

Use Modules

Use def instead of looping in the while loop so you can call it again at a different stage if needed. For example

def main():
 #code here
 return #return value

Use Try/Except

try:
 #code here
except:
 #error code here

to catch any errors and prevent the programme from crashing.

Tweak

A small tweak I would also do is put everything that is printed in brackets like

print("text here")

rather than

print "text here"

1) it makes better reading

2) allows python 3.0 users to use it inside the python shell