OO structure for holding and scoring exam data

Question

I'm writing some code to administer and grade multiple choice exams. I'd like this to be easily reusable, and I don't really want to have to do things like lay out quizzes and score them with a bunch of list and dict indexing for things like answers, prompts, etc.

I've tried to implement some classes to provide a consistent and simple interface for the rest of the code. (There's a long-term goal here of writing my own instructional management system.) The idea is to take an exam formatted as JSON with both standalone questions and questions that come in blocks, and sensibly represent them so that external code can just do easy stuff like pass a list of question-answer pairs to an exam object for grading.

This is a baseline for future functionality which will include programmatically adding/removing questions (and an interface for same), supplying iterable question-answer groupings to administer exams, providing a statistical summary of taker's scores by subject area, saving scores for later analysis etc.

My mind more naturally works in imperative and functional styles, not so much object-oriented, so I'd love some feedback on this implementation from that standpoint. Does this look like a sensible way of setting up the data structure? Is it appropriately pythonic? Am I violating any major OOP principles?

from collections import OrderedDict
class Question(object):
 """A single question with a unique correct answer.
 Attributes:
 block: string or None; string if this question belongs to block of
 questions, where the string is unique identifier for that block.
 qid: string, unique question identifier
 prompt: string, question prompt
 answers: ordered dict of {string, string} where first string is answer
 ID as number/letter and second string is answer text.
 correct: string, representing correct answerID
 explanation: string containing explanation of correct answer.
 subjects: list of strings representing topics (for competency analysis)
 """
 def __init__(self, BLOCK, QID, PROMPT, ANSWERS, CORRECT, EXPLANATION, SUBJECTS):
 self.qid = QID
 self.prompt = PROMPT
 self.answers = ANSWERS
 self.correct = CORRECT
 self.explanation = EXPLANATION
 self.subjects = SUBJECTS
 if not BLOCK:
 self.block = None
 else:
 self.block = BLOCK
 def ask(self):
 return {'block': self.block, 'qid': self.qid, 'prompt': self.prompt, 'answers': self.answers}
 def ans(self):
 return {'block': self.block, 'qid': self.qid, 'prompt': self.prompt, 'answers': self.answers, 'correct': self.correct, 'explanation': self.explanation, "subjects": self.subjects}
 def grade(self, studAns):
 tempans = self.ans()
 if studAns == self.correct:
 tempans['gotRight'] = True
 else:
 tempans['gotRight'] = False
 return tempans
 def getBlock(self):
 return self.block
 def getID(self):
 return self.qid
 def dictRepr(self):
 return {"block": self.block, "qid": self.qid, "prompt": self.prompt, "answers": self.answers, "correctans": self.correct, "explanation": self.explanation, "subjects": self.subjects}
class QBlock(object):
 """A qBlock is a block of questions, where each block has header text (like
 a prompt that applies to all questions) and question objects within it.
 Example: 'for questions 1-5, assume the following is true... '
 Attributes are header text, list of questions, and blockid.
 blockid must match blockid attribute of question that belongs in block.
 """
 def __init__(self, BLOCKID, HEADER):
 self.blockid = BLOCKID
 self.header = HEADER
 self.questions = []
 def ask(self):
 qsToAsk = [aQuestion.ask() for aQuestion in self.questions]
 return {'header': self.header, 'questions': qsToAsk}
 def addQuestion(self, aQuestion):
 if type(aQuestion) is not Question:
 raise TypeError
 self.questions.append(aQuestion)
 def getHeader(self):
 return self.header
 def getID(self):
 return self.blockid
 def getQuestions(self):
 return self.questions
 def dictRepr(self):
 return {'blockid': self.blockid, 'blockheader': self.header}
class NoneBlock(QBlock):
 """Special qBlock for standalone questions (that don't have an assigned
 block in the underlying data file)
 """
 def __init__(self):
 self.blockid = 0
 self.header = None
 self.questions = []
class Exam(object):
 def __init__(self):
 self.blocks = OrderedDict()
 self.questions = OrderedDict()
 self.grades = []
 def assignBlock(self, aQuestion):
 # aQuestion is a question object. dumps q in noneblock if its designated block does not exist.
 if aQuestion.getBlock() and (aQuestion.getBlock() in self.blocks):
 self.blocks[aQuestion.getBlock()].addQuestion(aQuestion)
 else:
 if 0 not in self.blocks:
 self.blocks[0] = NoneBlock()
 self.blocks[0].addQuestion(aQuestion)
 def addBlock(self, aBlock):
 # aBlock is a block object.
 self.blocks[aBlock.getID()] = aBlock
 def addQuestion(self, aQuestion):
 self.questions[aQuestion.getID()] = aQuestion
 self.assignBlock(aQuestion)
 def load(self, jsonfile):
 from json import load as jload
 with open(jsonfile) as thejson:
 qdict = jload(thejson, object_pairs_hook=OrderedDict)
 for biter in qdict['blocks']:
 btemp = QBlock(biter['blockid'], biter['blockheader'])
 self.addBlock(btemp)
 for qiter in qdict['questions']:
 qtemp = Question(qiter['block'], qiter['qid'], qiter['prompt'], qiter['answers'], qiter['correctans'], qiter['explanation'], qiter['subjects'])
 self.addQuestion(qtemp)
 def administer(self):
 return [eachblock.ask() for eachblock in self.blocks.values()]
 def grade(self, answers):
 # answers is a list of (qid, student answer) pairs as tupe of strings
 for answer in answers:
 self.grades.append(self.questions[answer[0]].grade(answer[1]))
 return self.grades

Sample data:

