5
\$\begingroup\$

Are there any ways I can clean this code up to make it more user-friendly?

import random #Random Function
import os

print("What is your name?") #Asking User Name
name = input().title() #.title() capitilises the first letter in each word, so when the name is printed in later code the first letter will always be capitilised.
class_name = input("Which class do you wish to input results for? ")
print (name, ", Welcome to the OCR Controlled Assessment Maths Test") 
score = 0  #Set Score to 0
question = 0 #Set Question Number to 0
while question < 10: #After 10 Questions, it will stop asking the user questions.
    operator = random.randint(1,3)#Addition,Multiplication,Subtraction
    num1 = random.randint(1,25)#Random Number between 1 and 10 to be used in the questions
    num2 = random.randint(1,25)#Random Number between 1 and 10 to be used in the questions
    if operator == 1: #If Random Operator = 1, it will ask addition questions
        print("What is", num1, "+", num2)
        ans = num1 + num2

    elif operator == 2:#If Random Operator = 2, it will ask multiplication questions
        print("What is", num1, "*", num2)
        ans = num1 * num2

    else: #If Random Operator = 3/Else, it will ask subtraction questions.
        print("What is", num1, "-", num2)
        ans = num1 - num2

    while True:
      try:
        user_ans = int(input()) #The Answer the user inputs to the question asked
      except ValueError: #When anything but a interger is inputted.
        print ("That is not a valid answer")
        continue #Dosen't print Invalid Code Error
      else:
        break

    if user_ans == ans: #If user answer matches that of the question
        print("Correct")
        score += 1

    else: #If user answer dosen't match that of a question.
        print("Incorrect")



    question += 1 #When a question is answered, it will +1 until it equals 10 at which point it will stop.

print("Well done", name, "you scored", score, "/10") #Print Final Score out of 10


class_name = class_name + ".txt"    #adds '.txt' to the end of the file so it can be used to create a file under the name a user specifies

file = open(class_name , 'a')   #opens the file in 'append' mode so you don't delete all the information
name = (name)
file.write(str(name + " : " )) #writes the information to the file
file.write(str(score))
file.write('\n')
file.close()    #safely closes the file to save the information

viewscore = input("Do you wish to view previous results for your class").lower()

if viewscore == "yes".lower():
    f = open(class_name, 'r')
    file_contents = f.read()
    print (file_contents)
    f.close()

elif viewscore != "yes".lower():
    print ("Press any key to exit")
    ex = input ("")
    os._exit(0)
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review. Your question is on the balance of being off-topic here. You can't ask add additional functions since we review existing code or implementing a new aspect in the code (the sort by alphanumeric). I would suggest you to clean your question of those elements. I would suggest you to visit the help center to better know how to ask a question here. \$\endgroup\$ – Marc-Andre Feb 2 '15 at 14:48
  • \$\begingroup\$ Who is the "user" you want it to be friendlier for? The person taking the quiz, the person administering the quiz, other developers using the code...? \$\endgroup\$ – jonrsharpe Feb 2 '15 at 17:43
9
\$\begingroup\$

A few advices :

  • Python has a code style called PEP8. It is a worth reading. Also, you'll find various tools to check that your code is PEP8-compliant like pep8. You'll also find various other tools to check the quality of your code : pyflakes, pychecker, pylint, etc.

  • avoid useless comments. It can be tempting to comment every line you write as you start programming. However, what you are doing adds more noise that it actually helps. As a rule of thumb, "Code Tells You How, Comments Tell You Why". Also, comments are easy to get out of sync with the code (you have places where your code says "25" and the comment says "10", how confusing).

  • you can add an argument to input so that you don't have to call print before.

  • your while question < 10 loop looks like it could/should be a for loop : for question in range(10): (and no need for an initialisation nor for a incrementation in the loop). Also, as a side-note, the convention in Python is to use _ for throw-away values, thus, the loop could be written : for _ in range(10):.

  • you should try to use the with keyword whenever you handle files. If you do so, you don't have to call close explicitly.

  • you should try to ask yourself if your variable describe how you use it. For instance, class_name is a proper name for what you do at the beginning. However, it doesn't sound like a proper filename. You could simple create a new variable : filename = class_name + ".txt"

  • avoid useless tests : in elif viewscore != "yes".lower(), the condition will always be true. You could simply write else.

  • calling lower() on "yes" doesn't seem really interesting does it ?

Now, for more "complicated" advices.

You can try to split the logic in small reusable units like functions (or classes). For instance, you could write :

def get_int_input():
    while True:
      try:
        return int(input())
      except ValueError:
        print ("That is not a valid answer")

