Maths quiz with difficulty levels and a score database

Question

How can I make the saving to the dictionaries more efficient as well as making my overall code more efficient?

import random
import sys
import shelve
def get_input_or_quit(prompt, quit="Q"):
 prompt += " (Press '{}' to exit) : ".format(quit)
 val = input(prompt).strip()
 if val.upper() == quit:
 sys.exit("Goodbye")
 return val
def prompt_bool(prompt):
 while True:
 val = get_input_or_quit(prompt).lower()
 if val == 'yes':
 return True
 elif val == 'no':
 return False
 else:
 print ("Invalid input '{}', please try again".format(val))
def prompt_int_small(prompt='', choices=(1,2)):
 while True:
 val = get_input_or_quit(prompt)
 try:
 val = int(val)
 if choices and val not in choices:
 raise ValueError("{} is not in {}".format(val, choices))
 return val
 except (TypeError, ValueError) as e:
 print(
 "Not a valid number ({}), please try again".format(e)
 )
def prompt_int_big(prompt='', choices=(1,2,3)):
 while True:
 val = get_input_or_quit(prompt)
 try:
 val = int(val)
 if choices and val not in choices:
 raise ValueError("{} is not in {}".format(val, choices))
 return val
 except (TypeError, ValueError) as e:
 print(
 "Not a valid number ({}), please try again".format(e)
 )
role = prompt_int_small("Are you a teacher or student? Press 1 if you are a student or 2 if you are a teacher")
if role == 1:
 score=0
 name=input("What is your name?")
 print ("Alright",name,"welcome to your maths quiz."
 " Remember to round all answers to 5 decimal places.")
 level_of_difficulty = prompt_int_big("What level of difficulty are you working at?\n"
 "Press 1 for low, 2 for intermediate "
 "or 3 for high\n")
 if level_of_difficulty == 3:
 ops = ['+', '-', '*', '/']
 else:
 ops = ['+', '-', '*']
 for question_num in range(1, 11):
 if level_of_difficulty == 1:
 max_number = 10
 else:
 max_number = 20
 number_1 = random.randrange(1, max_number)
 number_2 = random.randrange(1, max_number)
 operation = random.choice(ops)
 maths = round(eval(str(number_1) + operation + str(number_2)),5)
 print('\nQuestion number: {}'.format(question_num))
 print ("The question is",number_1,operation,number_2)
 answer = float(input("What is your answer: "))
 if answer == maths:
 print("Correct")
 score = score + 1
 else:
 print ("Incorrect. The actual answer is",maths)
 if score >5:
 print("Well done you scored",score,"out of 10")
 else:
 print("Unfortunately you only scored",score,"out of 10. Better luck next time")
 filename1 = []
 filename2 = []
 filename3 = []
 class_number = prompt_int_big("Before your score is saved ,are you in class 1, 2 or 3? Press the matching number")
 if class_number == 1:
 with shelve.open(filename1) as db:
 old_scores = db.get(name, [])
 old_scores.extend(str(score))
 db[name] =score[-3:]
 if class_number == 2:
 with shelve.open(filename2) as db:
 old_scores = db.get(name, [])
 old_scores.extend(str(score))
 db[name] =score[-3:]
 if class_number == 3:
 with shelve.open(filename3) as db:
 old_scores = db.get(name, [])
 old_scores.extend(str(score))
 db[name] =score[-3:]
 if prompt_bool("Do you wish to view previous results for your class"):
 for line in lines:
 print (line)
 else:
 sys.exit("Thanks for taking part in the quiz, your teacher should discuss your score with you later")
