3
\$\begingroup\$

What could I improve on this code?

from random import randint
guesses = 0
randomNum = (randint(0,100))
numPlayer = 0
while (numPlayer!=randomNum):
    numPlayer = int(input("Guess the number(0-100)"))
    if (numPlayer>randomNum) : 
        print "It's -"  
    elif (numPlayer<randomNum) : print("It's +")
    guesses=guesses+1

print ("Well done you guessed my number in %d guesses" % guesses)
\$\endgroup\$
  • \$\begingroup\$ You forgot a parenthesis: print "It's -" -> print("It's") copy paste problem probably \$\endgroup\$ – Caridorc Sep 19 '15 at 18:25
  • \$\begingroup\$ broken code: str > int is not possible \$\endgroup\$ – Caridorc Sep 19 '15 at 18:26
  • 1
    \$\begingroup\$ FYI that should be int(input( ... )) \$\endgroup\$ – Ethan Bierlein Sep 19 '15 at 18:29
  • \$\begingroup\$ I tried the code and didn't get any error \$\endgroup\$ – n00b Sep 19 '15 at 18:30
  • \$\begingroup\$ @Alex_n00b because you tried it in Python 2 \$\endgroup\$ – Caridorc Sep 19 '15 at 18:30
6
\$\begingroup\$

I rewrote your code, let me explain my changes line by line:

from random import randint

The same.

from itertools import count

You need to use modules in Python, itertools is incredibly useful and allows to avoid while and use instead the for loop (manual incrementing is terribly un-idiomatic in Python)

def number_guessing(minimum, maximum):

This is a function, you may use it as many times as you want, even in the same program with different arguments. Taking optional parameters for the messages for ultimate customization is left as an exercise for the reader.

randomNum = randint(minimum, maximum)

We use the parameters as literals in the call to randint, not just numbers.

for tries in count(1):

count() just counts, from 1 (the argument) to infinity. tries is just a number that counts the iterations of a loop, but automatically this time.

numPlayer = int(input("Guess the number({}-{})".format(minimum, maximum))

String interpolation, so the user sees a message that contains the appropraite numbers, not always 0 - 100.

    if numPlayer > randomNum:
        print ("My number is smaller"  )
    elif numPlayer < randomNum:
        print ("My number is bigger")

Using spaces around operators, removing unnecessary parentheses, and using messages that seem more understandable to me. (Your homework will make this messages fully customizable, so if you do not like my choices do not worry)

    elif numPlayer == randomNum:
        return tries

If the player guessed correctly our function halts and returns (gives back) how many tries it took for the user to guess the number.

print ("Well done you guessed my number in {} guesses".format(
    number_guessing(0, 50)))

We just call the function we wrote and print its result, that is how many tries it took.

Final code

Just to play around and implement the change I suggested:

from random import randint
from itertools import count

def number_guessing(minimum, maximum):
    randomNum = randint(minimum, maximum)
    for tries in count(1):
        numPlayer = int(input("Guess the number({}-{})".format(minimum, maximum)))
        if numPlayer > randomNum:
            print ("My number is smaller"  )
        elif numPlayer < randomNum:
            print ("My number is bigger")
        elif numPlayer == randomNum:
            return tries

print ("Well done you guessed my number in {} guesses".format(
    number_guessing(0, 2)))
\$\endgroup\$
  • \$\begingroup\$ Guess the number(0-100) is in contradiction with number_guessing(0, 2) \$\endgroup\$ – njzk2 Sep 19 '15 at 21:40
  • \$\begingroup\$ @njzk2 Fixed now; Thanks for spotting it \$\endgroup\$ – Caridorc Sep 19 '15 at 22:13
4
\$\begingroup\$

Style

First off Python has an official style guide, PEP8. Here's a small list of a few of your style violations:

  • Variable names should be in snake_case, and constants should be in UPPER_SNAKE_CASE. Function names should be in snake_case. Class names should be in PascalCase.
  • There is no need for the parentheses around your if or while conditions. This means that this:

    if (numPlayer>randomNum) : 
        print "It's -" 
    

    Could be re-written as this:

    if num_player > random_num:
        print("It's smaller.")
    
  • Your program also suffers from a lack of whitespace. There should be whitespace between binary, mathematical, comparison, boolean, and assignment operators. By doing this you add additional clarity to your code, and make it easier for the reader to read it.
  • Additionally, you should always put code blocks in if, while, etc, statements, one a separate line, like this:

    if blah:
        foobar()
    

    Not like this:

    if blah: foobar()
    

Input validation

You should never trust user input:

numPlayer = int(input("Guess the number(0-100)"))

What do you think would happen if the user entered something like blaghfooaspameggs? This would happen:

Traceback (most recent call last):
  File "...", line ..., in <module>
ValueError: invalid literal for int() with base 10: 'blaghfooaspameggs'

This is not a desired behavior, so we should fix this. In order to ensure that the user enters the right input, we need to validate it using a try/except clause, like this:

try:
    numPlayer = int(input("Guess the number(0-100)"))
    ...
except ValueError:
    print("Invalid integer input!")
    ...

Essentially what this does is "catch"/"ignore" the ValueError when the user enters invalid input, and jumps to the except ValueError block.


Nitpicks

  • This:

    guesses=guesses+1
    

    Can be simplified to an expression using the assignment operator +=, like this:

    guess += 1
    
  • The % operator has been deprecated since Python 2.6, and str.format should be used instead. This means that this line:

    print ("Well done you guessed my number in %d guesses" % guesses)
    

    Would become this:

    print("Well done you guessed my number in {} guesses".format(guesses))
    

    There's a whole lot you can do with str.format, but I'll leave it up to you to research and find out about it.

  • "It's -" and "It's +" aren't very useful messages. Clearer versions of these messages might be "The number is bigger.", or "The number is smaller.".

\$\endgroup\$
  • \$\begingroup\$ You are much faster typing than me :O \$\endgroup\$ – Caridorc Sep 19 '15 at 18:45
  • 1
    \$\begingroup\$ @Caridorc Lol, well I sort of had it prepared out a little once I knew it. :-D \$\endgroup\$ – Ethan Bierlein Sep 19 '15 at 18:46

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.