{
 "blocks":
 [
 {
 "blockid": "BLOCK01",
 "blockheader": "The following questions are all about CATS."
 }
 ],
 "questions":
 [
 {
 "block": "BLOCK01",
 "qid": "Q1",
 "prompt": "What noise does the animal make?",
 "answers": {"A": "Meow!", "B": "Woof!"},
 "correctans": "A",
 "explanation": "Have you ever seen a cat?",
 "subjects": ["Cats", "Noises"]
 },
 {
 "block": "",
 "qid": "Q2",
 "prompt": "What is the opposite of up?",
 "answers": {"A": "Left.", "B": "Down."},
 "correctans": "B",
 "explanation": "If you don't know this, you're probably falling as we speak.",
 "subjects": ["Directions", "Life Skills"]
 }
 ]
}

The idea is that the sample data represents an exam, which is saved in a json on disk; the user initializes an exam object and then calls exam.load() on the JSON to get the questions in, at which point the user's external code can display the exam to a taker using exam.administer(), accept the taker's answers and call exam.grade() to score them and do something else with that output, display explanations to the taker, etc.

• Legato: Welcome to Codereview, Paul. I hope you get some fine answers.

Answer 1

OOP

Since you say you're new to OOP, here are a few quick tips:

• When designing classes, think in terms of Abstract Data Types (ADT): an ADT is data + the things you can do on that data, for some well-defined purpose
• Encapsulation, information hiding: what information (data, operations) should the ADT hide? Internal details should be hidden from users.

So far you're doing fine, except for some naming issues. For example looking at Question:

• data: a question has an id, prompt, answers, etc, some of them with getters. So far so good.
• operations: ask, ans, grade, dictRepr

The data of this ADT is fine. The operations sound a bit odd. It's not clear enough what they might do without looking at the implementation, and for example grade does something very different from what I would guess. So there's at least a problem with naming.

On closer look, the methods return different representations of the question data as dictionary objects, with some of the data intentionally hidden. And I suppose ans is only used internally by grade. And the code in ans is duplicated in dictRepr. I suggest the following renames:

• ask -> question_data_as_dict
• dictRepr -> as_dict
• ans -> eliminate, as_dict already does the same job
• grade -> graded_data_as_dict

The idea is that if you look at these methods, you understand better the operations on the ADT.

Coding style

Some general naming issues:

• Variable names should be snake_case, not SHOUTCASE
• Method names should be snake_case, not camelCase

See more tips in the coding style guide.

Naming

Some of the names are quite poor. For example QBlock would be nicer to spell out as QuestionBlock.

In this piece of code:

for biter in qdict['blocks']:
 btemp = QBlock(biter['blockid'], biter['blockheader'])
 self.addBlock(btemp)
for qiter in qdict['questions']:
 qtemp = Question(qiter['block'], qiter['qid'], qiter['prompt'], qiter['answers'], qiter['correctans'],

qiter['explanation'], qiter['subjects']) self.addQuestion(qtemp)

Some of the variables can be easily named better:

• btemp -> block
• qtemp -> question

The Pythonic way

Instead of this:

if not BLOCK:
 self.block = None
else:
 self.block = BLOCK

Probably this is good enough:

self.block = BLOCK

Instead of this:

if studAns == self.correct:
 tempans['gotRight'] = True
else:
 tempans['gotRight'] = False

You can use boolean expressions directly:

tempans['gotRight'] = studAns == self.correct

• CodeYogi: The number of parameters in Question constructor is too long (>=3) which is a code smell.

Answer 2

I have more experience with Python than OOP so I'm going to focus a bit more on what makes your code Pythonic, espeically since janos already answered on that a bit.

Try to separate out the single line summary of your docstring with a blank line, it's more readable and used in some automated parsers.

"""A single question with a unique correct answer.
Attributes:
 block: string or None; string if this question belongs to block of
 questions, where the string is unique identifier for that block.

Using variable names in all caps is usually reserved for constants. You should make these lowercase instead. block wont clash with self.block, but if you insist on different names then don't use caps to delineate.

You could change your if check to a ternary, this is a debatable improvement but I like using them in cases like this:

 # value1 statement value2
 self.block = block if block else None

It evaluates as value1 if statement == True, otherwise it evaluates as value2.

ask and ans are good clean functions. I'd recommend adding brief docstrings clarifying the data they return. And just use the name answer. Shortening for 3 letters is rarely worth the slight clarity loss.

In grade, you could set tempans['getRight'] to just equal the expression you're using to test. It's easier to read because it's clear that the value gets set directly to that expression.

def grade(self, studAns):
 tempans = self.ans()
 tempans['gotRight'] = studAns == self.correct
 return tempans

I'm going to contradict janos here, but Python doesn't care much for getters and setters. It's Pythonic to access attributes directly. So if you want to get the value of some_question's block attribute, just read it with some_question.block. You can do the same with setting it too, just set some_question.block = some_value. If you really want to make a value private and inaccessible outside the class, the common style is add an underscore to the start of the name (ie. _block). That doesn't affect how it works, but signals to another coder that you don't want it accessed externally.

Python operates on the idea that it shouldn't restrict you to using getters and setters, instead letting you do what you want. So you may as well operate within those boundaries because it's complicated to actually enforce any kind of private variable system in Python.

Of course the exception to this is if you ever need to adjust data before returning/assigning it. Or if you need to validate values before setting them to attributes. In these cases it makes perfect sense, but it's entirely unnecessary to use functions for plain getters and setters in Python.

Also with dictRepr, you should name it dict_repr as the Python naming convention is snake_case, not camelCase. That said, I'd name it something other than repr as that seems similar to the common function name __repr__. Use dict_format instead perhaps?