I am learning Python, and would like to learn how to make this code simpler and easier to read.

Things I am hoping to learn:

  1. What variables might be appropriate?
  2. How can I make it simpler?
  3. Is there any way to make it more efficient?
  4. What can be put into functions without sacrificing readability?
import random
yn = raw_input("Do you wanna build a mathman? y/n ")
if yn == "n":
    exit()

else:
    while yn =="y":
        x =int( random.choice(range(1,100)))
        y =int( random.choice(range(1,100)))
        a = str(x)
        b = str(y)
        s = 0
        xy = x * y
        print "What is " + a  + " x " + b + "?"
        ua = int(raw_input("What is the answer? "))
        while ua != xy:
            print "You are wrong, try again "
            ua = int(raw_input("Try again- "))
            s = s - 1
        yn = raw_input("You are correct, would you like to go again? y/n ")
        s = s + 1
        t = str(s)
    print "Your score is " + t

First of all, you have the following construct:

if yn == "n":
    exit()
else:
    lots
    of
    code

But if yn equals "n", the exit() will have been executed, and you won't reach any other lines anyway. So you can just write

if yn == "n":
    exit()

lots
of
code

This is like an 'early return' (except not in a function, but in the main body of code).

Second, you have

x =int( random.choice(range(1,100)))
...
a = str(x)
...
print "What is " + a + " x " + b + "?"

The spacing on the line with x = is off. It should be

x = int(random.choice(range(1, 100))

That is: no spaces next to ( and ). A space on each side of the =, and a space after the ,.

Furthermore, you should really use string formatting, allowing you to write

print "What is {} + {}?".format(x, y)

This way you don't need intermediate variables a and b.

I assume ua means user_answer here:

print "What is " + a  + " x " + b + "?"
ua = int(raw_input("What is the answer? "))

Why not just write

problem = "What is {} x {}? ".format(x, y)
user_answer = int(raw_input(problem))

You're basically asking a question, and then "What is the answer to the question?". Better to just ask the question alone.

Instead of writing xy, you could write solution or answer.

Another thing of note: the score (I assume that's what you mean with s) gets reset to 0 each time you get a new problem statement, but the score (t) is only printed at the end of the rounds.

That is, you have (pseudo-code)

while play_again:
    ...
    s = 0
    answered = ...
    while answered != solution:
        answered = ...
        s -= 1  # You had s = s - 1, that is equivalent to s -= 1
    s += 1
    t = str(s)
print "Your score is " + t

While instead I think you mean

s = 0
while play_again:
    ...
    answered = ...
    while answered != solution:
        answered = ...
        s -= 1  # You had s = s - 1, that is equivalent to s -= 1
    s += 1
t = str(s)
print "Your score is " + t

Note that the last two lines can be replaced with

print "Your score is {}".format(s)

(Also note: if the user inputs something different from y or n on the first go, you get a NameError: t is referenced before it is assigned because you never enter the while loop).

  • 1
    (By the way: you should also use functions, but I decided to not include that in my suggestions as you are still learning and I did not want to overwhelm you just yet.) – Sjoerd Job Postmus Jan 25 '16 at 21:30
yn = raw_input("Do you wanna build a mathman? y/n ")
if yn == "n":
    exit()

else:

There's a lot of code in your else while the else isn't actually necessary. If the if gets triggered the program will terminate anyway, so all following code is an implicit else.

Note that you ask a y/n question while not checking for a y/n answer. You're also not checking for no, NO and more of such variations which one may expect to be valid.

One way to solve this is by repeatedly asking the user for an answer until you get one you're willing to handle. One way of doing this would be:

valid_input = { 'yes', 'y', 'no', 'n' }
while True:
    user_input = raw_input("Do you want to build a Math Man? y/n ")
    if user_input and user_input.lower() not in valid_input:
        print('Invalid input')
        continue
    break

This forces your user_input to lower-case and checks whether the user wants to continue or not. This will also counter empty input. You can also easily expand on this.

You're currently using the following variable names:

yn, x, y, a, b, x, xy, ua, s, t

Please don't! That's a maintainability nightmare! Try to put more thought in your variable naming.

You wanted to know how functions could help you, and seemed worried about them making code less readable. But in reality, functions make your code more readable as long as you name them well, consider this:

ua = int(raw_input("What is the answer? "))

The line isn't really the clearest. What's ua, what does raw_input do and why is it being inted? People who've used Python will know the basic functions and follow this line, but that doesn't mean it can't be improved. You could use a function to get this value, for example:

def get_guess(message):
    return int(raw_input(message))

Let's explain what's happening in this function. raw_input is being called, with the string message, which is whatever string you want to print to the user. Then as before its result is being turned to an integer and then returned. Returning a value is basically how you'll assign the result of the user's input to a variable. This is how you'd use it in practice:

guess = get_guess("What is the answer? ")

Now notice that you have a simple single function, named for exactly what it does in this context.

The other benefit is that if there's anything else you'd want to do with getting the users input then you can now add it without having to repeat it. In this case, you don't have anything to handle what happens if the user enters an empty string, "twelve" or "banana". In Python, how you'd normally resolve this is by catching the error that occurs when you try to convert an int. You basically catch the exception it causes, which is a ValueError. This is how it would look:

def get_guess(message):
    try:
        return int(raw_input(message))
    except ValueError:
        print("That's not a valid number.")

All this does, is return an integer based on the user input if it can. If the string isn't a valid integer, the error is caught and instead your error warning from the except block is printed. Though that also means that you wont get a value returned, so it's safest to also wrap all this in an infinite loop:

def get_guess(message):
    while True:
        try:
            return int(raw_input(message))
        except ValueError:
            print("That's not a valid number.")

This will loop over and over until a value is successfully returned and then the function exits successfully. You still just need to call it with get_guess, but now it's a much more robust and reliable piece of code.

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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