GH-791 Better use of result pattern

davidszkiba · mona-shakiba · commit d3fd88faa389 · 2025-02-10T12:09:20.000+01:00
diff --git a/classes/local/none.php b/classes/local/none.php
@@ -0,0 +1,50 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Сlass status.
+ *
+ * @package    local_catquiz
+ * @copyright  2023 Wunderbyte GmbH <georg.maisser@wunderbyte.at>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace local_catquiz\local;
+
+use Exception;
+use local_catquiz\local\status;
+
+/**
+ * Provides methods to obtain results.
+ *
+ *
+ * @package    local_catquiz
+ * @copyright  2023 Wunderbyte GmbH <georg.maisser@wunderbyte.at>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class none extends result {
+    public function and_then(callable $op): result {
+        return $this;
+    }
+
+    public function or_else(callable $op): result {
+        return $op($this);
+    }
+
+    public function expect() {
+        throw new Exception($this->geterrormessage());
+    }
+}
diff --git a/classes/local/result.php b/classes/local/result.php
@@ -24,6 +24,7 @@
 
 namespace local_catquiz\local;
 
+use Exception;
 use local_catquiz\local\status;
 
 /**
@@ -34,8 +35,7 @@
  * @copyright  2023 Wunderbyte GmbH <georg.maisser@wunderbyte.at>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class result {
-
+abstract class result {
     /**
      * @var string status
      */
@@ -63,25 +63,34 @@ public function __construct($value = null, string $status = status::OK) {
      * @param string $status
      * @param mixed|null $value
      *
-     * @return result
+     * @return none
      *
      */
     public static function err(string $status = status::ERROR_GENERAL, $value = null) {
-        return new result($value, $status);
+        return new none($value, $status);
     }
 
     /**
      * Returns OK result.
      *
      * @param mixed|null $value
      *
-     * @return result
+     * @return some
      *
      */
     public static function ok($value = null) {
-        return new result($value, status::OK);
+        return new some($value, status::OK);
     }
 
+    abstract public function and_then(callable $op): result;
+
+    abstract public function or_else(callable $op): result;
+
+    /**
+     * @throws Exception
+     */
+    abstract public function expect();
+
     /**
      * Returns status.
      *
diff --git a/classes/local/some.php b/classes/local/some.php
@@ -0,0 +1,50 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Сlass status.
+ *
+ * @package    local_catquiz
+ * @copyright  2023 Wunderbyte GmbH <georg.maisser@wunderbyte.at>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace local_catquiz\local;
+
+use local_catquiz\local\status;
+
+/**
+ * Provides methods to obtain results.
+ *
+ *
+ * @package    local_catquiz
+ * @copyright  2023 Wunderbyte GmbH <georg.maisser@wunderbyte.at>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class some extends result {
+
+    public function and_then(callable $op): result {
+        return $op($this);
+    }
+
+    public function or_else(callable $op): result {
+        return $this;
+    }
+
+    public function expect() {
+        return $this;
+    }
+}
diff --git a/classes/teststrategy/strategy.php b/classes/teststrategy/strategy.php
@@ -87,16 +87,15 @@ abstract class strategy {
      */
     public int $catcontextid;
 
-    /**
-     * @var array<preselect_task>
-     */
-    public array $scoremodifiers;
-
     /**
      * @var progress
      */
     protected progress $progress;
 
+    private cache $cache;
+
+    private $result;
+
     /**
      * These data provide the context for the selection of the next question.
      *
@@ -115,8 +114,7 @@ abstract class strategy {
     public function __construct() {
         global $CFG;
         require_once($CFG->dirroot . '/local/catquiz/lib.php');
-
-        $this->scoremodifiers = info::get_score_modifiers();
+        $this->cache = cache::make('local_catquiz', 'adaptivequizattempt');
     }
 
     /**
@@ -160,15 +158,14 @@ public function get_description(): string {
      */
     public function return_next_testitem(array $context) {
         $this->context = $context;
-        $cache = cache::make('local_catquiz', 'adaptivequizattempt');
         $maxtime = $context['progress']->get_starttime() + $context['max_attempttime_in_sec'];
         if (time() > $maxtime) {
             // The 'endtime' holds the last timepoint that is relevant for the
             // result. So if a student tries to answer a question after the
             // time limit, this value is set to the time limit (starttime + max
             // attempt time).
-            $cache->set('endtime', $maxtime);
-            $cache->set('catquizerror', status::EXCEEDED_MAX_ATTEMPT_TIME);
+            $this->cache->set('endtime', $maxtime);
+            $this->cache->set('catquizerror', status::EXCEEDED_MAX_ATTEMPT_TIME);
             return result::err(status::EXCEEDED_MAX_ATTEMPT_TIME);
         }
 
@@ -193,96 +190,63 @@ public function return_next_testitem(array $context) {
         }
 
         // Core methods called in every strategy.
-        $res = $this->update_personability();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
-        }
-        $context = $res->unwrap();
+        $res = $this->update_personability()
+            ->and_then(fn () => $this->first_question_selector())
+            ->or_else(fn ($res) => $this->after_error($res));
 
-        $res = $this->first_question_selector();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
-        }
         $val = $res->unwrap();
         // If the result contains a single object, this is the question to be returned.
         // Othewise, it contains the updated $context array with the ability and standarderror set in such a way, that the
         // teststrategy will return the correct question (e.g. the question corresponding to mean ability of all students).
         if (is_object($val)) {
             return $res;
         }
