From 7f92606ec95cce9b51e45886e3a4588b83b1e5b6 Mon Sep 17 00:00:00 2001
From: Lyla Fischer <fischerl@mitx.mit.edu>
Date: Thu, 10 May 2012 10:38:06 -0400
Subject: [PATCH] responses to code review

---
 djangoapps/courseware/capa/responsetypes.py   | 32 +++++++++++--------
 .../courseware/test_files/multichoice.xml     | 12 +++----
 djangoapps/courseware/tests.py                |  2 +-
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/djangoapps/courseware/capa/responsetypes.py b/djangoapps/courseware/capa/responsetypes.py
index 9fde05d8d9..ac4c84d8a6 100644
--- a/djangoapps/courseware/capa/responsetypes.py
+++ b/djangoapps/courseware/capa/responsetypes.py
@@ -6,6 +6,7 @@ import random
 import scipy
 import traceback
 import copy
+import abc
 
 from calc import evaluator, UndefinedVariable
 from django.conf import settings
@@ -38,10 +39,17 @@ def compare_with_tolerance(v1, v2, tol):
     return abs(v1-v2) <= tolerance
 
 class GenericResponse(object):
+    __metaclass__=abc.ABCMeta
+
+    @abc.abstractmethod
     def grade(self, student_answers):
         pass
+
+    @abc.abstractmethod
     def get_answers(self):
         pass
+
+    #not an abstract method because plenty of responses will not want to preprocess anything, and we should not require that they override this method.
     def preprocess_response(self):
         pass
 
@@ -57,7 +65,8 @@ class MultipleChoiceResponse(GenericResponse):
 
         self.answer_id = xml.xpath('//*[@id=$id]//choicegroup/@id',
                                    id=xml.get('id'))
-        assert len(self.answer_id) == 1, "should have exactly one choice group per multiplechoicceresponse"
+        if not len(self.answer_id) == 1:
+            raise Exception("should have exactly one choice group per multiplechoicceresponse")
         self.answer_id=self.answer_id[0]
 
     def grade(self, student_answers):
@@ -79,19 +88,14 @@ class TrueFalseResponse(MultipleChoiceResponse):
             response.set("type", "TrueFalse")
 
     def grade(self, student_answers):
-        correct = copy.deepcopy(self.correct_choices)
-        if self.answer_id in student_answers and student_answers[self.answer_id]:
-            for answer in student_answers[self.answer_id]:
-                if answer in correct:
-                    correct.remove(answer)
-                else:
-                    return {self.answer_id:'incorrect'}
-            if len(correct) != 0:
-                return {self.answer_id:'incorrect'}
-            else:
-                return{self.answer_id:'correct'}
-        else:
-            return {self.answer_id:'incorrect'}
+        correct = set(self.correct_choices)
+        answers = set(student_answers.get(self.answer_id, []))
+        
+        if correct == answers:
+            return { self.answer_id : 'correct'}
+        
+        return {self.answer_id : 'incorrect'}
+
 
 class NumericalResponse(GenericResponse):
     def __init__(self, xml, context):
diff --git a/djangoapps/courseware/test_files/multichoice.xml b/djangoapps/courseware/test_files/multichoice.xml
index 3d7265bf5d..60bf02ec59 100644
--- a/djangoapps/courseware/test_files/multichoice.xml
+++ b/djangoapps/courseware/test_files/multichoice.xml
@@ -1,19 +1,19 @@
 <problem>
-<multiplechoiceresponse max="10" randomize="yes">
+<multiplechoiceresponse>
   <choicegroup>
-    <choice location="random" correct="false" name="foil1">
+    <choice correct="false" name="foil1">
       <startouttext />This is foil One.<endouttext />
     </choice>
-    <choice location="random" correct="false" name="foil2">
+    <choice correct="false" name="foil2">
       <startouttext />This is foil Two.<endouttext />
     </choice>
-    <choice location="random" correct="true" name="foil3">
+    <choice correct="true" name="foil3">
       <startouttext />This is foil Three.<endouttext />
     </choice>
-    <choice location="random" correct="false" name="foil4">
+    <choice correct="false" name="foil4">
       <startouttext />This is foil Four.<endouttext />
     </choice>
-    <choice location="random" correct="false" name="foil5">
+    <choice correct="false" name="foil5">
       <startouttext />This is foil Five.<endouttext />
     </choice>
   </choicegroup>
diff --git a/djangoapps/courseware/tests.py b/djangoapps/courseware/tests.py
index c99da7fbda..d038373bfe 100644
--- a/djangoapps/courseware/tests.py
+++ b/djangoapps/courseware/tests.py
@@ -63,7 +63,7 @@ class ModelsTest(unittest.TestCase):
 
 class MultiChoiceTest(unittest.TestCase):
     def test_MC_grade(self):
-        multichoice_file = os.getcwd()+"/djangoapps/courseware/test_files/multichoice.xml"
+        multichoice_file = os.path.dirname(__file__)+"/test_files/multichoice.xml"
         test_lcp = lcp.LoncapaProblem(multichoice_file, '1')
         correct_answers = {'1_2_1':'foil3'}
         self.assertEquals(test_lcp.grade_answers(correct_answers)['1_2_1'], 'correct')