Bug #5972 - Some question types mark a blank answer inconsistently.

Also, cleaned up the marking code for numerical and short answer questions, partly by making the base-class method more useful.

Rewrote the way the way short answer questions do wildcard matching to be more robust, Unicode safe, and generally not suck.

This is a big change, so I am pleased to report I tested it using my new Selinium  test suite. See: http://moodle.org/mod/forum/discuss.php?d=51161
This commit is contained in:
tjhunt
2006-08-04 16:53:43 +00:00
parent 00c300692f
commit 5a14d563b9
4 changed files with 67 additions and 127 deletions
+10 -8
View File
@@ -617,8 +617,8 @@ function get_question_states(&$questions, $cmoptions, $attempt) {
$states[$i]->changed = true;
// Create the empty question type specific information
if (!$QTYPES[$questions[$i]->qtype]
->create_session_and_responses($questions[$i], $states[$i], $cmoptions, $attempt)) {
if (!$QTYPES[$questions[$i]->qtype]->create_session_and_responses(
$questions[$i], $states[$i], $cmoptions, $attempt)) {
return false;
}
$states[$i]->last_graded = clone($states[$i]);
@@ -936,15 +936,16 @@ function question_process_responses(&$question, &$state, $action, $cmoptions, &$
if (question_isgradingevent($action->event)) {
$state->responses = $state->last_graded->responses;
}
// Check for unchanged responses (exactly unchanged, not equivalent).
// We also have to catch questions that the student has not yet attempted
$sameresponses = (($state->responses == $action->responses) or
($state->responses == array(''=>'') && array_keys(array_count_values($action->responses))===array('')));
$sameresponses = !$state->last_graded->event == QUESTION_EVENTOPEN &&
$state->responses == $action->responses;
// If the response has not been changed then we do not have to process it again
// unless the attempt is closing or validation is requested
if ($sameresponses and QUESTION_EVENTCLOSE != $action->event
and QUESTION_EVENTVALIDATE != $action->event) {
and QUESTION_EVENTVALIDATE != $action->event) {
return true;
}
@@ -974,13 +975,14 @@ function question_process_responses(&$question, &$state, $action, $cmoptions, &$
// Unless the attempt is closing, we want to work out if the current responses
// (or equivalent responses) were already given in the last graded attempt.
if((QUESTION_EVENTCLOSE != $action->event) and $QTYPES[$question->qtype]->compare_responses(
$question, $state, $state->last_graded)) {
if(QUESTION_EVENTCLOSE != $action->event && QUESTION_EVENTOPEN != $state->last_graded->event &&
$QTYPES[$question->qtype]->compare_responses($question, $state, $state->last_graded)) {
$state->event = QUESTION_EVENTDUPLICATE;
}
// If we did not find a duplicate or if the attempt is closing, perform grading
if ((!$sameresponses and (QUESTION_EVENTDUPLICATE != $state->event)) or (QUESTION_EVENTCLOSE == $action->event)) {
if ((!$sameresponses and QUESTION_EVENTDUPLICATE != $state->event) or
QUESTION_EVENTCLOSE == $action->event) {
// Decrease sumgrades by previous grade and then later add new grade
$attempt->sumgrades -= (float)$state->last_graded->grade;
+6 -27
View File
@@ -40,7 +40,7 @@ class question_numerical_qtype extends question_shortanswer_qtype {
}
$this->get_numerical_units($question);
// If units are defined we strip off the defaultunit from the answer, if
// If units are defined we strip off the default unit from the answer, if
// it is present. (Required for compatibility with the old code and DB).
if ($defaultunit = $this->get_default_numerical_unit($question)) {
foreach($question->options->answers as $key => $val) {
@@ -255,11 +255,11 @@ class question_numerical_qtype extends question_shortanswer_qtype {
return true;
}
function compare_responses(&$question, &$state, &$teststate) {
$response = isset($state->responses['']) ? $state->responses[''] : '';
$testresponse = isset($teststate->responses[''])
? $teststate->responses[''] : '';
return ($response == $testresponse);
function compare_responses(&$question, $state, $teststate) {
if (isset($state->responses['']) && isset($teststate->responses[''])) {
return $state->responses[''] == $teststate->responses[''];
}
return false;
}
/**
@@ -294,27 +294,6 @@ class question_numerical_qtype extends question_shortanswer_qtype {
return false;
}
function grade_responses(&$question, &$state, $cmoptions) {
$answers = &$question->options->answers;
$state->raw_grade = 0;
foreach($answers as $answer) {
if($this->test_response($question, $state, $answer)) {
$state->raw_grade = $answer->fraction;
break;
}
}
// Make sure we don't assign negative or too high marks
$state->raw_grade = min(max((float) $state->raw_grade,
0.0), 1.0) * $question->maxgrade;
$state->penalty = $question->penalty * $question->maxgrade;
// mark the state as graded
$state->event = ($state->event == QUESTION_EVENTCLOSE) ? QUESTION_EVENTCLOSEANDGRADE : QUESTION_EVENTGRADE;
return true;
}
function get_correct_responses(&$question, &$state) {
$correct = parent::get_correct_responses($question, $state);
if ($unit = $this->get_default_numerical_unit($question)) {
+18 -17
View File
@@ -249,7 +249,9 @@ class default_questiontype {
// will use the empty string '' as the index for that one response. This will
// automatically be stored in and restored from the answer field in the
// question_states table.
$state->responses = array('' => '');
$state->responses = array(
'' => '',
);
return true;
}
@@ -814,27 +816,26 @@ class default_questiontype {
* @param object $cmoptions
*/
function grade_responses(&$question, &$state, $cmoptions) {
/* The default implementation uses the comparison method to check if
the responses given are equivalent to the responses for each answer
in turn and sets the marks and penalty accordingly. This works for the
most simple question types. */
// The default implementation uses the test_response method to
// compare what the student entered against each of the possible
// answers stored in the question, and uses the grade from the
// first one that matches. It also sets the marks and penalty.
// This should be good enought for most simple question types.
$teststate = clone($state);
$teststate->raw_grade = 0;
$state->raw_grade = 0;
foreach($question->options->answers as $answer) {
$teststate->responses[''] = $answer->answer;
if($this->compare_responses($question, $state, $teststate)) {
$state->raw_grade = min(max((float) $answer->fraction,
0.0), 1.0) * $question->maxgrade;
if($this->test_response($question, $state, $answer)) {
$state->raw_grade = $answer->fraction;
break;
}
}
if (empty($state->raw_grade)) {
$state->raw_grade = 0.0;
}
// Only allow one attempt at the question
$state->penalty = 1;
// Make sure we don't assign negative or too high marks.
$state->raw_grade = min(max((float) $state->raw_grade,
0.0), 1.0) * $question->maxgrade;
// Update the penalty.
$state->penalty = $question->penalty * $question->maxgrade;
// mark the state as graded
$state->event = ($state->event == QUESTION_EVENTCLOSE) ? QUESTION_EVENTCLOSEANDGRADE : QUESTION_EVENTGRADE;
+33 -75
View File
@@ -35,6 +35,8 @@ class question_shortanswer_qtype extends default_questiontype {
}
function save_question_options($question) {
$result = new stdClass;
if (!$oldanswers = get_records("question_answers", "question", $question->id, "id ASC")) {
$oldanswers = array();
}
@@ -55,7 +57,7 @@ class question_shortanswer_qtype extends default_questiontype {
return $result;
}
} else { // This is a completely new answer
unset($answer);
$answer = new stdClass;
$answer->answer = trim($dataanswer);
$answer->question = $question->id;
$answer->fraction = $question->fraction[$key];
@@ -123,6 +125,7 @@ class question_shortanswer_qtype extends default_questiontype {
/// This implementation is also used by question type 'numerical'
$correctanswers = $this->get_correct_responses($question, $state);
$readonly = empty($options->readonly) ? '' : 'readonly="readonly"';
$formatoptions = new stdClass;
$formatoptions->noclean = true;
$formatoptions->para = false;
$nameprefix = $question->name_prefix;
@@ -182,82 +185,36 @@ class question_shortanswer_qtype extends default_questiontype {
return false;
}
function grade_responses(&$question, &$state, $cmoptions) {
function compare_responses($question, $state, $teststate) {
if (isset($state->responses['']) && isset($teststate->responses[''])) {
if ($question->options->usecase) {
return $state->responses[''] == $teststate->responses[''];
} else {
return strcasecmp($state->responses[''], $teststate->responses['']) == 0;
}
}
return false;
}
function test_response(&$question, $state, $answer) {
return $this->compare_string_with_wildcard($state->responses[''],
$answer->answer, !$question->options->usecase);
}
function compare_string_with_wildcard($string, $pattern, $ignorecase) {
// Break the string on non-escaped asterisks.
$bits = preg_split('/(?<!\\\\)\*/', $pattern);
// Escape regexp special characters in the bits.
$bits = array_map('preg_quote', $bits);
// Put it back together to make the regexp.
$regexp = '|^' . implode('.*', $bits) . '$|u';
$teststate = clone($state);
$state->raw_grade = 0;
// Compare the response with every teacher answer in turn
// and return the first one that matches.
foreach($question->options->answers as $answer) {
// Now we use a bit of a hack: we put the answer into the response
// of a teststate so that we can use the function compare_responses()
$teststate->responses[''] = trim($answer->answer);
if($this->compare_responses($question, $state, $teststate)) {
$state->raw_grade = $answer->fraction;
break;
}
// Make the match insensitive if requested to.
if ($ignorecase) {
$regexp .= 'i';
}
// Make sure we don't assign negative or too high marks
$state->raw_grade = min(max((float) $state->raw_grade,
0.0), 1.0) * $question->maxgrade;
$state->penalty = $question->penalty * $question->maxgrade;
// mark the state as graded
$state->event = ($state->event == QUESTION_EVENTCLOSE) ? QUESTION_EVENTCLOSEANDGRADE : QUESTION_EVENTGRADE;
return true;
}
function compare_responses(&$question, &$state, &$teststate) {
// In this questiontype this function is not only used to compare responses
// between two different states but it is also used by grade_responses() and
// by test_responses() to compare responses with answers.
if (isset($state->responses[''])) {
$response0 = trim(stripslashes($state->responses['']));
} else {
$response0 = '';
}
if (isset($teststate->responses[''])) {
$response1 = trim(stripslashes($teststate->responses['']));
} else {
$response1 = '';
}
if (!$question->options->usecase) { // Don't compare case
$response0 = strtolower($response0);
$response1 = strtolower($response1);
}
/// These are things to protect in the strings when wildcards are used
$search = array('\\', '+', '(', ')', '[', ']', '-');
$replace = array('\\\\', '\+', '\(', '\)', '\[', '\]', '\-');
if (strpos(' '.$response1, '*')) {
$response1 = str_replace('\*','@@@@@@',$response1);
$response1 = str_replace('*','.*',$response1);
$response1 = str_replace($search, $replace, $response1);
$response1 = str_replace('@@@@@@', '\*',$response1);
if (ereg('^'.$response1.'$', $response0)) {
return true;
}
} else if ($response1 == $response0) {
return true;
}
return false;
}
function test_response(&$question, &$state, &$answer) {
$teststate = clone($state);
$teststate->responses[''] = trim($answer->answer);
if($this->compare_responses($question, $state, $teststate)) {
return true;
}
return false;
return preg_match($regexp, trim($string));
}
/// BACKUP FUNCTIONS ////////////////////////////
@@ -307,6 +264,7 @@ class question_shortanswer_qtype extends default_questiontype {
$sho_info = $shortanswers[$i];
//Now, build the question_shortanswer record structure
$shortanswer = new stdClass;
$shortanswer->question = $new_question_id;
$shortanswer->answers = backup_todb($sho_info['#']['ANSWERS']['0']['#']);
$shortanswer->usecase = backup_todb($sho_info['#']['USECASE']['0']['#']);