if role == 2:
 class_number = prompt_int_big("Which class' scores would you like to see? Press 1 for class 1, 2 for class 2 or 3 for class 3")
 filename = (str(class_number) + "txt")
 sort_or_not = int(input("Would youlike to sort these scores in any way? Press 1 if the answer is no or 2 if the answer is yes"))
 if sort_or_not == 1:
 f = open(filename, "r")
 lines = [line for line in f if line.strip()]
 lines.sort()
 for line in lines:
 print (line)
 if sort_or_not == 2:
 type_of_sort = int(input("How would you like to sort these scores? Press 1 for scores in alphabetical order with each student's highest score for the tests, 2 if you would like to see the students' highest scores sorted from highest to lowest and 3 if you like to see these student's average scores sorted from highest to lowest"))
 if type_of_sort == 1:
 with open(filename , 'r') as r:
 for line in sorted(r):
 print(line, end='')
 if type_of_sort == 2:
 file = open(filename, 'r')
 lines = file.read().splitlines()
 s = {lines[i]:[int(k) for k in lines[i+1:i+4]] for i in range(0,len(lines),4)}
 for i in sorted(s.keys()):
 print (i,max(s[i]))
 avg_mark = lambda name:sum(s[name])/len(s[name])
 for i in sorted(s.keys(),key=avg_mark,reverse=True):
 print (i,avg_mark(i))
 if type_of_sort == 3:
 file = open(filename, 'r')
 lines = file.read().splitlines()
 s = {lines[i]:[int(k) for k in lines[i+1:i+4]] for i in range(0,len(lines),4)}
 for i in sorted(s.keys()):
 print (i,max(s[i]))
 high_mark = lambda name:max(s[name])
 for i in sorted(s.keys(),key=high_mark,reverse=True):
 print (i,high_mark(i))

• jacwah: Hi, we all want our code to be more efficient – titles should instead explain what your code does. If you were to edit your question to include a description of your project that would help reviewers a lot!

Answer 1

if val.upper() == quit:

That depends on quit being ALLCAPS. If it isn't, your if statement will never execute. I would suggest if val.upper() == quit.upper() so that whoever uses the function doesn't need to use ALLCAPS.

sys.exit("Goodbye")

This was success, wasn't it? When you pass a string to sys.exit(), it prints the message to stderr and uses 1 (failure) as the exit code. You should print the message separately.

if val == 'yes':

What if the user typed Yes? Your program would say Invalid input ... You should use if val.lower() == 'yes': instead. Your indentation is not right either. PEP 8, the Python style guide, says:

Use 4 spaces per indentation level.

You don't follow that in your if, elif, and else statements.

except (TypeError, ValueError) as e:

You shouldn't be getting a TypeError. You can remove that and make it except ValueError as e:

Your prompt_int_small() and prompt_int_big() functions are identical except for the list of valid choices. Don't duplicate code. Instead, you might use SMALL = (1, 2) and BIG = (1, 2, 3). Then you could use prompt_int(..., SMALL) or prompt_int(..., BIG).

maths = round(eval(str(number_1) + operation + str(number_2)),5)

For one thing, you should use a format string: eval("{0} {1} {2}".format(number_1, operation, number_2)). Even that isn't ideal. Instead of using a list for ops, use a dictionary:

import operator
ops = ['+': operator.add, '-': operator.sub, ...]

Then you can use maths = round(ops[operation](number_1, number_2), 5)

answer = float(input("What is your answer: "))

What if the user typed ninety? You'd have an error. You should do something like this:

while True:
 try:
 answer = float(input("What is your answer: "))
 break
 except ValueError:
 print("Please type a number in decimal form.")

You might also consider rounding the user's input so that if he is more precise than your program, he won't be told that it's the wrong answer.

score = score + 1

It's so common to add something to a variable (or subtract, multiply, etc.) that many languages have shortcuts. Python is one of those languages, so you can use score += 1. I will note, though, that += often uses a method called __iadd__ on mutable objects that actually modifies the object, so you might not want to do that if you have multiple "copies" of the same mutable object that are actually just the same object.

if score >5:

It's better to have a space between > and 5 to improve readbility. From PEP 8:

Always surround these binary operators with a single space on either side: assignment (=), augmented assignment (+=,-= etc), comparisons (==,<,>,!=,<>,<=,>=,in,not in,is,is not), Booleans (and,or,not).

I would recommend that you read that whole PEP.

if class_number == 1:
 with shelve.open(filename1) as db:
 old_scores ...
 ...
if class_number == 2:
 with shelve.open(filename2) as db:
 ...
if class_number == 3:
 ...

It's more duplicate code. I would do it like this:

filename = [filename1, filename2, filename3][class_number - 1]
with shelve.open(filename) as db:
 old_scores = db.get(name, [])
 old_scores.extend(str(score))
 db[name] = score[-3:]

