7
\$\begingroup\$

I started learning Python just yesterday, and so I am aware of my lack of knowledge regarding the language. Below is a program I wrote to simulate a guessing game. I ended up creating a lot of ad hoc patches for some possible errors, and suspect that there are more elegant ways to write the program. I'd like pointers/assistance/an example of a "best practices" version of the program I write below:

import random, sys

sys.setrecursionlimit(10000)    #I expect any troll who's trying to fuck with the program to give up before they reach 10,000.
control = True    #global variable to control the behaviour of the 'guessNumber()' function.
errors = 0    #To store the number of errors a user triggers.

def guessNumber(maxx):
    global errors
    global control
    try:
        maxx = float(maxx)    #Incase a cheeky user decided to submit floating point numbers.
        if int(maxx) != maxx:     #If they submitted a floating point number instead of an integer.
            print("Input whole numbers only.")
            errors += 1
            guessNumber(input('Input the Maximum number: \t'))  #Call a new instance of 'guessNumber()'
        else:   #If they indeed submitted a whole number.
            maxx = int(maxx)
    except ValueError:  #They didn't submit a whole number.
        print("Input whole numbers only.")
        errors += 1
        guessNumber(input('Input the Maximum number: \t'))  #Call a new instance of 'guessNumber()'

    if not control:    #control variable. If the program has already properly evaluated, it should terminate.
        return None

    print('I am thinking of a whole number between 1 and ' + str(maxx) + ' (both inclusive).')        
    num = random.randint(1, maxx)
    count = 0
    check = True
    var = None

    while check:
        print("Guess the number.")
        try:
            var = float(input())    #Incase a cheeky user decided to submit floating point numbers.
            if int(var) != var:     #If they submitted a floating point number instead of an integer.
                print("Input whole numbers only.")
                errors += 1
                continue
            else:   #If they indeed submitted a whole number.
                var = int(var)
        except ValueError:  #They didn't submit a whole number.
            print("Input whole numbers only.")
            errors += 1
            continue
        count +=  1
        if var == num:  #If they guess the number right.
            check = False
        elif var > num:
            print('Too high! \nTry a little lower.')
        elif var < num:
            print('Too low! \nTry a little higher.')

    print('Congratulations!!!\nYou guessed the number after ' + str(count) + " tries and triggering " + str(errors) + " errors.")
    control = False     #After every completed execution, we set control to false so that if the function was called from another instance of itself it terminates and doesn't cause errors.
    return None

guessNumber(input('Input the Maximum number (integers only): \t'))

I am particularly worried about the memory cost, due to using recursion to deal with errors.

\$\endgroup\$
  • \$\begingroup\$ My try catch if else is complicated, but it works. In your case, if a user inputs a floating point number instead of an integer, it wouldn't treat it as an error, and I want users to only input integers. If you can create better code that achieves this functionality, then I'm all ears. \$\endgroup\$ – Tobi Alafin Jun 30 '17 at 4:58
12
\$\begingroup\$
  1. Your comments describe what the code is doing, not why the code's doing what it is. And so are generally unhelpful.
  2. Please don't use profanity in comments, or anywhere. It makes you look unprofessional, and is not really that helpful.
  3. To hell with sys.setrecursionlimit. It just delays the inevitable, rather than fixes your problem.
  4. Rather than using global variables, you could instead use a closure.
  5. Your recursion should be in a separate function. That only gets user input. But it'd be better if it were a while loop.

    For example you could use:

    def get_user_input(fn, prompt='', exception_message=''):
        while True:
            try:
                return fn(input(prompt))
            except ValueError:
                print(exception_message)
    
  6. Don't repeat yourself. You write get_user_input out twice. In different ways.

  7. The while loop for your higher or lower game seems pretty good. Nice job on that.

And so ignoring your error part I'd use:

import random


def get_user_input(fn, prompt='', exception_message=''):
    while True:
        try:
            return fn(input(prompt))
        except ValueError:
            print(exception_message)


def main():
    limit = get_user_input(int, prompt='Input the Maximum number: \t',
                           exception_message='Input whole numbers only.')


    print('I am thinking of a whole number between 1 and {limit} (both inclusive).'.format(limit=limit))
    number = random.randint(1, limit)
    count = 0
    while True:
        print("Guess the number.")
        guess = get_user_input(int, exception_message='Input whole numbers only.')
        count += 1
        if guess == number:
            break

        if guess > number:
            print('Too high!\nTry a little lower.')
        else guess < number:
            print('Too low!\nTry a little higher.')

    print('Congratulations!!!\nYou guessed the number after {count} tries.'.format(count=count))

