From 5a14d563b9a870bb5b1dd95efe4fb0b6de716f29 Mon Sep 17 00:00:00 2001 From: tjhunt Date: Fri, 4 Aug 2006 16:53:43 +0000 Subject: [PATCH] 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 --- lib/questionlib.php | 18 ++-- question/type/numerical/questiontype.php | 33 ++----- question/type/questiontype.php | 35 ++++---- question/type/shortanswer/questiontype.php | 100 ++++++--------------- 4 files changed, 63 insertions(+), 123 deletions(-) diff --git a/lib/questionlib.php b/lib/questionlib.php index 7d19b899bc..8d4d0dd041 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -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; diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php index a8e8427a36..70fb65d14a 100644 --- a/question/type/numerical/questiontype.php +++ b/question/type/numerical/questiontype.php @@ -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)) { diff --git a/question/type/questiontype.php b/question/type/questiontype.php index df5cdbdd7e..2e005847d2 100644 --- a/question/type/questiontype.php +++ b/question/type/questiontype.php @@ -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; diff --git a/question/type/shortanswer/questiontype.php b/question/type/shortanswer/questiontype.php index 92f810492e..061a6c26c6 100644 --- a/question/type/shortanswer/questiontype.php +++ b/question/type/shortanswer/questiontype.php @@ -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) { - - $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; + 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; } } - - // 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; + return false; } - 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) { + return $this->compare_string_with_wildcard($state->responses[''], + $answer->answer, !$question->options->usecase); } - 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; + function compare_string_with_wildcard($string, $pattern, $ignorecase) { + // Break the string on non-escaped asterisks. + $bits = preg_split('/(?question = $new_question_id; $shortanswer->answers = backup_todb($sho_info['#']['ANSWERS']['0']['#']); $shortanswer->usecase = backup_todb($sho_info['#']['USECASE']['0']['#']); -- 2.39.5