That's much shorter. I'm not sure why you are extending old_scores with the string version of score. Why not old_scores.append(score)? Also, db[name] = score[-3:] doesn't seem right. Maybe I'm reading your code wrong, but I thought that score was an integer. You can't do score[-3:] if it is. I can't say what to replace it with because I'm not sure what you are trying to do.

sys.exit(...)

See above (exiting with a message).

filename = (str(class_number) + "txt")

For one thing, those outer parentheses are useless and don't even improve readability. class_number will be either 1, 2, or 3. That means that filename will be either 1txt, 2txt, or 3txt. Is that really what you want? You might consider something like:

import os
filename = os.path.extsep.join((str(class_number), "txt"))

Since you aren't using os or os.path anywhere else, you might use from os.path import extsep instead of import os. That way the line would be considerably shortened.

sort_or_not = int(input(...))

You are again converting input to a number without validation-checking. You don't even need that in this case, though, because you already defined a function for doing exactly what you are doing here. Use it. On the side, you have a typo. There should be a space between you and like.

f = open(filename, "r")

Always use a with statement when dealing with files. Also, open() by default opens with "r", so it isn't necessary to specify. If you want to specify anyway to make that more obvious, go ahead. I just thought I'd point it out.

lines = [line for line in f if line.strip()]

That's great that you use list comprehensions. A lot of people don't know about them. It's also great that you realize that you can iterate through a file object without using .readlines(). There are also a lot of people who don't know about that.

lines.sort()

Aren't you running this block of code because the user just said he didn't want the scores to be sorted? It's your program, so do what you want; but I would expect that you would want to carry out the wishes of your user.

type_of_sort = int(input("How would you like to sort these scores? Press 1 for scores ...

Again, use the functions you defined. Another thing: That line seems to go one forever. I would split it up a bit. From PEP 8:

Limit all lines to a maximum of 79 characters.

It's a good rule. My terminal is 80 characters wide and I don't use word wrap. Limiting the lines to 79 characters makes it much easier to read. Just take a look at the scrollbar on your browser. That line really shortens the bar, doesn't it?

for i in sorted(s.keys()):

Right now, just go into a Python shell. Create a non-empty dictionary. Iterate through it with something like for item in mydict: and in that loop print item. You will see that it is each key in the dictionary that is printed. That is because the __iter__ method on a dictionary goes through its keys. For the same reason, you can use sorted(s) instead of sorted(s.keys()). I would also suggest for key, value in sorted(s.items()). That way you don't need to use s[i]; just use value. In Python 2, it would be .iteritems() because .items() returns a list and .iteritems() returns an iterator.

avg_mark = lambda name:sum(s[name])/len(s[name])

PEP 8 to the rescue:

Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.

Yes:

def f(x): return 2*x

No:

f = lambda x: 2*x

The first form means that the name of the resulting function object is specifically 'f' instead of the generic '<lambda>'. This is more useful for tracebacks and string representations in general. The use of the assignment statement eliminates the sole benefit a lambda expression can offer over an explicit def statement (i.e. that it can be embedded inside a larger expression)

I think that since you are using avg_mark inside your loop as well as in your sorting, I would change it slightly so that it returns what it does but with an added , name That way, you can do:

for mark, student in sorted(map(avg_mark, s), reverse=True):
 print(student, mark)

That improves performance a little because it calls avg_mark only once. In Python 2, you would use itertools.imap() instead of map() because map() returns a list and itertools.imap() returns an iterator. An iterator is more efficient because you aren't loading the whole thing into memory.

If type_of_sort is either 2 or 3, you have quite a bit of duplicate code. I might do something like:

if type_of_sort == 1:
 ...
else:
 if type_of_sort == 2:
 def get_mark(name):
 return (sum(s[name])/len(s[name]), name)
 else:
 def get_mark(name):
 return (max(s[name]), name)
 with open(filename, 'r') as file:
 lines = file.read().splitlines()
 s = ...
 for i in sorted(s):
 print(i, max(s[i]))
 for mark, student in sorted(s, key=get_mark, reverse=True):
 print(student, mark)

That shortens your code.

Answer 2

To make dictionaries fast do error-proofed indexing:

try:
 return dictionary[index]
except IndexError:
 return -1

Thus, you don't have to search the dictionary to check if the key is there, much faster.

• Sjoerd Job Postmus: dictionaries raise KeyError (subclass of LookupError). Lists raise IndexError, which you probably mean.