MDL-61077 question stats: make calculations more robust

mod/quiz/report/statistics/report.php

@@ -201,7 +201,7 @@ public function display($quiz, $cm, $course) {
 
         } else if ($qid) {
             // Report on an individual sub-question indexed questionid.
-            if (is_null($questionstats->for_subq($qid, $variantno))) {
+            if (!$questionstats->has_subq($qid, $variantno)) {
                 print_error('questiondoesnotexist', 'question');
             }
 

question/classes/statistics/questions/all_calculated_for_qubaid_condition.php

@@ -95,23 +95,41 @@ public function initialise_for_slot($slot, $question, $variant = null) {
         }
     }
 
+    /**
+     * Do we have stats for a particular quesitonid (and optionally variant)?
+     *
+     * @param int  $questionid The id of the sub question.
+     * @param int|null $variant if not null then we want the object to store a variant of a sub-question's stats.
+     * @return bool whether those stats exist (yet).
+     */
+    public function has_subq($questionid, $variant = null) {
+        if ($variant === null) {
+            return isset($this->subquestionstats[$questionid]);
+        } else {
+            return isset($this->subquestionstats[$questionid]->variantstats[$variant]);
+        }
+    }
+
     /**
      * Reference for a item stats instance for a questionid and optional variant no.
      *
      * @param int  $questionid The id of the sub question.
      * @param int|null $variant if not null then we want the object to store a variant of a sub-question's stats.
-     * @return calculated_for_subquestion|null null if the stats object does not yet exist.
+     * @return calculated|calculated_for_subquestion stats instance for a questionid and optional variant no.
+     *     Will be a calculated_for_subquestion if no variant specified.
+     * @throws \coding_exception if there is an attempt to respond to a non-existant set of stats.
      */
     public function for_subq($questionid, $variant = null) {
         if ($variant === null) {
             if (!isset($this->subquestionstats[$questionid])) {
-                return null;
+                throw new \coding_exception('Reference to unknown question id ' . $questionid);
             } else {
                 return $this->subquestionstats[$questionid];
             }
         } else {
             if (!isset($this->subquestionstats[$questionid]->variantstats[$variant])) {
-                return null;
+                throw new \coding_exception('Reference to unknown question id ' . $questionid .
+                        ' variant ' . $variant);
             } else {
                 return $this->subquestionstats[$questionid]->variantstats[$variant];
             }
@@ -136,23 +154,39 @@ public function get_all_slots() {
         return array_keys($this->questionstats);
     }
 
+    /**
+     * Do we have stats for a particular slot (and optionally variant)?
+     *
+     * @param int  $slot The slot no.
+     * @param int|null $variant if provided then we want the object which stores a variant of a position's stats.
+     * @return bool whether those stats exist (yet).
+     */
+    public function has_slot($slot, $variant = null) {
+        if ($variant === null) {
+            return isset($this->questionstats[$slot]);
+        } else {
+            return isset($this->questionstats[$slot]->variantstats[$variant]);
+        }
+    }
+
     /**
      * Get position stats instance for a slot and optional variant no.
      *
      * @param int  $slot The slot no.
-     * @param null $variant if provided then we want the object which stores a variant of a position's stats.
-     * @return calculated|null An instance of the class storing the calculated position stats.
+     * @param int|null $variant if provided then we want the object which stores a variant of a position's stats.
+     * @return calculated|calculated_for_subquestion An instance of the class storing the calculated position stats.
+     * @throws \coding_exception if there is an attempt to respond to a non-existant set of stats.
      */
     public function for_slot($slot, $variant = null) {
         if ($variant === null) {
             if (!isset($this->questionstats[$slot])) {
-                return null;
+                throw new \coding_exception('Reference to unknown slot ' . $slot);
             } else {
                 return $this->questionstats[$slot];
             }
         } else {
             if (!isset($this->questionstats[$slot]->variantstats[$variant])) {
-                return null;
+                throw new \coding_exception('Reference to unknown slot ' . $slot . ' variant ' . $variant);
             } else {
                 return $this->questionstats[$slot]->variantstats[$variant];
             }

question/classes/statistics/questions/calculator.php

@@ -106,7 +106,7 @@ public function calculate($qubaids) {
                 $israndomquestion = ($step->questionid != $this->stats->for_slot($step->slot)->questionid);
                 $breakdownvariants = !$israndomquestion && $this->stats->for_slot($step->slot)->break_down_by_variant();
                 // If this is a variant we have not seen before create a place to store stats calculations for this variant.
-                if ($breakdownvariants && is_null($this->stats->for_slot($step->slot , $step->variant))) {
+                if ($breakdownvariants && !$this->stats->has_slot($step->slot, $step->variant)) {
                     $question = $this->stats->for_slot($step->slot)->question;
                     $this->stats->initialise_for_slot($step->slot, $question, $step->variant);
                     $this->stats->for_slot($step->slot, $step->variant)->randomguessscore =
@@ -118,14 +118,14 @@ public function calculate($qubaids) {
 
                 // If this is a random question do the calculations for sub question stats.
                 if ($israndomquestion) {
-                    if (is_null($this->stats->for_subq($step->questionid))) {
+                    if (!$this->stats->has_subq($step->questionid)) {
                         $this->stats->initialise_for_subq($step);
                     } else if ($this->stats->for_subq($step->questionid)->maxmark != $step->maxmark) {
                         $this->stats->for_subq($step->questionid)->differentweights = true;
                     }
 
                     // If this is a variant of this subq we have not seen before create a place to store stats calculations for it.
-                    if (is_null($this->stats->for_subq($step->questionid, $step->variant))) {
+                    if (!$this->stats->has_subq($step->questionid, $step->variant)) {
                         $this->stats->initialise_for_subq($step, $step->variant);
                     }
 

question/classes/statistics/responses/analysis_for_question.php

@@ -128,6 +128,11 @@ public function get_analysis_for_subpart($variantno, $subpartid) {
         if (!isset($this->subparts[$variantno])) {
             $this->initialise_stats_for_variant($variantno);
         }
+        if (!isset($this->subparts[$variantno][$subpartid])) {
+            debugging('Unexpected sub-part id ' . $subpartid .
+                    ' encountered.');
+            $this->subparts[$variantno][$subpartid] = new analysis_for_subpart();
+        }
         return $this->subparts[$variantno][$subpartid];
     }
 

question/classes/statistics/responses/analysis_for_subpart.php

@@ -47,6 +47,11 @@
  */
 class analysis_for_subpart {
 
+    /**
+     * @var analysis_for_class[]
+     */
+    protected $responseclasses;
+
     /**
      * Takes an array of possible_responses as returned from {@link \question_type::get_possible_responses()}.
      *
@@ -57,14 +62,11 @@ public function __construct(array $responseclasses = null) {
             foreach ($responseclasses as $responseclassid => $responseclass) {
                 $this->responseclasses[$responseclassid] = new analysis_for_class($responseclass, $responseclassid);
             }
+        } else {
+            $this->responseclasses = [];
         }
     }
 
-    /**
-     * @var analysis_for_class[]
-     */
-    protected $responseclasses;
-
     /**
      * Unique ids for response classes.
      *
@@ -81,7 +83,12 @@ public function get_response_class_ids() {
      * @return analysis_for_class
      */
     public function get_response_class($classid) {
+        if (!isset($this->responseclasses[$classid])) {
+            debugging('Unexpected class id ' . $classid . ' encountered.');
+            $this->responseclasses[$classid] = new analysis_for_class('[Unknown]', $classid);
+        }
         return $this->responseclasses[$classid];
+
     }
 
     /**