Instead of having some code to handle the different possible operations, you could have a structure containing the relevant information : the symbol to be printed and the corresponding operation. You could use a list of tuple to store everything and then, it's easy to pick one at random and use it :

import operator

OPERATIONS = [
    (operator.add, "+"),
    (operator.mul, "*"),
    (operator.sub, "-")
    ]

for _ in range(10):
    num1 = random.randint(1,25)
    num2 = random.randint(1,25)
    op, symbol = random.choice(OPERATIONS)
    print("What is", num1, symbol, num2)
    if get_int_input() == op(num1, num2):
        print("Correct")
        score += 1
    else:
        print("Incorrect")

If you ever need to add one, it's quite easy.

It is also a good habit to use a if __name__ == '__main__': guard.

Similarly to what we've done to get an integer input, we can define a function to get a boolean input.

The final code looks like :

import random
import operator

OPERATIONS = [
    (operator.add, "+"),
    (operator.mul, "*"),
    (operator.sub, "-")
    ]

NB_QUESTIONS = 10

def get_int_input(prompt=''):
    while True:
      try:
        return int(input(prompt))
      except ValueError:
        print("Not a valid input (integer is expected)")

def get_bool_input(prompt=''):
    while True:
        val = input(prompt).lower()
        if val == 'yes':
            return True
        elif val == 'no':
            return False
        else:
            print("Not a valid input (yes/no is expected)")

if __name__ == '__main__':
    name = input("What is your name?").title()
    class_name = input("Which class do you wish to input results for? ")
    print(name, ", Welcome to the OCR Controlled Assessment Maths Test")

    score = 0
    for _ in range(NB_QUESTIONS):
        num1 = random.randint(1,25)
        num2 = random.randint(1,25)
        op, symbol = random.choice(OPERATIONS)
        print("What is", num1, symbol, num2)
        if get_int_input() == op(num1, num2):
            print("Correct")
            score += 1
        else:
            print("Incorrect")

    print("Well done", name, "you scored", score, "/", NB_QUESTIONS)

    filename = class_name + ".txt"

    with open(filename, 'a') as f:
        f.write(str(name) + " : " + str(score) + '\n')

    if get_bool_input("Do you wish to view previous results for your class"):
        with open(filename, 'r') as f:
            print(f.read())
    else:
        input ("Press any key to exit")
\$\endgroup\$
2
\$\begingroup\$

You're a bit inconsistent with the whitespace in your output.

For example, when asking for the name, you print the prompt on its own line:

print("What is your name?")
name = input().title()

But then the following question leaves the cursor on the same line after a trailing space:

class_name = input("Which class do you wish to input results for? ")

Then, in the quiz itself, you go back to printing the prompt on its own line:

else: #If Random Operator = 3/Else, it will ask subtraction questions.
    print("What is", num1, "-", num2)
    ans = num1 - num2

while True:
  try:
    user_ans = int(input()) 

Using print() with multiple arguments like this will result in a superfluous space before the comma:

print (name, ", Welcome to the OCR Controlled Assessment Maths Test")

You've used string concatenation in your program, but the code would be more readable using str.format().


@Josay has already given you good advice. I'd take it a step further and define a quiz() function:

def quiz(num_questions):
    …
    return score

if __name__ == '__main__':
    name = input("What is your name? ").title()
    class_name = input("Which class do you wish to input results for? ")
    print('{}, Welcome to the OCR Controlled Assessment Maths Test'.format(name))

    score = quiz(10)

    filename = class_name + ".txt"
    with open(filename, 'a+') as f:
        f.write("{} : {}\n".format(name, score))
        if get_bool_input("Do you wish to view previous results for your class? "):
            f.seek(0)
            print(f.read(), end='')
        else:
            input("Press any key to exit")
\$\endgroup\$
  • \$\begingroup\$ the last section of code poster by 200_success doesnt work can you tell me why ? What is your name? cameron Which class do you wish to input results for? a Cameron, Welcome to the OCR Controlled Assessment Maths Test Traceback (most recent call last): File "C:/Python34/test 3.py", line 9, in <module> score = quiz(10) File "C:/Python34/test 3.py", line 2, in quiz return score NameError: name 'score' is not defined. \$\endgroup\$ – user71408 Apr 27 '15 at 23:05
  • \$\begingroup\$ @CameronWilkinson The is a placeholder for the bulk of the quiz code, to be filled in with the loop that asks num_questions questions, accepts the answers, and calculates the score. \$\endgroup\$ – 200_success Apr 27 '15 at 23:32

protected by Community May 22 '15 at 10:00

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.