diff --git a/question/type/calculated/question.php b/question/type/calculated/question.php index a113ef4f67d..7715726be83 100644 --- a/question/type/calculated/question.php +++ b/question/type/calculated/question.php @@ -37,24 +37,25 @@ require_once($CFG->dirroot . '/question/type/numerical/question.php'); */ class qtype_calculated_question extends qtype_numerical_question { /** @var qtype_calculated_dataset_loader helper for loading the dataset. */ - protected $datasetloader; + public $datasetloader; /** @var qtype_calculated_variable_substituter stores the dataset we are using. */ - protected $vs; + public $vs; public function start_attempt(question_attempt_step $step) { - $maxnumber = $this->datasetloader->get_number_of_datasets(); + $maxnumber = $this->datasetloader->get_number_of_items(); $setnumber = rand(1, $maxnumber); // TODO implement the $synchronizecalculated bit from create_session_and_responses. - $this->vs = $this->datasetloader->load_dataset($setnumber); + $this->vs = new qtype_calculated_variable_substituter( + $this->datasetloader->get_values($setnumber), + get_string('decsep', 'langconfig')); + $this->calculate_all_expressions(); $step->set_qt_var('_dataset', $setnumber); foreach ($this->vs->get_values() as $name => $value) { $step->set_qt_var('_var_' . $name, $value); } - $this->calculate_all_expressions(); - parent::start_attempt($step); } @@ -65,8 +66,9 @@ class qtype_calculated_question extends qtype_numerical_question { $values[substr($name, 5)] = $value; } } - $this->vs = new qtype_calculated_variable_substituter($values); + $this->vs = new qtype_calculated_variable_substituter( + $values, get_string('decsep', 'langconfig')); $this->calculate_all_expressions(); parent::apply_attempt_state($step); @@ -135,21 +137,12 @@ class qtype_calculated_dataset_loader { } /** - * Load a particular set of values for each dataset used by this question. + * Actually query the database for the values. * @param int $itemnumber which set of values to load. - * 0 < $itemnumber <= {@link get_number_of_items()}. - * @return qtype_calculated_variable_substituter with the correct variable - * -> value substitutions set up. + * @return array name => value; */ - public function load_values($itemnumber) { - if ($itemnumber <= 0 || $itemnumber > $this->get_number_of_items()) { - $a = new stdClass(); - $a->id = $this->questionid; - $a->item = $itemnumber; - throw new moodle_exception('cannotgetdsfordependent', 'question', '', $a); - } - - $values = $DB->get_records_sql(' + protected function load_values($itemnumber) { + return $DB->get_records_sql(' SELECT qdd.name, qdi.value FROM {question_dataset_items} qdi JOIN {question_dataset_definitions} qdd ON qdd.id = qdi.definition @@ -157,8 +150,23 @@ class qtype_calculated_dataset_loader { WHERE qd.question = ? AND qdi.itemnumber = ? ', array($this->questionid, $itemnumber)); + } - return new qtype_calculated_variable_substituter($values); + /** + * Load a particular set of values for each dataset used by this question. + * @param int $itemnumber which set of values to load. + * 0 < $itemnumber <= {@link get_number_of_items()}. + * @return array name => value. + */ + public function get_values($itemnumber) { + if ($itemnumber <= 0 || $itemnumber > $this->get_number_of_items()) { + $a = new stdClass(); + $a->id = $this->questionid; + $a->item = $itemnumber; + throw new moodle_exception('cannotgetdsfordependent', 'question', '', $a); + } + + return $this->load_values($itemnumber); } } @@ -177,6 +185,9 @@ class qtype_calculated_variable_substituter { /** @var array variable name => value */ protected $values; + /** @var string character to use for the decimal point in displayed numbers. */ + protected $decimalpoint; + /** @var array variable names wrapped in {...}. Used by {@link substitute_values()}. */ protected $search; @@ -184,14 +195,21 @@ class qtype_calculated_variable_substituter { * @var array variable values, with negative numbers wrapped in (...). * Used by {@link substitute_values()}. */ - protected $replace; + protected $safevalue; + + /** + * @var array variable values, with negative numbers wrapped in (...). + * Used by {@link substitute_values()}. + */ + protected $prettyvalue; /** * Constructor * @param array $values variable name => value. */ - public function __construct(array $values) { + public function __construct(array $values, $decimalpoint) { $this->values = $values; + $this->decimalpoint = $decimalpoint; // Prepare an array for {@link substitute_values()}. $this->search = array(); @@ -205,20 +223,25 @@ class qtype_calculated_variable_substituter { } $this->search[] = '{' . $name . '}'; - if ($value < 0) { - $this->replace[] = '(' . $value . ')'; - } else { - $this->replace[] = $value; - } + $this->safevalue[] = '(' . $value . ')'; + $this->prettyvalue[] = $this->format_float($value); } } + /** + * Display a float properly formatted with a certain number of decimal places. + * @param $x + */ + public function format_float($x) { + return str_replace('.', $this->decimalpoint, $x); + } + /** * Return an array of the variables and their values. * @return array name => value. */ public function get_values() { - return clone($this->values); + return $this->values; } /** @@ -228,28 +251,58 @@ class qtype_calculated_variable_substituter { * @return float the computed result. */ public function calculate($expression) { - $exp = $this->substitute_values($expression); - // This validation trick from http://php.net/manual/en/function.eval.php - if (!@eval('return true; $result = ' . $exp . ';')) { - throw new moodle_exception('illegalformulasyntax', 'qtype_calculated', '', $expression); - } - return eval('return ' . $exp . ';'); + return $this->calculate_raw($this->substitute_values_for_eval($expression)); } /** - * Substitute variable placehodlers like {a} with their value. + * Evaluate an expression after the variable values have been substituted. + * @param string $expression the expression. A PHP expression with placeholders + * like {a} for where the variables need to go. + * @return float the computed result. + */ + protected function calculate_raw($expression) { + // This validation trick from http://php.net/manual/en/function.eval.php + if (!@eval('return true; $result = ' . $expression . ';')) { + throw new moodle_exception('illegalformulasyntax', 'qtype_calculated', '', $expression); + } + return eval('return ' . $expression . ';'); + } + + /** + * Substitute variable placehodlers like {a} with their value wrapped in (). * @param string $expression the expression. A PHP expression with placeholders * like {a} for where the variables need to go. * @return string the expression with each placeholder replaced by the * corresponding value. */ - protected function substitute_values($expression) { - return str_replace($this->search, $this->replace, $expression); + protected function substitute_values_for_eval($expression) { + return str_replace($this->search, $this->safevalue, $expression); } + /** + * Substitute variable placehodlers like {a} with their value without wrapping + * the value in anything. + * @param string $text some content with placeholders + * like {a} for where the variables need to go. + * @return string the expression with each placeholder replaced by the + * corresponding value. + */ + protected function substitute_values_pretty($text) { + return str_replace($this->search, $this->prettyvalue, $text); + } + + /** + * Replace any embedded variables (like {a}) or formulae (like {={a} + {b}}) + * in some text with the corresponding values. + * @param string $text the text to process. + * @return string the text with values substituted. + */ public function replace_expressions_in_text($text) { - // TODO - return $text; + $vs = $this; // Can't see to use $this in a PHP closure. + $text = preg_replace_callback('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~', function ($matches) use ($vs) { + return $vs->format_float($vs->calculate($matches[1])); + }, $text); + return $this->substitute_values_pretty($text); } /** diff --git a/question/type/calculated/renderer.php b/question/type/calculated/renderer.php index b3fea565e15..b3aca3074af 100644 --- a/question/type/calculated/renderer.php +++ b/question/type/calculated/renderer.php @@ -26,6 +26,8 @@ defined('MOODLE_INTERNAL') || die(); +require_once($CFG->dirroot . '/question/type/numerical/renderer.php'); + /** * Generates the output for calculated questions. diff --git a/question/type/calculated/simpletest/helper.php b/question/type/calculated/simpletest/helper.php new file mode 100644 index 00000000000..106065f797e --- /dev/null +++ b/question/type/calculated/simpletest/helper.php @@ -0,0 +1,96 @@ +. + +/** + * Test helpers for the calculated question type. + * + * @package qtype + * @subpackage calculated + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +defined('MOODLE_INTERNAL') || die(); + + +/** + * Test helper class for the calculated question type. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_calculated_test_helper extends question_test_helper { + public function get_test_questions() { + return array('sum'); + } + + /** + * Makes a calculated question with correct ansewer 3.14, and various incorrect + * answers with different feedback. + * @return qtype_calculated_question + */ + public function make_calculated_question_sum() { + question_bank::load_question_definition_classes('calculated'); + $q = new qtype_calculated_question(); + test_question_maker::initialise_a_question($q); + $q->name = 'Simple sum'; + $q->questiontext = 'What is {a} + {b}?'; + $q->generalfeedback = 'Generalfeedback: {={a} + {b}} is the right answer.'; + $q->answers = array( + 13 => new qtype_numerical_answer(13, '{a} + {b}', 1.0, 'Very good.', FORMAT_HTML, 0), + 14 => new qtype_numerical_answer(14, '{a} - {b}', 0.0, 'Add. not subtract!.', FORMAT_HTML, 0), + 17 => new qtype_numerical_answer(17, '*', 0.0, 'Completely wrong.', FORMAT_HTML, 0), + ); + $q->qtype = question_bank::get_qtype('calculated'); + $q->unitdisplay = qtype_numerical::UNITNONE; + $q->unitgradingtype = 0; + $q->unitpenalty = 0; + $q->ap = new qtype_numerical_answer_processor(array()); + + $q->datasetloader = new qtype_calculated_test_dataset_loader(0, array( + array('a' => 1, 'b' => 5), + array('a' => 3, 'b' => 4), + )); + + return $q; + } +} + + +/** + * Test implementation of {@link qtype_calculated_dataset_loader}. Gets the values + * from an array passed to the constructor, rather than querying the database. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_calculated_test_dataset_loader extends qtype_calculated_dataset_loader{ + protected $valuesets; + + public function __construct($questionid, array $valuesets) { + parent::__construct($questionid); + $this->valuesets = $valuesets; + } + + public function get_number_of_items() { + return count($this->valuesets); + } + + public function load_values($itemnumber) { + return $this->valuesets[$itemnumber - 1]; + } +} diff --git a/question/type/calculated/simpletest/testquestion.php b/question/type/calculated/simpletest/testquestion.php new file mode 100644 index 00000000000..d368e987258 --- /dev/null +++ b/question/type/calculated/simpletest/testquestion.php @@ -0,0 +1,111 @@ +. + + +/** + * Unit tests for the calculated question definition class. + * + * @package qtype + * @subpackage calculated + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +require_once($CFG->dirroot . '/question/engine/simpletest/helpers.php'); + + +/** + * Unit tests for qtype_calculated_definition. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_calculated_question_test extends UnitTestCase { + public function test_is_complete_response() { + $question = test_question_maker::make_question('calculated'); + + $this->assertFalse($question->is_complete_response(array())); + $this->assertTrue($question->is_complete_response(array('answer' => '0'))); + $this->assertTrue($question->is_complete_response(array('answer' => 0))); + $this->assertFalse($question->is_complete_response(array('answer' => 'test'))); + } + + public function test_is_gradable_response() { + $question = test_question_maker::make_question('calculated'); + + $this->assertFalse($question->is_gradable_response(array())); + $this->assertTrue($question->is_gradable_response(array('answer' => '0'))); + $this->assertTrue($question->is_gradable_response(array('answer' => 0))); + $this->assertTrue($question->is_gradable_response(array('answer' => 'test'))); + } + + public function test_grading() { + $question = test_question_maker::make_question('calculated'); + $question->start_attempt(new question_attempt_step()); + $values = $question->vs->get_values(); + + $this->assertEqual(array(0, question_state::$gradedwrong), + $question->grade_response(array('answer' => $values['a'] - $values['b']))); + $this->assertEqual(array(1, question_state::$gradedright), + $question->grade_response(array('answer' => $values['a'] + $values['b']))); + } + + public function test_get_correct_response() { + $question = test_question_maker::make_question('calculated'); + $question->start_attempt(new question_attempt_step()); + $values = $question->vs->get_values(); + + $this->assertEqual(array('answer' => $values['a'] + $values['b']), + $question->get_correct_response()); + } + + public function test_get_question_summary() { + $question = test_question_maker::make_question('calculated'); + $question->start_attempt(new question_attempt_step()); + $values = $question->vs->get_values(); + + $qsummary = $question->get_question_summary(); + $this->assertEqual('What is ' . $values['a'] . ' + ' . $values['b'] . '?', $qsummary); + } + + public function test_summarise_response() { + $question = test_question_maker::make_question('calculated'); + $question->start_attempt(new question_attempt_step()); + $values = $question->vs->get_values(); + + $this->assertEqual('3.1', $question->summarise_response(array('answer' => '3.1'))); + } + + public function test_classify_response() { + $question = test_question_maker::make_question('calculated'); + $question->start_attempt(new question_attempt_step()); + $values = $question->vs->get_values(); + + $this->assertEqual(array( + new question_classified_response(13, $values['a'] + $values['b'], 1.0)), + $question->classify_response(array('answer' => $values['a'] + $values['b']))); + $this->assertEqual(array( + new question_classified_response(14, $values['a'] - $values['b'], 0.0)), + $question->classify_response(array('answer' => $values['a'] - $values['b']))); + $this->assertEqual(array( + new question_classified_response(17, 7 * $values['a'], 0.0)), + $question->classify_response(array('answer' => 7 * $values['a']))); + $this->assertEqual(array( + question_classified_response::no_response()), + $question->classify_response(array('answer' => ''))); + } +} \ No newline at end of file diff --git a/question/type/calculated/simpletest/testvariablesubstituter.php b/question/type/calculated/simpletest/testvariablesubstituter.php index 37729628d9c..63e1be10a57 100644 --- a/question/type/calculated/simpletest/testvariablesubstituter.php +++ b/question/type/calculated/simpletest/testvariablesubstituter.php @@ -37,24 +37,57 @@ require_once($CFG->dirroot . '/question/type/calculated/question.php'); */ class qtype_calculated_variable_substituter_test extends UnitTestCase { public function test_simple_expression() { - $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2)); + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); $this->assertEqual(3, $vs->calculate('{a} + {b}')); } public function test_simple_expression_negatives() { - $vs = new qtype_calculated_variable_substituter(array('a' => -1, 'b' => -2)); + $vs = new qtype_calculated_variable_substituter(array('a' => -1, 'b' => -2), '.'); $this->assertEqual(1, $vs->calculate('{a}-{b}')); } public function test_cannot_use_nonnumbers() { $this->expectException(); - $vs = new qtype_calculated_variable_substituter(array('a' => 'frog', 'b' => -2)); + $vs = new qtype_calculated_variable_substituter(array('a' => 'frog', 'b' => -2), '.'); } public function test_invalid_expression() { $this->expectException(); - $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2)); + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); $vs->calculate('{a} + {b}?'); } + public function test_tricky_invalid_expression() { + $this->expectException(); + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); + $vs->calculate('{a}{b}'); // Have to make sure this does not just evaluate to 12. + } + + public function test_replace_expressions_in_text_simple_var() { + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); + $this->assertEqual('1 + 2', $vs->replace_expressions_in_text('{a} + {b}')); + } + + public function test_replace_expressions_in_confusing_text() { + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); + $this->assertEqual("(1) 1\n(2) 2", $vs->replace_expressions_in_text("(1) {a}\n(2) {b}")); + } + + public function test_replace_expressions_in_text_formula() { + $vs = new qtype_calculated_variable_substituter(array('a' => 1, 'b' => 2), '.'); + $this->assertEqual('= 3', $vs->replace_expressions_in_text('= {={a} + {b}}')); + } + + public function test_replace_expressions_in_text_negative() { + $vs = new qtype_calculated_variable_substituter(array('a' => -1, 'b' => 2), '.'); + $this->assertEqual('temperatures -1 and 2', + $vs->replace_expressions_in_text('temperatures {a} and {b}')); + } + + public function test_replace_expressions_in_text_commas_for_decimals() { + $vs = new qtype_calculated_variable_substituter( + array('phi' => 1.61803399, 'pi' => 3.14159265), ','); + $this->assertEqual('phi (1,61803399) + pi (3,14159265) = 4,75962664', + $vs->replace_expressions_in_text('phi ({phi}) + pi ({pi}) = {={phi} + {pi}}')); + } } diff --git a/question/type/calculated/simpletest/testwalkthrough.php b/question/type/calculated/simpletest/testwalkthrough.php new file mode 100644 index 00000000000..ae4bb94f9c6 --- /dev/null +++ b/question/type/calculated/simpletest/testwalkthrough.php @@ -0,0 +1,104 @@ +. + +/** + * This file contains overall tests of numerical questions. + * + * @package qtype + * @subpackage calculated + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/question/engine/simpletest/helpers.php'); + + +/** + * Unit tests for the calculated question type. + * + * @copyright 2011 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class qtype_calculated_walkthrough_test extends qbehaviour_walkthrough_test_base { + public function test_interactive() { + + // Create a gapselect question. + $q = test_question_maker::make_question('calculated'); + $q->hints = array( + new question_hint(1, 'This is the first hint.', FORMAT_HTML), + new question_hint(2, 'This is the second hint.', FORMAT_HTML), + ); + $this->start_attempt_at_question($q, 'interactive', 3); + $values = $q->vs->get_values(); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Submit blank. + $this->process_submission(array('-submit' => 1, 'answer' => '')); + + // Verify. + $this->check_current_state(question_state::$invalid); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation(), + $this->get_contains_validation_error_expectation(), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Sumit something that does not look like a number. + $this->process_submission(array('-submit' => 1, 'answer' => 'newt')); + + // Verify. + $this->check_current_state(question_state::$invalid); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation(), + $this->get_contains_validation_error_expectation(), + new PatternExpectation('/' . + preg_quote(get_string('invalidnumber', 'qtype_numerical') . '/')), + $this->get_does_not_contain_try_again_button_expectation(), + $this->get_no_hint_visible_expectation()); + + // Now get it right. + $this->process_submission(array('-submit' => 1, 'answer' => $values['a'] + $values['b'])); + + // Verify. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(3); + $this->check_current_output( + $this->get_contains_mark_summary(3), + $this->get_contains_submit_button_expectation(false), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_validation_error_expectation(), + $this->get_no_hint_visible_expectation()); + } +}