3
\$\begingroup\$

I'm not really even sure if this counts as a game. There is so little code, but it works. It is a game where you have to guess the computers number, and it tells you how many times you took to get it right. I kind of want feedback, and not ways to improve the code. I want to know ways to improve my future projects and what to avoid again.

def main():
    # 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  

    def ask_number(question, low, high, step):
        response = None
        while response not in range(low, high):
            response = int(input(question))
        return response
        tries += step


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

    # set the initial values
    the_number = random.randint(1, 100)
    # Create the priming read here
    tries = 0
    guess= 0
    while int(guess) != int(the_number):

        guess = ask_number("Enter your guess:", 1, 100, 1)

        if int(guess) == int(the_number):
            print("Your right on the money!")
        elif int(guess) > int(the_number):
            print("To high!")
        elif int(guess) < int(the_number):
            print("To low!")

        tries += 1
    #Didnt know how to make it reloop to the start...
    print("You guessed it!  The number was", the_number)
    print("And it only took you", tries, "tries!")

    #Program Closing  
    input("Press the enter key to exit.")

main()
\$\endgroup\$
  • 4
    \$\begingroup\$ As a general note, asking so many review questions in quick succession seems a little unwise - why not take some time to absorb the responses to your previous questions and apply them to your other code before seeking review on it? This will, if nothing else, mean you don't keep getting told the same things over and over again. \$\endgroup\$ – jonrsharpe Dec 3 '14 at 17:11
  • 1
    \$\begingroup\$ Totally agree with this! Code reviews can be quite a time consumming activity and it can be quite appreciable if one gets the feeling that its review was taken into account before submitting more code. \$\endgroup\$ – Josay Dec 3 '14 at 17:21
  • \$\begingroup\$ @josay This program was written before the first RPG was even written... This was my first program... I have not written any new programs, but i have been working on the RPG today \$\endgroup\$ – Amateur Programer Dec 3 '14 at 19:15
  • \$\begingroup\$ @jonrsharpe This program was written before the first RPG was even written... This was my first program... I have not written any new programs, but i have been working on the RPG today \$\endgroup\$ – Amateur Programer Dec 3 '14 at 19:15
  • 1
    \$\begingroup\$ So why not have another go at updating it before asking for a review, trying to implement the feedback from your other questions? There's really not much point getting feedback on code that isn't like code you would write now. \$\endgroup\$ – jonrsharpe Dec 3 '14 at 19:18
6
\$\begingroup\$

import random should be at the top of the script.


The call to main should be guarded by if __name__ == "__main__":.


the_number and guess are already int, so the repeated calls to e.g. int(the_number) are redundant.


Docstrings should be formatted properly, as multi-line strings """ """ not comments #. Many of the actual comments are redundant - it's generally obvious what's going on (well done!)


The step argument to ask_number doesn't make much sense, and there's no error handling for non-numeric inputs.


The "magic numbers" 1 and 100 should be factored out.


Use str.format for combining strings with other objects.


Think about your logic more carefully - if guess isn't equal to or greater than the_number it must be less, you can just use else.


100 isn't in range(1, 100).


I would write it as:

import random  

def main(low=1, high=100):
    """Guess My Number

    The computer picks a random number between low and high
    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

    """
    print("Welcome to 'Guess My Number'!")
    print("I'm thinking of a number between {} and {}.".format(low, high))
    print("Try to guess it in as few attempts as possible.")

    the_number = random.randint(low, high)
    tries = 0

    while True:
        guess = ask_number("Enter your guess:", low, high)
        if guess == the_number:
            print("You're right on the money!")
            break
        elif guess > the_number:
            print("Too high!")
        else:
            print("Too low!")
        tries += 1

    print("You guessed it! The number was {}.".format(the_number))
    print("And it only took you {} tries!".format(tries))
    input("Press the enter key to exit.")

def ask_number(question, low, high):
    """Get the user to input a number in the appropriate range."""
    while True:
        try:
            response = int(input(question))
        except ValueError:
            print "Not an integer"
        else:
            if response in range(low, high+1):
                return response
            print "Out of range"

if __name__ == "__main__":
    main()

To play again, you could recursively call main, or have a loop at the end of the script:

if __name__ == "__main__":
    while True:
        main()
        again = input("Play again (y/n)? ").lower()
        if again not in {"y", "yes"}:
            break
\$\endgroup\$
4
\$\begingroup\$

You could declare a constant at the start of your programme:

INTRO_MESSAGE = """Welcome to 'Guess My Number'!
I'm thinking of a number between 1 and 100.
Try to guess it in as few attempts as possible."""

so that you can then

print(INTRO_MESSAGE)

In such a small programme it is not a great deal, but I think that declaring constants at the start of your programme instead of using magic numbers or "magic strings" is a very good habit that will make larger programmes cleaner.


  • You should always follow PEP8 when you are trying to write good code in Python
  • Use triple quoted strings and .format()
  • Check your spelling to -> too.
  • Put a blank line at the end of your while loop.
\$\endgroup\$
3
\$\begingroup\$

jonrsharpe's excellent answer hilights pretty much everything I wanted to answer (and a lot more too). One remaining thing : some_int in range(some_int2, some_int3) is a non-efficient way of checking that some_int >= some_int2 and some_int < some_int3 as it iterates over all values til it finds something that match.

The expression above can be written idiomatically : some_int2 <= some_int < some_int3. Isn't this beautiful ?

\$\endgroup\$
  • \$\begingroup\$ But surely range objects have an efficient __contains__ method? \$\endgroup\$ – Janne Karila Dec 3 '14 at 19:34
  • \$\begingroup\$ @JanneKarila in 2.x range creates a list, so the membership test is O(n). I'm not sure how efficient it is on a 3.x range object, but there's overhead in creating the object on each iteration (which could be factored out to once per call, but it's still hard to beat an inline calculation). \$\endgroup\$ – jonrsharpe Dec 4 '14 at 9:33

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.