comments and refactoring

common/lib/capa/capa/graders/draganddrop.py

@@ -64,9 +64,8 @@ class PositionsCompare(list):
         elif (isinstance(self[0], (unicode, str)) and
               isinstance(other[0], (unicode, str))):
             return ''.join(self) == ''.join(other)
-        else:  # improper argument types
-            # Now we have no (float / int or lists of list and float / int pair)
-            # or two string / unicode lists pair
+        else:  # improper argument types: no (float / int or lists of list
+            #and float / int pair) or two string / unicode lists pair
             return False
 
     def __ne__(self, other):
@@ -103,18 +102,14 @@ class PositionsCompare(list):
 
 
 class DragAndDrop(object):
-    """ Grader for drag and drop inputtype.
+    """ Grader class for drag and drop inputtype.
     """
-
     def __init__(self):
         self.correct_groups = OrderedDict()  # correct groups from xml
         self.correct_positions = OrderedDict()  # correct positions for comparing
         self.user_groups = OrderedDict()  # will be populated from user answer
         self.user_positions = OrderedDict()  # will be populated from user answer
 
-        # flag to check if user answer has more draggables than correct answer
-        self.incorrect = False
-
     def grade(self):
         ''' Grader user answer.
 
@@ -126,39 +121,30 @@ class DragAndDrop(object):
 
         Returns: bool.
         '''
+        for draggable in self.excess_draggables:
+            if not self.excess_draggables[draggable]:
+                return False  # user answer has more draggables than correct answer
 
-        if self.incorrect:  # user answer has more draggables than correct answer
-            return False
-
-        # checks if we have same groups of draggables
-        if sorted(self.correct_groups.keys()) != sorted(self.user_groups.keys()):
-            return False
-
-        # checks if for every groups draggables names are same
+        # Number of draggables in user_groups may be smaller that in
+        # correct_groups, that is incorrect.
         for groupname, draggable_ids in self.correct_groups.items():
             if sorted(draggable_ids) != sorted(self.user_groups[groupname]):
                 return False
 
-        # from now self.correct_groups and self.user_groups are equal if
-        # order is ignored
-
-        # Next check in every group that user positions of every element are equal
-        # with correct positions for every rule
-
-        passed_rules = dict()
-        for rule in ('exact', 'anyof'):
-            passed_rules[rule] = 0
-            for groupname in self.correct_groups:
+        # Check that in every group, for rule of that group, user positions of
+        #every element are equal with correct positions
+        for groupname in self.correct_groups:
+            rules_executed = 0
+            for rule in ('exact', 'anyof'):   # every group has only one rule
                 if self.correct_positions[groupname].get(rule, []):
+                    rules_executed += 1
                     if not self.compare_positions(
                             self.correct_positions[groupname][rule],
                             self.user_positions[groupname]['user'], flag=rule):
                         return False
-                    passed_rules[rule] += 1
-
-        # if no rule was executed for all groups
-        if sum(passed_rules.values()) == 0:
-            return False
+            if not rules_executed:  # no correct rules for current group
+            # probably xml content mistake - wrong rules names
+                return False
 
         return True
 
@@ -199,7 +185,7 @@ class DragAndDrop(object):
                             '5': 't2',
                             '7':'t2'}
 
-            It is draggable_name: dragable_position mapping
+            It is draggable_name: dragable_position mapping.
 
         Correct answer in list form is designed for complex cases::
 
@@ -215,6 +201,7 @@ class DragAndDrop(object):
         'rule': 'anyof'
         }
                         ]
+
         Correct answer in list form is list of dicts, and every dict must have
         3 keys: 'draggables', 'targets' and 'rule'. 'Draggables' value is
         list of draggables ids, 'targes' values are list of targets ids, 'rule'
@@ -241,7 +228,7 @@ class DragAndDrop(object):
         self.use_targets = user_answer.get('use_targets')
 
         # check if we have draggables that are not in correct answer:
-        check_extra_draggables = {}
+        self.excess_draggables = {}
 
         # create identical data structures from user answer and correct answer
         for i in xrange(0, len(correct_answer)):
@@ -258,30 +245,56 @@ class DragAndDrop(object):
                     self.user_groups[groupname].append(draggable_name)
                     self.user_positions[groupname]['user'].append(
                                             draggable_dict[draggable_name])
-                    check_extra_draggables[draggable_name] = True
+                    self.excess_draggables[draggable_name] = True
                 else:
-                    check_extra_draggables[draggable_name] = \
-                    check_extra_draggables.get(draggable_name, False)
-
-        for draggable in check_extra_draggables:
-            if not check_extra_draggables[draggable]:
-                self.incorrect = True
+                    self.excess_draggables[draggable_name] = \
+                    self.excess_draggables.get(draggable_name, False)
 
 
 def grade(user_input, correct_answer):
-    """Args:
-            user_input, correct_answer: json. Format:
+    """ Populates DragAndDrop instance from user_input and correct_answer and
+        calls DragAndDrop.drade for grading.
 
-            user_input: see module docstring
+        Supports two interfaces for correct_answer: dict and list.
 
-            correct_answer:
-                if use_targets is True:
-                    {'1': 't1',  'name_with_icon': 't2'}
-                else:
-                    {'1': '[10, 10]',  'name_with_icon': '[[10, 10], 20]'}
-    Support 2 interfaces"""
-    # import ipdb; ipdb.set_trace()
+        Args:
+            user_input: json. Format::
+
+                {"use_targets": false, "draggables":
+                [{"1": [10, 10]}, {"name_with_icon": [20, 20]}]}'
+
+                or
+
+                {"use_targets": true, "draggables": [{"1": "t1"}, \
+                {"name_with_icon": "t2"}]}
+
+            correct_answer: dict or list.
+
+                Dict form::
+
+                        {'1': 't1',  'name_with_icon': 't2'}
+
+                        or
+
+                        {'1': '[10, 10]',  'name_with_icon': '[[10, 10], 20]'}
+
+                List form::
+
+                    correct_answer = [
+                    {
+                        'draggables':  ['l3_o', 'l10_o'],
+                        'targets':  ['t1_o', 't9_o'],
+                        'rule': 'anyof'
+                    },
+                    {
+                        'draggables': ['l1_c','l8_c'],
+                        'targets': ['t5_c','t6_c'],
+                        'rule': 'anyof'
+                    }
+                    ]
+
+        Returns: bool
+    """
     dnd = DragAndDrop()
-    # import ipdb; ipdb.set_trace()
     dnd.populate(correct_answer=correct_answer, user_answer=user_input)
     return dnd.grade()