From a3e6ad77d272207752b5144605a123e544303f66 Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 27 Jun 2012 16:17:36 +0100
Subject: [PATCH] MDL-34066 adaptive behaviour: refactor logic out of renderer.
These changes move the logic to the behaviour class, which is how things
should be.
It also makes it easier to re-use the code that displays the messages
like "Marks for this submission: 1.00/1.00. Accounting for previous
tries, this gives 0.33/1.00." in other places.
To try to make it clearer what is going on, I introduced a new class
qbehaviour_adaptive_mark_details to hold and document the data that the
behaviour needs to return, and the renderer can base its display on.
As far as my testing can tell (and there are new unit tests), this
commit does not change the existing behaviour.
This commit also replaces all the string concatenation that is going on,
which should help translators. At the moment, a few of the old strings
are still in the language file, and are used in the unit tests to verify
that the behaviour has not changed.
Thanks to Oleg Sychev for making a helpful suggestion about the API.
---
question/behaviour/adaptive/behaviour.php | 112 ++++++++++++++++++
.../adaptive/lang/en/qbehaviour_adaptive.php | 12 +-
question/behaviour/adaptive/renderer.php | 104 +++++++---------
.../adaptive/tests/mark_display_test.php | 110 +++++++++++++++++
.../behaviour/adaptivenopenalty/renderer.php | 3 +-
5 files changed, 277 insertions(+), 64 deletions(-)
create mode 100644 question/behaviour/adaptive/tests/mark_display_test.php
diff --git a/question/behaviour/adaptive/behaviour.php b/question/behaviour/adaptive/behaviour.php
index e85bc6815b0..84bcd0b7c88 100644
--- a/question/behaviour/adaptive/behaviour.php
+++ b/question/behaviour/adaptive/behaviour.php
@@ -216,4 +216,116 @@ class qbehaviour_adaptive extends question_behaviour_with_save {
public function is_state_improvable(question_state $state) {
return $state == question_state::$todo;
}
+
+ /**
+ * @return qbehaviour_adaptive_mark_details the information about the current state-of-play, scoring-wise,
+ * for this adaptive attempt.
+ */
+ public function get_adaptive_marks() {
+
+ // Try to find the last graded step.
+ $gradedstep = $this->get_graded_step();
+ if (is_null($gradedstep) || $this->qa->get_max_mark() == 0) {
+ // No score yet.
+ return new qbehaviour_adaptive_mark_details(question_state::$todo);
+ }
+
+ // Work out the applicable state.
+ if ($this->qa->get_state()->is_commented()) {
+ $state = $this->qa->get_state();
+ } else {
+ $state = question_state::graded_state_for_fraction(
+ $gradedstep->get_behaviour_var('_rawfraction'));
+ }
+
+ // Prepare the grading details.
+ $details = $this->adaptive_mark_details_from_step($gradedstep, $state, $this->qa->get_max_mark(), $this->question->penalty);
+ $details->improvable = $this->is_state_improvable($this->qa->get_state());
+ return $details;
+ }
+
+ /**
+ * Actually populate the qbehaviour_adaptive_mark_details object.
+ * @param question_attempt_step $gradedstep the step that holds the relevant mark details.
+ * @param question_state $state the state corresponding to $gradedstep.
+ * @param unknown_type $maxmark the maximum mark for this question_attempt.
+ * @param unknown_type $penalty the penalty for this question, as a fraction.
+ */
+ protected function adaptive_mark_details_from_step(question_attempt_step $gradedstep,
+ question_state $state, $maxmark, $penalty) {
+
+ $details = new qbehaviour_adaptive_mark_details($state);
+ $details->maxmark = $maxmark;
+ $details->actualmark = $gradedstep->get_fraction() * $details->maxmark;
+ $details->rawmark = $gradedstep->get_behaviour_var('_rawfraction') * $details->maxmark;
+
+ $details->currentpenalty = $penalty * $details->maxmark;
+ $details->totalpenalty = $details->currentpenalty * $this->qa->get_last_behaviour_var('_try', 0);
+
+ $details->improvable = $this->is_state_improvable($gradedstep->get_state());
+
+ return $details;
+ }
+}
+
+
+/**
+ * This class encapsulates all the information about the current state-of-play
+ * scoring-wise. It is used to communicate between the beahviour and the renderer.
+ *
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class qbehaviour_adaptive_mark_details {
+ /** @var question_state the current state of the question. */
+ public $state;
+
+ /** @var float the maximum mark for this question. */
+ public $maxmark;
+
+ /** @var float the current mark for this question. */
+ public $actualmark;
+
+ /** @var float the raw mark for this question before penalties were applied. */
+ public $rawmark;
+
+ /** @var float the the amount of additional penalty this attempt attracted. */
+ public $currentpenalty;
+
+ /** @var float the total that will apply to future attempts. */
+ public $totalpenalty;
+
+ /** @var bool whether it is possible for this mark to be improved in future. */
+ public $improvable;
+
+ /**
+ * Constructor.
+ * @param question_state $state
+ */
+ public function __construct($state, $maxmark = null, $actualmark = null, $rawmark = null,
+ $currentpenalty = null, $totalpenalty = null, $improvable = null) {
+ $this->state = $state;
+ $this->maxmark = $maxmark;
+ $this->actualmark = $actualmark;
+ $this->rawmark = $rawmark;
+ $this->currentpenalty = $currentpenalty;
+ $this->totalpenalty = $totalpenalty;
+ $this->improvable = $improvable;
+ }
+
+ /**
+ * Get the marks, formatted to a certain number of decimal places, in the
+ * form required by calls like get_string('gradingdetails', 'qbehaviour_adaptive', $a).
+ * @param int $markdp the number of decimal places required.
+ * @return array ready to substitute into language strings.
+ */
+ public function get_formatted_marks($markdp) {
+ return array(
+ 'max' => format_float($this->maxmark, $markdp),
+ 'cur' => format_float($this->actualmark, $markdp),
+ 'raw' => format_float($this->rawmark, $markdp),
+ 'penalty' => format_float($this->currentpenalty, $markdp),
+ 'totalpenalty' => format_float($this->totalpenalty, $markdp),
+ );
+ }
}
diff --git a/question/behaviour/adaptive/lang/en/qbehaviour_adaptive.php b/question/behaviour/adaptive/lang/en/qbehaviour_adaptive.php
index 6c415de1836..2dc2e53bc66 100644
--- a/question/behaviour/adaptive/lang/en/qbehaviour_adaptive.php
+++ b/question/behaviour/adaptive/lang/en/qbehaviour_adaptive.php
@@ -25,8 +25,16 @@
$string['disregardedwithoutpenalty'] = 'The submission was invalid, and has been disregarded without penalty.';
$string['gradingdetails'] = 'Marks for this submission: {$a->raw}/{$a->max}.';
+$string['gradingdetailswithadjustment'] = 'Marks for this submission: {$a->raw}/{$a->max}. Accounting for previous tries, this gives <strong>{$a->cur}/{$a->max}</strong>.';
+$string['gradingdetailswithadjustmentpenalty'] = 'Marks for this submission: {$a->raw}/{$a->max}. Accounting for previous tries, this gives <strong>{$a->cur}/{$a->max}</strong>. This submission attracted a penalty of {$a->penalty}.';
+$string['gradingdetailswithadjustmenttotalpenalty'] = 'Marks for this submission: {$a->raw}/{$a->max}. Accounting for previous tries, this gives <strong>{$a->cur}/{$a->max}</strong>. This submission attracted a penalty of {$a->penalty}. Total penalties so far: {$a->totalpenalty}.';
+$string['gradingdetailswithpenalty'] = 'Marks for this submission: {$a->raw}/{$a->max}. This submission attracted a penalty of {$a->penalty}.';
+$string['gradingdetailswithtotalpenalty'] = 'Marks for this submission: {$a->raw}/{$a->max}. This submission attracted a penalty of {$a->penalty}. Total penalties so far: {$a->totalpenalty}.';
+$string['notcomplete'] = 'Not complete';
+$string['pluginname'] = 'Adaptive mode';
+
+// Old strings these are currently only used in the unit tests, to verify that the new
+// strings give the same results as the old strings.
$string['gradingdetailsadjustment'] = 'Accounting for previous tries, this gives <strong>{$a->cur}/{$a->max}</strong>.';
$string['gradingdetailspenalty'] = 'This submission attracted a penalty of {$a}.';
$string['gradingdetailspenaltytotal'] = 'Total penalties so far: {$a}.';
-$string['notcomplete'] = 'Not complete';
-$string['pluginname'] = 'Adaptive mode';
diff --git a/question/behaviour/adaptive/renderer.php b/question/behaviour/adaptive/renderer.php
index aa88f9b46c0..3c1f90f8260 100644
--- a/question/behaviour/adaptive/renderer.php
+++ b/question/behaviour/adaptive/renderer.php
@@ -42,85 +42,70 @@ class qbehaviour_adaptive_renderer extends qbehaviour_renderer {
}
public function feedback(question_attempt $qa, question_display_options $options) {
+
+ // If the latest answer was invalid, display an informative message.
if ($qa->get_state() == question_state::$invalid) {
- // If the latest answer was invalid, display an informative message
- $output = '';
- $info = $this->disregarded_info();
- if ($info) {
- $output = html_writer::tag('div', $info, array('class' => 'gradingdetails'));
- }
- return $output;
+ return html_writer::nonempty_tag('div', $this->disregarded_info(),
+ array('class' => 'gradingdetails'));
}
- // Try to find the last graded step.
+ // Otherwise get the details.
+ return $this->render_adaptive_marks(
+ $qa->get_behaviour()->get_adaptive_marks(), $options);
+ }
- $gradedstep = $qa->get_behaviour()->get_graded_step($qa);
- if (is_null($gradedstep) || $qa->get_max_mark() == 0 ||
- $options->marks < question_display_options::MARK_AND_MAX) {
+ /**
+ * Display the scoring information about an adaptive attempt.
+ * @param qbehaviour_adaptive_mark_details contains all the score details we need.
+ * @param question_display_options $options display options.
+ */
+ public function render_adaptive_marks(qbehaviour_adaptive_mark_details $details, question_display_options $options) {
+ if ($details->state == question_state::$todo || $options->marks < question_display_options::MARK_AND_MAX) {
+ // No grades yet.
return '';
}
- // Display the grading details from the last graded state
- $mark = new stdClass();
- $mark->max = $qa->format_max_mark($options->markdp);
-
- $actualmark = $gradedstep->get_fraction() * $qa->get_max_mark();
- $mark->cur = format_float($actualmark, $options->markdp);
-
- $rawmark = $gradedstep->get_behaviour_var('_rawfraction') * $qa->get_max_mark();
- $mark->raw = format_float($rawmark, $options->markdp);
-
- // let student know wether the answer was correct
- if ($qa->get_state()->is_commented()) {
- $class = $qa->get_state()->get_feedback_class();
- } else {
- $class = question_state::graded_state_for_fraction(
- $gradedstep->get_behaviour_var('_rawfraction'))->get_feedback_class();
- }
-
- $gradingdetails = get_string('gradingdetails', 'qbehaviour_adaptive', $mark);
-
- $gradingdetails .= $this->penalty_info($qa, $mark, $options);
-
- $output = '';
- $output .= html_writer::tag('div', get_string($class, 'question'),
- array('class' => 'correctness ' . $class));
- $output .= html_writer::tag('div', $gradingdetails,
- array('class' => 'gradingdetails'));
- return $output;
+ // Display the grading details from the last graded state.
+ $class = $details->state->get_feedback_class();
+ return html_writer::tag('div', get_string($class, 'question'),
+ array('class' => 'correctness ' . $class))
+ . html_writer::tag('div', $this->grading_details($details, $options),
+ array('class' => 'gradingdetails'));
}
/**
* Display the information about the penalty calculations.
- * @param question_attempt $qa the question attempt.
- * @param object $mark contains information about the current mark.
+ * @param qbehaviour_adaptive_mark_details contains all the score details we need.
* @param question_display_options $options display options.
+ * @return string html fragment
*/
- protected function penalty_info(question_attempt $qa, $mark,
- question_display_options $options) {
+ protected function grading_details(qbehaviour_adaptive_mark_details $details, question_display_options $options) {
- $currentpenalty = $qa->get_question()->penalty * $qa->get_max_mark();
- $totalpenalty = $currentpenalty * $qa->get_last_behaviour_var('_try', 0);
+ $mark = $details->get_formatted_marks($options->markdp);
- if ($currentpenalty == 0) {
- return '';
+ if ($details->currentpenalty == 0 && $details->totalpenalty == 0) {
+ return get_string('gradingdetails', 'qbehaviour_adaptive', $mark);
}
+
$output = '';
// Print details of grade adjustment due to penalties
- if ($mark->raw != $mark->cur) {
- $output .= ' ' . get_string('gradingdetailsadjustment', 'qbehaviour_adaptive', $mark);
- }
+ if ($details->rawmark != $details->actualmark) {
+ if (!$details->improvable) {
+ return get_string('gradingdetailswithadjustment', 'qbehaviour_adaptive', $mark);
+ } else if ($details->totalpenalty > $details->currentpenalty) {
+ return get_string('gradingdetailswithadjustmenttotalpenalty', 'qbehaviour_adaptive', $mark);
+ } else {
+ return get_string('gradingdetailswithadjustmentpenalty', 'qbehaviour_adaptive', $mark);
+ }
- // Print information about any new penalty, only relevant if the answer can be improved.
- if ($qa->get_behaviour()->is_state_improvable($qa->get_state())) {
- $output .= ' ' . get_string('gradingdetailspenalty', 'qbehaviour_adaptive',
- format_float($currentpenalty, $options->markdp));
-
- // Print information about total penalties so far, if larger than current penalty.
- if ($totalpenalty > $currentpenalty) {
- $output .= ' ' . get_string('gradingdetailspenaltytotal', 'qbehaviour_adaptive',
- format_float($totalpenalty, $options->markdp));
+ } else {
+ if (!$details->improvable) {
+ return get_string('gradingdetails', 'qbehaviour_adaptive', $mark);
+ } else if ($details->totalpenalty > $details->currentpenalty) {
+ return get_string('gradingdetailswithtotalpenalty', 'qbehaviour_adaptive', $mark);
+ } else {
+ return get_string('gradingdetailswithpenalty', 'qbehaviour_adaptive', $mark);
}
}
@@ -133,5 +118,4 @@ class qbehaviour_adaptive_renderer extends qbehaviour_renderer {
protected function disregarded_info() {
return get_string('disregardedwithoutpenalty', 'qbehaviour_adaptive');
}
-
}
diff --git a/question/behaviour/adaptive/tests/mark_display_test.php b/question/behaviour/adaptive/tests/mark_display_test.php
new file mode 100644
index 00000000000..f77fdc9ecae
--- /dev/null
+++ b/question/behaviour/adaptive/tests/mark_display_test.php
@@ -0,0 +1,110 @@
+<?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/>.
+
+
+/**
+ * This file contains tests that just test the display mark/penalty information.
+ *
+ * @package qbehaviour_adaptive
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once(dirname(__FILE__) . '/../../../engine/lib.php');
+require_once(dirname(__FILE__) . '/../behaviour.php');
+
+
+/**
+ * Unit tests for the adaptive behaviour the display of mark/penalty information.
+ *
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class qbehaviour_adaptive_mark_display_test extends basic_testcase {
+ /** @var qbehaviour_adaptive_renderer the renderer to test. */
+ protected $renderer;
+
+ /** @var question_display_options display options to use when rendering. */
+ protected $options;
+
+ protected function setUp() {
+ global $PAGE;
+ parent::setUp();
+ $this->renderer = $PAGE->get_renderer('qbehaviour_adaptive');
+ $this->options = new question_display_options();
+ }
+
+ public function test_blank_before_graded() {
+ $this->assertEquals('',
+ $this->renderer->render_adaptive_marks(new qbehaviour_adaptive_mark_details(
+ question_state::$todo), $this->options));
+ }
+
+ public function test_correct_no_penalty() {
+ $this->assertEquals('<div class="correctness correct">' . get_string('correct', 'question') . '</div>' .
+ '<div class="gradingdetails">' .
+ get_string('gradingdetails', 'qbehaviour_adaptive',
+ array('cur' => '1.00', 'raw' => '1.00', 'max' => '1.00')) . '</div>',
+ $this->renderer->render_adaptive_marks(new qbehaviour_adaptive_mark_details(
+ question_state::$gradedright, 1, 1, 1, 0, 0, false), $this->options));
+ }
+
+ public function test_partial_first_try() {
+ $this->assertEquals('<div class="correctness partiallycorrect">' . get_string('partiallycorrect', 'question') . '</div>' .
+ '<div class="gradingdetails">' .
+ get_string('gradingdetails', 'qbehaviour_adaptive',
+ array('cur' => '0.50', 'raw' => '0.50', 'max' => '1.00')) . ' ' .
+ get_string('gradingdetailspenalty', 'qbehaviour_adaptive', '0.10') . '</div>',
+ $this->renderer->render_adaptive_marks(new qbehaviour_adaptive_mark_details(
+ question_state::$gradedpartial, 1, 0.5, 0.5, 0.1, 0.1, true), $this->options));
+ }
+
+ public function test_partial_second_try() {
+ $mark = array('cur' => '0.80', 'raw' => '0.90', 'max' => '1.00');
+ $this->assertEquals('<div class="correctness partiallycorrect">' . get_string('partiallycorrect', 'question') . '</div>' .
+ '<div class="gradingdetails">' .
+ get_string('gradingdetails', 'qbehaviour_adaptive', $mark) . ' ' .
+ get_string('gradingdetailsadjustment', 'qbehaviour_adaptive', $mark) . ' ' .
+ get_string('gradingdetailspenalty', 'qbehaviour_adaptive', '0.10') . ' ' .
+ get_string('gradingdetailspenaltytotal', 'qbehaviour_adaptive', '0.20') . '</div>',
+ $this->renderer->render_adaptive_marks(new qbehaviour_adaptive_mark_details(
+ question_state::$gradedpartial, 1, 0.8, 0.9, 0.1, 0.2, true), $this->options));
+ }
+
+ public function test_correct_third_try() {
+ $mark = array('cur' => '0.80', 'raw' => '1.00', 'max' => '1.00');
+ $this->assertEquals('<div class="correctness partiallycorrect">' . get_string('partiallycorrect', 'question') . '</div>' .
+ '<div class="gradingdetails">' .
+ get_string('gradingdetails', 'qbehaviour_adaptive', $mark) . ' ' .
+ get_string('gradingdetailsadjustment', 'qbehaviour_adaptive', $mark) . '</div>',
+ $this->renderer->render_adaptive_marks(new qbehaviour_adaptive_mark_details(
+ question_state::$gradedpartial, 1, 0.8, 1.0, 0.1, 0.3, false), $this->options));
+ }
+
+ public function test_correct_third_try_if_we_dont_increase_penalties_for_wrong() {
+ $mark = array('cur' => '0.80', 'raw' => '1.00', 'max' => '1.00');
+ $this->assertEquals('<div class="correctness partiallycorrect">' . get_string('partiallycorrect', 'question') . '</div>' .
+ '<div class="gradingdetails">' .
+ get_string('gradingdetails', 'qbehaviour_adaptive', $mark) . ' ' .
+ get_string('gradingdetailsadjustment', 'qbehaviour_adaptive', $mark) . '</div>',
+ $this->renderer->render_adaptive_marks(new qbehaviour_adaptive_mark_details(
+ question_state::$gradedpartial, 1, 0.8, 1.0, 0, 0.2, false), $this->options));
+ }
+}
diff --git a/question/behaviour/adaptivenopenalty/renderer.php b/question/behaviour/adaptivenopenalty/renderer.php
index 8b1e738cf36..34d3ae184f8 100644
--- a/question/behaviour/adaptivenopenalty/renderer.php
+++ b/question/behaviour/adaptivenopenalty/renderer.php
@@ -38,8 +38,7 @@ require_once(dirname(__FILE__) . '/../adaptive/renderer.php');
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class qbehaviour_adaptivenopenalty_renderer extends qbehaviour_adaptive_renderer {
- protected function penalty_info(question_attempt $qa, $mark,
- question_display_options $options) {
+ protected function penalty_info(qbehaviour_adaptive_mark_details $details, question_display_options $options) {
return '';
}
protected function disregarded_info() {