13
\$\begingroup\$

I have been using Python 3 to learn programming, and this is a very basic task, but I want to gain good programming patterns/habits at the very beginning.

The program will first randomly generate a number unknown to the user. The user needs to guess what that number is. The program interacts with the user.

import random
randomNumber = random.randrange(0,100)
print("Random number has been generated")
guessed = False
while guessed==False:
    userInput = int(input("Your guess pleas: "))
    if userInput==randomNumber:
        guessed = True
        print("Well done!")
    elif userInput>100:
        print("Our guess range is between 0 and 100, please try a bit lower")
    elif userInput<0:
        print("Our guess range is between 0 and 100, please try a bit higher")
    elif userInput>randomNumber:
        print("Try one more time, a bit lower")
    elif userInput < randomNumber:
        print("Try one more time, a bit higher")

print("End of program")
\$\endgroup\$
  • 9
    \$\begingroup\$ "Your guess pleas: ": you're missing an e in please. \$\endgroup\$ – Mathias Ettinger Jul 1 '16 at 8:38
8
\$\begingroup\$

Follow PEP8

PEP8 specifies that variables should be named like functions, using _underscores_ over mixedCase:

randomNumber -> random_number

userInput -> user_input


Use chained comparisons

Building on Graipher's point, I would negate the condition and take advantage of Python's chained comparison operators:

elif not 0 <= user_input <= 100:
    print("Our guess range is between 0 and 100, please try again")

Keep your code DRY

2 sets of strings in your code are very similar to each other. The conditional branches that bring you there are also similar and compare against the same variables.

In those cases, you can combine the branch conditions and strings together and branch only at the point when the strings differ, minimizing the duplication between strings. str.format() or the % operator along with conditional expressions are your friends here:

elif not 0 <= user_input <= 100:    # Combines 2 branches into 1
    print("Our guess range is between 0 and 100, please try a bit {}."
    #                                                             ^
    # The strings only starts to differ based on the condition here
         .format("higher" if user_input < 0 else "lower"))

Add compatibility checks

Although you were only referring to Python 3 for this exercise, it's a good idea to add compatibility for Python 2 for real applications.

Simply redefining input() is enough in this simple program:

if sys.version_info[0] < 3:
    input = raw_input

For more complicated programs, I suggest you look into six.


Group code into functions

This is helpful even for a small program since it prevents too many variables from polluting the module scope and allows your module to be importable from another program. It's also easier to read and extend later on.

(In the example below, you can easily add a range_min and range_max parameter to start_guess() to easily tune the range of numbers to guess from)


Use is False instead of == False if necessary

You should generally avoid direct comparisons to a Boolean and just use not <variable> as Toastrackenigma said if you know the possible values of the variable.

However, in cases where the variable value can also be an int, which is probably a sign of bad programming practice anyway, it's better to use is False over == False because 0 == False.

(This is not needed in the sample below)


Updated code with mine and everyone's suggestions:

import random
import sys

# Compatibility with Python 2
if sys.version_info[0] < 3:
    input = raw_input


def start_guess():
    random_number = random.randrange(0, 101)
    print("Random number has been generated.")

    while True:
        try:
            user_input = int(input("Your guess please: "))
        except ValueError:
            print("Please enter a number.")
            continue
        if user_input == random_number:
            print("Well done!")
            break
        elif not 0 <= user_input <= 100:    # If it's not between 0 to 100
            print("Our guess range is between 0 and 100, please try a bit {}."
                # User_input at this point must be outside the range
                # so if it's not below the min, it's definitely above the max
                 .format("higher" if user_input < 0 else "lower"))
        else:
            print("Try one more time, a bit {}."
                # User_input at this point must be inside the range
                # so if it's not below the random_number, it's definitely above it
                 .format("higher" if user_input < random_number else "lower"))


if __name__ == '__main__':
    start_guess()
    print("End of program.")
\$\endgroup\$
16
\$\begingroup\$

I would probably combine these two if clauses:

elif userInput>100:
    print("Our guess range is between 0 and 100, please try a bit lower")
elif userInput<0:
    print("Our guess range is between 0 and 100, please try a bit higher")

To:

elif user_input > 100 or user_input < 0:
    print("Our guess range is between 0 and 100, please try again")

Trusting that the user is clever enough to realise that if he entered 1000 (which is still visible in the line just above this output), that he is too high for range(1,101).


Incidentally, there may be a small bug in your program:

random.randrange(1,100)

will never generate 100, just like range(1,100) will only give you the numbers from 1 to 99, inclusive. But you also allow the user to guess 0 or 100.

Use either of these two:

random.randrange(0,101)
random.randint(0,100)

The latter actually does return an integer N, such that a <= N <= b


Your program will crash if the user enters any non-number. Better use something like:

while True:
    try:
        user_input = int(input("Your guess please: "))
    except ValueError:
        print("Please enter a number")
        continue
    if user_input == random_number:
        print("You win")
        break
    elif user_input < 0 or user_input > 100:
        print("Our guess range is between 0 and 100, please try again")
    elif user_input < random_number:
        # Notify user
    elif user_input > random_number:
        # Notify user

Which will ask the user until a valid number is supplied. You can also put all the other checking code in there as well.

\$\endgroup\$
10
\$\begingroup\$

Two comments:

\$\endgroup\$
10
\$\begingroup\$

Always do yourself a favor and use this convention:

if __name__ == '__main__'

convention from the beginning. This prevents confusing behaviour, such as when you want to use a function that's further down in your code. It also helps to structure your code better into functions.

\$\endgroup\$
7
\$\begingroup\$

Basic Advice

  • We don't normally use comparison operators for booleans like you do in

    while guessed==False : 
    

    Instead, we can use

    while not guessed :
    

    (same as the code that you have) or

    while guessed :
    

    (opposite to the code you have - runs while true)

    The reason for this is that doing another comparison is simply an overkill. The statement is going to evaluate to a boolean anyway, so why not use the one that you already have, and invert if necessary? To check if something is true, comparison actually does nothing, and in this example negation (with not) is slightly faster and easier to read than what you have.

  • Either: Put spaces around your all of your operators for readability, or stick to not using spaces around any of your operators - don't mix and match like you do in this program. Do note however that although it is good to be consistent, it is even better to stick with standards and simply put spaces around all of your operators.

Comment

Your program seems to be based on good, solid principles, and has been (for the most part) nicely implemented.

\$\endgroup\$
  • 10
    \$\begingroup\$ It's not a good recommendation to never put spaces around operator. It directly violates the official style guide PEP8 linked from the other answer. \$\endgroup\$ – janos Jul 1 '16 at 8:40
  • \$\begingroup\$ @janos I was trying to say that OP should use consistent practices in their code, but also stick to standards and put spaces around their operators. I can definitely see how I didn't quite convey the latter, so I've added another sentence to clarify that. Thanks for pointing it out :) \$\endgroup\$ – Toastrackenigma Jul 1 '16 at 23:08

protected by Simon Forsberg May 17 '17 at 13:56

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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