5
\$\begingroup\$

I have been programming a maths quiz that can be used for teachers and I have have been trying to make the code as short as possible so it is easier to understand.

If there is any way I could make it more concise, please tell me and explain the programming behind it.

import sys import random def get_bool_input(prompt=''): while True: val = input(prompt).lower() if val == 'yes': return True elif val == 'no': return False else: sys.exit("Not a valid input (yes/no is expected) please try again") status = input("Are you a teacher or student? Press 1 if you are a student or 2 if you are a teacher") if status == "1": score=0 name=input("What is your name?") print ("Alright",name,"welcome to your maths quiz") level_of_difficulty = int(input(("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 not in (1,2,3): sys.exit("That is not a valid level of difficulty, please try again") if level_of_difficulty == 3: ops = ['+', '-', '*', '/'] else: ops = ['+', '-', '*'] for question_num in range(1, 11): if level_of_difficulty == 1: number_1 = random.randrange(1, 10) number_2 = random.randrange(1, 10) else: number_1 = random.randrange(1, 20) number_2 = random.randrange(1, 20) 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") class_number = input("Before your score is saved ,are you in class 1, 2 or 3? Press the matching number") if class_number not in ("1","2","3"): sys.exit("That is not a valid class, unfortunately your score cannot be saved, please try again") else: filename = (class_number + "txt") with open(filename, 'a') as f: f.write("\n" + str(name) + " scored " + str(score) + " on difficulty level " + str(level_of_difficulty)) with open(filename, 'a') as f: f = open(filename, "r") lines = [line for line in f if line.strip()] f.close() lines.sort() if get_bool_input("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 status == "2": class_number = input("Which classes scores would you like to see? Press 1 for class 1, 2 for class 2 or 3 for class 3") if class_number not in (1,2,3): sys.exit("That is not a valid class") filename = (class_number + "txt") with open(filename, 'a') as f: f = open(filename, "r") lines = [line for line in f if line.strip()] f.close() lines.sort() for line in lines: print (line)
\$\endgroup\$
1
  • 3
    \$\begingroup\$ To clarify, do you want the code to be as short as possible, or as readable as possible? Readability and conciseness usually go together, but there are times when being laconic is a detriment to readability. \$\endgroup\$ Commented Jul 24, 2015 at 20:01

2 Answers 2

Reset to default
7
\$\begingroup\$

First thoughts:

The user is prompted to enter the class number after taking the test, and if it's not a valid number the program exits. This means they lose the results of the test. You could ask that question up front.

There's a large gap between the check to see if status is '1' and to see if status is '2'. I'd put the code for the student and teacher in functions, so that the result looks more readable:

def student_questions(): pass def teacher_questions(): pass if status == "1": student_questions() elif status == "2": teacher_questions()

There's a lot of prompting the user for input and then checking to see if it's valid - that could easily be a function. 

def ask_question(question, valid_answers): while ... print ... input ... if input is valid: return input

The only other comment I'd make straight away is that while choosing the operation at random works, it does mean that one student might get all divisions, others get a broad selection, and so on. Perhaps there needs to be a check to make sure that each student gets at least one of every type? Or a non-random selection?

\$\endgroup\$
1
  • \$\begingroup\$ For the last comment the third level that involves division is meant to test the students calculator skills so it doesn't really matter and for the other two levels the questions are fairly basic. \$\endgroup\$ Commented Jul 24, 2015 at 11:14
6
\$\begingroup\$

Adding to what Simon said above,

Unused Loop

No need for the loop in get_bool_input; as it's written, regardless of what the user decides, the function will break out, whether it be by returning a value or with sys.exit().

Unnecessary sys.exits

That said, you have lots of sys.exits--instead of exiting the program altogether, which would force the user to restart everything and retype all the information, you could have a loop like this:

 # Selecting the difficulty level level_of_difficulty = ... while level_of_difficulty not in (1, 2, 3): print("That is not a valid level of difficulty, please try again") level_of_difficulty = ... # try it again ... # Selecting class number class_number = input(...) while class_number not in (1, 2, 3): print("That is not a valid class") class_number = input(...) # try it again

Repetition of Prompting

Echoing what Simon said in the other answer, you'll notice how these are exactly the same snippets of code with the only difference being the values that they can be. You could try writing a function that looks something like this:

def promptSelection(valid_values, selection_type, prompt): result = input(prompt) while result not in valid_values: print("Sorry, that's not a valid {0}".format(selection_type)) result = input(prompt) return result

There is a little bit of repetition because of Python's lack of in-line assignment/return (i.e. You can't while result = input(...):), but it might make it easier.

Concatenation vs. string.format()

The parts where you concatenate a lot of strings you can try this instead for readability. For example:

# Old f.write("\n" + str(name) + " scored " + str(score) + ...) # New f.write("\n{0} scored {1} on difficulty level {2}".format(name, score, level_of_difficulty))

The documentation for this can be found here: https://docs.python.org/release/3.1.3/library/stdtypes.html#str.format

Bug Potential

In the part where you define class_number, it's defined as a string, but you compare to integers in the next line if class_number not in (1, 2, 3)

Single Source of Truth

This one is fairly trivial in this particular situation but for future reference, for the part where you find the random numbers you could do this instead:

# This goes at the top of your code LEVEL_ONE_NUMBER = 10 LEVEL_OTHER_NUMBER = 20 ... # Assigning the numbers for the problem max_range = LEVEL_ONE_NUMBER if level_of_difficulty != 1: max_range = LEVEL_OTHER_NUMBER number_1 = random.randrange(1, max_range) number_2 = random.randrange(1, max_range)

That way you only have to change one value (LEVEL_ONE or LEVEL_OTHER_NUMBER), and you can use them elsewhere in your code should you so desire. This is typically called using single source of truth.

Mutually Exclusive Options

Your number of options are fairly small (e.g. 3 options), so this won't be as big of a deal, but in the future you can use elif statements to signify that things are mutually exclusive. It might not seem particularly helpful in this situation, but they can help with readability and in some cases avoid repetition. They're also helpful if there's a chance that the thing you're checking will change value midway through your code (which it shouldn't, but you never know). For example:

# Using elif if status == "1": ... elif status == "2": ... else: ... # If status neither "1" nor "2"

A scenario that elif is helpful might be good if you want to split up a value by score:

score = someNumber letterGrade = "" if score < 60: letterGrade = "F" elif score < 70: # No need to write "score >= 60" because it's implied that it's >= 60 by the "else if" letterGrade = "D" elif score < 80: # No need to write "score >= 70" because it's elif letterGrade = "C" elif score < 90: # No need for "score >= 80" because it's elif letterGrade = "B" else: letterGrade = "A"

General Notes:

  • Try putting the code that's floating (starting at status = ...) and putting it into a function. Even though this is a fairly small program, it's good to compartmentalize things so if you decide to expand on this to say, be more than just a maths quiz, you don't have to rewrite everything.
  • Comments around parts of your code that serve a specific purpose can help with clarity. For example, you could do something like # Selecting name when you prompt for a name, # Choosing numbers for problem when you assign number_1 and number_2, and other similar situations.
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.