Skip to content

Commit

Permalink
MDL-27418 qtype numeric go back to the very permissive number parsing…
Browse files Browse the repository at this point in the history
… from Moodle 2.0.

I hope my unit tests are good enough to make this change safe.
  • Loading branch information
timhunt committed Jun 28, 2011
1 parent eef75d4 commit 1a1353a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 16 deletions.
42 changes: 33 additions & 9 deletions question/type/numerical/questiontype.php
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,11 @@ protected function build_regex() {
}

/**
* This method can be used for more locale-strict parsing of repsonses. At the
* moment we don't use it, and instead use the more lax parsing in apply_units.
* This is just a note that this funciton was used in the past, so if you are
* intersted, look through version control history.
*
* Take a string which is a number with or without a decimal point and exponent,
* and possibly followed by one of the units, and split it into bits.
* @param string $response a value, optionally with a unit.
Expand Down Expand Up @@ -585,25 +590,44 @@ protected function parse_response($response) {
}

/**
* Takes a number in localised form, that is, using the decsep and thousandssep
* defined in the lanuage pack, and possibly with a unit after it. It separates
* off the unit, if present, and converts to the default unit, by using the
* given unit multiplier.
* Takes a number in almost any localised form, and possibly with a unit
* after it. It separates off the unit, if present, and converts to the
* default unit, by using the given unit multiplier.
*
* @param string $response a value, optionally with a unit.
* @return array(numeric, sting) the value with the unit stripped, and normalised
* by the unit multiplier, if any, and the unit string, for reference.
*/
public function apply_units($response, $separateunit = null) {
list($beforepoint, $decimals, $exponent, $unit) = $this->parse_response($response);
// Strip spaces (which may be thousands separators) and change other forms
// of writing e to e.
$response = str_replace(' ', '', $response);
$response = preg_replace('~(?:e|E|(?:x|\*|×)10(?:\^|\*\*))([+-]?\d+)~', 'e$1', $response);

// If a . is present or there are multiple , (i.e. 2,456,789 ) assume ,
// is a thouseands separator, and strip it, else assume it is a decimal
// separator, and change it to ..
if (strpos($response, '.') !== false || substr_count($response, ',') > 1) {
$response = str_replace(',', '', $response);
} else {
$response = str_replace(',', '.', $response);
}

if (is_null($beforepoint)) {
$regex = '[+-]?(?:\d+(?:\\.\d*)?|\\.\d+)(?:e[-+]?\d+)?';
if ($this->unitsbefore) {
$regex = "/$regex$/";
} else {
$regex = "/^$regex/";
}
if (!preg_match($regex, $response, $matches)) {
return array(null, null);
}

$numberstring = $beforepoint . '.' . $decimals;
if ($exponent) {
$numberstring .= 'e' . $exponent;
$numberstring = $matches[0];
if ($this->unitsbefore) {
$unit = substr($response, 0, -strlen($numberstring));
} else {
$unit = substr($response, strlen($numberstring));
}

if (!is_null($separateunit)) {
Expand Down
4 changes: 2 additions & 2 deletions question/type/numerical/simpletest/testform.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ public function tearDown() {

public function test_is_valid_number() {
$this->assertTrue($this->form->is_valid_number('1,001'));
$this->assertFalse($this->form->is_valid_number('1.001'));
$this->assertTrue($this->form->is_valid_number('1.001'));
$this->assertTrue($this->form->is_valid_number('1'));
$this->assertTrue($this->form->is_valid_number('1,e8'));
$this->assertFalse($this->form->is_valid_number('1001 xxx'));
$this->assertFalse($this->form->is_valid_number('1.e8'));
$this->assertTrue($this->form->is_valid_number('1.e8'));
}
}
10 changes: 5 additions & 5 deletions question/type/numerical/simpletest/testquestion.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,17 @@ public function test_grading_currency() {
$this->assertEqual(array(1, question_state::$gradedright),
$question->grade_response(array('answer' => '$1332')));
$this->assertEqual(array(1, question_state::$gradedright),
$question->grade_response(array('answer' => '$ 1,332')));
$question->grade_response(array('answer' => '$ 1332')));
$this->assertEqual(array(0.8, question_state::$gradedpartial),
$question->grade_response(array('answer' => 'frog 1332')));
$this->assertEqual(array(0.8, question_state::$gradedpartial),
$question->grade_response(array('answer' => '1332')));
$this->assertEqual(array(0.8, question_state::$gradedpartial),
$question->grade_response(array('answer' => ' 1,332')));
$question->grade_response(array('answer' => ' 1332')));
$this->assertEqual(array(0, question_state::$gradedwrong),
$question->grade_response(array('answer' => '1332 $')));
$this->assertEqual(array(0, question_state::$gradedwrong),
$question->grade_response(array('answer' => '1,332 frogs')));
$question->grade_response(array('answer' => '1332 frogs')));
$this->assertEqual(array(0, question_state::$gradedwrong),
$question->grade_response(array('answer' => '$1')));
}
Expand Down Expand Up @@ -225,7 +225,7 @@ public function test_classify_response_currency() {
new question_classified_response(14, '$100', 0)),
$num->classify_response(array('answer' => '$100')));
$this->assertEqual(array(
new question_classified_response(13, '1,332', 0.8)),
$num->classify_response(array('answer' => '1,332')));
new question_classified_response(13, '1 332', 0.8)),
$num->classify_response(array('answer' => '1 332')));
}
}

0 comments on commit 1a1353a

Please sign in to comment.