if __name__ == '__main__':
    main()

If errors is super important to you, you could return the amount of errors from get_user_input and add it to a local errors variable. Say:

errors = 0
while True:
    limit, input_errors = get_user_input(...)
    errors += input_errors
\$\endgroup\$
  • \$\begingroup\$ in your code, you have number = get_user_input ... and then if var == num... In that case, what is var? Did you mean var = get_user_input? \$\endgroup\$ – martin jakubik Jun 30 '17 at 7:54
  • \$\begingroup\$ @martinjakubik Thank you for catching that, I was changing from var to a better variable name. Seems I got distracted half way through. I've now updated the code, :) \$\endgroup\$ – Peilonrayz Jun 30 '17 at 8:30
  • \$\begingroup\$ :) I thought 3% chance it's some python magic (wouldn't be the 1st time). This site needs jenkins voters! \$\endgroup\$ – martin jakubik Jun 30 '17 at 11:42
5
\$\begingroup\$

The Python Styleguide

  • Limit all lines to 79 characters. Limit comments and docstrings to 72 characters.

  • Comments should preferably start with a single space: # Like this, which significantly improves readability;

  • Comments may document code above them, which means putting them on a seperate line. This way, you can avoid exceeding the length limit;

  • Python naming conventions for variables and functions are snake_case, not camelCase;

  • Python naming conventions for constants are UPPERCASE_WITH_UNDERSCORES.

Naming

You might want to use better (that is, self-explanatory) variable / function names. Names such as maxx are bad. If you're trying to avoid shadowing the built-in max, it is preferable to add a trailing underscore: max_. Names like var are too generic.

Simplifying code

Some parts of your code are rather fragile and inefficient. Let's take a look:

while check:
        print("Guess the number.")
        try:
            var = float(input())    #Incase a cheeky user decided to submit floating 
point numbers.
            if int(var) != var:     #If they submitted a floating point number instead 
of an integer.
                print("Input whole numbers only.")
                errors += 1
                continue
            else:   #If they indeed submitted a whole number.
                var = int(var)

I'll go over the problems one by one.

  • You are casting input to float directly. If the user input contains letters, the program will crash.

  • input without a prompt doesn't signify to the user that they are supposed to input something, which may lead to confusion, as the users are left waiting for something to happen.

  • In general, this piece of code is misleading, because a ValueError doesn't necessarily mean input can safely be converted to int. So you try emulating a try / except using if / else.

Improved code:

while check:
    print("Guess the number.")
    guess = input("Your guess: ")
    try:
        guess = int(guess)
    except ValueError:
        print("Input numbers only.")
        # This is less complicated than try - if -> | else -> | except
        errors += 1
        continue

General advice

  • Only use global where absolutely needed. In this case, avoid using it altogether. Excess use of global variables can lead to spaghetti code.
\$\endgroup\$
  • 1
    \$\begingroup\$ I think you're mixing "Constants" with "Global Variable Names" :) \$\endgroup\$ – Peilonrayz Jun 29 '17 at 15:45
  • \$\begingroup\$ BTW OP doesn't have any constants, :) The global variables are variables, rather than constants, :) \$\endgroup\$ – Peilonrayz Jun 29 '17 at 15:54
  • 2
    \$\begingroup\$ If they were constants, they wouldn't need global control, you can read globals without that, but you need it to update the variable. They are not constants, errors += 1 and control = False, change the value. \$\endgroup\$ – Peilonrayz Jun 29 '17 at 16:02
  • \$\begingroup\$ How To Automate The Boring Stuff With Python (the book I'm reading defended the use of camelCase (PEP said we shouldn't adhere strictly to formatting guidelines where not necessary), and since I use it in my other programs, I decided to be consistent. I'll look at the other formatting errors. \$\endgroup\$ – Tobi Alafin Jun 30 '17 at 4:56
  • \$\begingroup\$ My try catch if else is complicated, but it works. In your case, if a user inputs a floating point number instead of an integer, it wouldn't treat it as an error, and I want users to only input integers. If you can create better code that achieves this functionality, then I'm all ears. \$\endgroup\$ – Tobi Alafin Jun 30 '17 at 4:59

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.