-        $this->context = $val;
 
-        $res = $this->add_scale_standarderror();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
-        }
-        $this->context = $res->unwrap();
-
-        $res = $this->maximumquestionscheck();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
-        }
-
-        $res = $this->removeplayedquestions();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
-        }
-        $this->context = $res->unwrap();
-
-        $res = $this->noremainingquestions();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
+        try {
+            $this->add_scale_standarderror()
+                ->and_then(fn () => $this->maximumquestionscheck())
+                ->and_then(fn () => $this->removeplayedquestions())
+                ->and_then(fn () => $this->noremainingquestions())
+                ->and_then(fn () => $this->fisherinformation())
+                ->or_else(fn($res) => $this->after_error($res))
+                ->expect();
+        } catch (Exception $e) {
+            return $this->result;
         }
 
-        $this->context = $this->fisherinformation()->unwrap();
-
         $res = $this->maybereturnpilot();
         $val = $res->unwrap();
         // If the value is an object, it is the pilot question that should be returned.
         if (is_object($val)) {
             return $res;
         }
-        $this->context = $val;
-
-        $this->context = $this->remove_uncalculated()->unwrap();
 
-        $this->context = $this->mayberemovescale()->unwrap();
-
-        $this->context = $this->last_time_played_penalty()->unwrap();
-
-        $res = $this->filterbystandarderror();
-        if ($res->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $res->get_status());
-            return $res;
+        try {
+            $this->remove_uncalculated()
+                ->and_then(fn() => $this->mayberemovescale())
+                ->and_then(fn() => $this->last_time_played_penalty())
+                ->and_then(fn() => $this->filterbystandarderror())
+                ->or_else(fn($res) => $this->after_error($res))
+                ->expect();
+        } catch (Exception $e) {
+            return $this->result;
         }
 
-        $this->context = $this->filterbytestinfo()->unwrap();
-
-        $this->context = $this->filterbyquestionsperscale()->unwrap();
-
-        $result = $this->select_question();
-
-        $this->progress->save();
-
-        $this->update_attemptfeedback($context);
-
-        if ($result->iserr()) {
-            $cache->set('endtime', time());
-            $cache->set('catquizerror', $result->get_status());
-            return $result;
+        try {
+            $result = $this->filterbytestinfo()
+                ->and_then(fn() => $this->filterbyquestionsperscale())
+                ->and_then(fn() => $this->select_question())
+                ->and_then(
+                    function ($res) {
+                        $this->progress->save();
+                        $this->update_attemptfeedback($this->context);
+                        return $res;
+                    }
+                )
+                ->or_else(fn ($res) => $this->after_error($res))
+                ->expect();
+        } catch (Exception $e) {
+            return $this->result;
         }
 
         $selectedquestion = $result->unwrap();
@@ -304,6 +268,15 @@ public function return_next_testitem(array $context) {
         return result::ok($selectedquestion);
     }
 
+    private function after_error($result) {
+        $this->progress->save();
+        $this->update_attemptfeedback($this->context);
+        $this->cache->set('endtime', time());
+        $this->cache->set('catquizerror', $result->get_status());
+        $this->result = $result;
+        return $result;
+    }
+
     /**
      * If true, the check page reload is called before updating the ability.
      *
