Skip to content

Commit 8c25cdd

Browse files
mkassaeimkassaei
committed
Tidy up questiontype.php and question.php #834537
1 parent e8320d8 commit 8c25cdd

File tree

7 files changed

+489
-145
lines changed

7 files changed

+489
-145
lines changed

‎question.php‎

Lines changed: 124 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class qtype_drawlines_question extends question_graded_automatically {
5959
public function get_expected_data() {
6060
$expecteddata = [];
6161
foreach ($this->lines as $line) {
62-
$expecteddata[$this->choice($line->number - 1)] = PARAM_RAW;
62+
$expecteddata[$this->field($line->number - 1)] = PARAM_RAW;
6363
}
6464
return $expecteddata;
6565
}
@@ -75,8 +75,8 @@ public function is_complete_response(array $response): bool {
7575
return false;
7676
}
7777
foreach ($this->lines as $key => $line) {
78-
if (isset($response[$this->choice($key)]) &&
79-
!line::are_response_coordinates_valid($response[$this->choice($key)], $line->type)) {
78+
if (isset($response[$this->field($key)]) &&
79+
!line::are_response_coordinates_valid($response[$this->field($key)], $line->type)) {
8080
return false;
8181
}
8282
}
@@ -87,7 +87,7 @@ public function is_complete_response(array $response): bool {
8787
public function get_correct_response() {
8888
$response = [];
8989
foreach ($this->lines as $key => $line) {
90-
$response[$this->choice($key)] = line::get_coordinates($line->zonestart) . ' '
90+
$response[$this->field($key)] = line::get_coordinates($line->zonestart) . ' '
9191
. line::get_coordinates($line->zoneend);
9292
}
9393
return $response;
@@ -98,14 +98,14 @@ public function summarise_response(array $response): ?string {
9898
$responsewords = [];
9999
$answers = [];
100100
foreach ($this->lines as $key => $line) {
101-
if (array_key_exists($this->choice($key), $response) && $response[$this->choice($key)] != '') {
102-
$coordinates = explode(' ', $response[$this->choice($key)]);
101+
if (array_key_exists($this->field($key), $response) && $response[$this->field($key)] != '') {
102+
$coordinates = explode(' ', $response[$this->field($key)]);
103103
if ($line->type == 'lineinfinite' && count($coordinates) == 4) {
104-
$coordinates = explode(' ', $response[$this->choice($key)]);
104+
$coordinates = explode(' ', $response[$this->field($key)]);
105105
$answers[] = 'Line ' . $line->number . ': ' . $coordinates[1] . ' ' . $coordinates[2];
106106
continue;
107107
}
108-
$answers[] = 'Line ' . $line->number . ': ' . $response[$this->choice($key)];
108+
$answers[] = 'Line ' . $line->number . ': ' . $response[$this->field($key)];
109109
}
110110
}
111111
if (count($answers) > 0) {
@@ -115,9 +115,12 @@ public function summarise_response(array $response): ?string {
115115
}
116116

117117
#[\Override]
118-
public function is_gradable_response(array $response) {
118+
public function is_gradable_response(array $response): bool {
119+
if (!isset($response)) {
120+
return false;
121+
}
119122
foreach ($this->lines as $key => $line) {
120-
if (array_key_exists($this->choice($key), $response)) {
123+
if (array_key_exists($this->field($key), $response)) {
121124
return true;
122125
}
123126
}
@@ -127,75 +130,79 @@ public function is_gradable_response(array $response) {
127130
#[\Override]
128131
public function is_same_response(array $prevresponse, array $newresponse) {
129132
foreach ($this->lines as $key => $line) {
130-
$fieldname = $this->choice($key);
133+
$fieldname = $this->field($key);
131134
if (!question_utils::arrays_same_at_key_missing_is_blank($prevresponse, $newresponse, $fieldname)) {
132135
return false;
133136
}
134137
}
135138
return true;
136139
}
137140

138-
#[\Override]
139-
public function grade_response(array $response): array {
140-
// Retrieve the number of right responses and the total number of responses.
141-
if ($this->grademethod == 'partial') {
142-
[$numright, $total] = $this->get_num_parts_right_grade_partialt($response);
143-
} else {
144-
[$numright, $total] = $this->get_num_parts_right_grade_allornone($response);
145-
}
146-
$fraction = $numright / $total;
147-
return [$fraction, question_state::graded_state_for_fraction($fraction)];
148-
}
149-
150141
/**
151142
* Get the number of correct choices selected in the response, for 'Give partial credit' grade method.
152143
*
153144
* @param array $response The response list.
154145
* @return array The array of number of correct lines (start, end or both points of lines).
155146
*/
156-
public function get_num_parts_right_grade_partialt(array $response): array {
147+
public function get_num_parts_right_grade_partial(array $response): array {
157148
if (!$response) {
158149
return [0, 0];
159150
}
151+
$rightstart = 0;
152+
$rightend = 0;
153+
160154
$numpartright = 0;
155+
$numpartrightarray = [];
161156
foreach ($this->lines as $key => $line) {
162-
if (array_key_exists($this->choice($key), $response) && $response[$this->choice($key)] !== '') {
163-
$coords = explode(' ', $response[$this->choice($key)]);
164-
if ($line->type == 'lineinfinite') {
157+
if (array_key_exists($this->field($key), $response) && $response[$this->field($key)] !== '') {
158+
$coords = explode(' ', $response[$this->field($key)]);
159+
if ($line->type === line::TYPE_LINE_INFINITE) {
165160
if (count($coords) == 2) {
166161
// Response with 2 coordinates (x1,y1 x2,y2).
167162
if (line::is_item_positioned_correctly_on_axis(
168163
$coords[0], $line->zonestart, $line->zoneend, 'start')) {
169-
$numpartright++;
164+
$rightstart++;
170165
}
171166
if (line::is_item_positioned_correctly_on_axis(
172167
$coords[1], $line->zonestart, $line->zoneend, 'end')) {
173-
$numpartright++;
168+
$rightend++;
174169
}
175170
} else {
176171
// Response has 4 coordinates(x1,y1 x2,y2 x3,y3 x4,y4).
177172
// Here we need to consider x2,y2 x3,y3 for calculation.
178173
if (line::is_item_positioned_correctly_on_axis(
179174
$coords[1], $line->zonestart, $line->zoneend, 'start')) {
180-
$numpartright++;
175+
$rightstart++;
181176
}
182177
if (line::is_item_positioned_correctly_on_axis(
183178
$coords[2], $line->zonestart, $line->zoneend, 'end')) {
184-
$numpartright++;
179+
$rightend++;
185180
}
186181
}
187182
} else {
188183
if (line::is_dragitem_in_the_right_place($coords[0], $line->zonestart)) {
189-
$numpartright++;
184+
$numpartrightarray[$line->number]['start'] = 1;
185+
$rightstart++;
186+
} else {
187+
$numpartrightarray[$line->number]['start'] = 0;
190188
}
191189
if (line::is_dragitem_in_the_right_place($coords[1], $line->zoneend)) {
192-
$numpartright++;
190+
$numpartrightarray[$line->number]['end'] = 1;
191+
$rightend++;
192+
} else {
193+
$numpartrightarray[$line->number]['end'] = 0;
194+
193195
}
196+
$numpartright = $rightstart + $rightend;
194197
}
195198
}
196199
}
197-
$total = count($this->lines) * 2;
198-
return [$numpartright, $total];
200+
$numtotal = count($this->lines) * 2;
201+
if (!empty($numpartrightarray)) {
202+
203+
return [$numpartrightarray, $numpartright, $numtotal];
204+
}
205+
return [$numpartright, $numtotal];
199206
}
200207

201208
/**
@@ -210,25 +217,25 @@ public function get_num_parts_right_grade_allornone(array $response): array {
210217
}
211218
$numright = 0;
212219
foreach ($this->lines as $key => $line) {
213-
if (array_key_exists($this->choice($key), $response) && $response[$this->choice($key)] !== '') {
214-
$coords = explode(' ', $response[$this->choice($key)]);
215-
if ($line->type == 'lineinfinite') {
220+
if (array_key_exists($this->field($key), $response) && $response[$this->field($key)] !== '') {
221+
$coords = explode(' ', $response[$this->field($key)]);
222+
if ($line->type == line::TYPE_LINE_INFINITE) {
216223
if (count($coords) == 2) {
217224
// Response with 2 coordinates (x1,y1 x2,y2 x3,y3 x4,y4).
218225
$isstartrightplace = line::is_item_positioned_correctly_on_axis(
219-
$coords[0], $line->zonestart, $line->zoneend, 'start'
226+
$coords[0], $line->zonestart, $line->zoneend, 'start'
220227
);
221228
$isendrightplace = line::is_item_positioned_correctly_on_axis(
222-
$coords[1], $line->zonestart, $line->zoneend, 'end'
229+
$coords[1], $line->zonestart, $line->zoneend, 'end'
223230
);
224231
} else {
225232
// Response has 4 coordinates(x1,y1 x2,y2 x3,y3 x4,y4).
226233
// Here we need to consider x2,y2 x3,y3 for calculation.
227234
$isstartrightplace = line::is_item_positioned_correctly_on_axis(
228-
$coords[1], $line->zonestart, $line->zoneend, 'start'
235+
$coords[1], $line->zonestart, $line->zoneend, 'start'
229236
);
230237
$isendrightplace = line::is_item_positioned_correctly_on_axis(
231-
$coords[2], $line->zonestart, $line->zoneend, 'end'
238+
$coords[2], $line->zonestart, $line->zoneend, 'end'
232239
);
233240
}
234241
if ($isstartrightplace && $isendrightplace) {
@@ -274,15 +281,15 @@ public function get_validation_error(array $response): string {
274281
public function classify_response(array $response) {
275282
$classifiedresponse = [];
276283
foreach ($this->lines as $key => $line) {
277-
if (array_key_exists($this->choice($key), $response) && $response[$this->choice($key)] !== '') {
284+
if (array_key_exists($this->field($key), $response) && $response[$this->field($key)] !== '') {
278285
if ($this->grademethod == 'partial') {
279286
$fraction = 0.5;
280287
} else {
281288
$fraction = 1;
282289
}
283290
$classifiedresponse[$key] = new question_classified_response(
284291
$line->number,
285-
'Line ' . $line->number . ': ' . $response[$this->choice($key)],
292+
'Line ' . $line->number . ': ' . $response[$this->field($key)],
286293
$fraction);
287294
} else {
288295
$classifiedresponse[$key] = question_classified_response::no_response();
@@ -291,6 +298,49 @@ public function classify_response(array $response) {
291298
return $classifiedresponse;
292299
}
293300

301+
#[\Override]
302+
public function grade_response(array $response): array {
303+
// Retrieve the number of right responses and the total number of responses.
304+
[$numright, $numtotal] = $this->retrieve_numright_numtotal($response);
305+
$fraction = $numright / $numtotal;
306+
return [$fraction, question_state::graded_state_for_fraction($fraction)];
307+
}
308+
309+
/**
310+
* Return number of right responses and the total number of answers.
311+
*
312+
* @param array $response The respnse array
313+
* @return array|int[] The array containing number of correct responses and the total.
314+
*/
315+
public function retrieve_numright_numtotal(array $response): array {
316+
if ($this->grademethod == 'partial') {
317+
[$notused, $numright, $numtotal] = $this->get_num_parts_right_grade_partial($response);
318+
} else {
319+
[$numright, $numtotal] = $this->get_num_parts_right_grade_allornone($response);
320+
}
321+
return [$numright, $numtotal];
322+
}
323+
324+
/**
325+
* Return the final garde for the question.
326+
*
327+
* @param array $responses The respnses array
328+
* @return array
329+
*/
330+
public function get_tries_responses(array $responses): array {
331+
$allgrades = [];
332+
foreach ($responses as $trynumber => $response) {
333+
if ($this->grademethod == 'partial') {
334+
[$array, $numpartright, $numtotal] = $this->get_num_parts_right_grade_partial($response);
335+
$allgrades[$trynumber] = $array;
336+
} else {
337+
[$numright, $numtotal] = $this->get_num_parts_right_grade_allornone($response);
338+
$allgrades[$trynumber] = $numright;
339+
}
340+
}
341+
return $allgrades;
342+
}
343+
294344
/**
295345
* Work out a final grade for this attempt, taking into account
296346
* all the tries the student made and return the grade value.
@@ -302,26 +352,47 @@ public function classify_response(array $response) {
302352
* @param int $totaltries The maximum number of tries allowed.
303353
*
304354
* @return float the fraction that should be awarded for this
305-
* sequence of response.
355+
* sequence of responses.
306356
*/
307-
public function compute_final_grade(array $responses, int $totaltries): float {
308-
// TODO: To incorporate the question penalty for interactive with multiple tries behaviour.
309-
357+
public function compute_final_grade(array $responses, int $totaltries): int|float {
358+
$penalties = 0;
310359
$grade = 0;
311-
foreach ($responses as $response) {
360+
$allgrades = $this->get_tries_responses($responses);
361+
; foreach ($responses as $trynumber => $response) {
312362
[$fraction, $state] = $this->grade_response($response);
313-
$grade += $fraction;
363+
if ($state->is_graded() === true) {
364+
if ($totaltries === 1) {
365+
return $fraction;
366+
}
367+
[$numright, $numtotal] = $this->retrieve_numright_numtotal($response);
368+
$grade = max(0, $fraction - $penalties);
369+
if ($state->get_feedback_class() === 'correct') {
370+
return $grade;
371+
}
372+
if ($state->get_feedback_class() === 'incorrect') {
373+
$penalties += $this->penalty;
374+
}
375+
if ($state->get_feedback_class() === 'partiallycorrect') {
376+
$trynumright = 0;
377+
foreach($this->lines as $line) {
378+
$trynumright += $allgrades[$trynumber][$line->number]['start'] +
379+
$allgrades[$trynumber][$line->number]['end'];
380+
}
381+
$partpenaly = (($numtotal - $trynumright) * $this->penalty / $numtotal);
382+
$penalties += min($this->penalty, $partpenaly);
383+
}
384+
}
314385
}
315386
return $grade;
316387
}
317388

318389
/**
319-
* Get a choice identifier
390+
* Get a choice index identifier
320391
*
321-
* @param int $choice stem number
392+
* @param int $choice
322393
* @return string the question-type variable name.
323394
*/
324-
public function choice($choice) {
395+
public function field($choice): string {
325396
return 'c' . $choice;
326397
}
327398
}

‎questiontype.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,6 @@ public function save_hints($fromform, $withparts = false) {
172172
}
173173
}
174174

175-
#[\Override]
176-
protected function make_question_instance($questiondata) {
177-
question_bank::load_question_definition_classes($this->name());
178-
return new qtype_drawlines_question;
179-
}
180-
181175
#[\Override]
182176
protected function initialise_question_instance(question_definition $question, $questiondata): void {
183177
parent::initialise_question_instance($question, $questiondata);

‎renderer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ protected function hidden_field_for_qt_var(question_attempt $qa, $varname, $valu
104104
* @return mixed
105105
*/
106106
protected function hidden_field_choice(question_attempt $qa, $choicenumber, $value = null, $class = null) {
107-
$varname = 'c'. $choicenumber;
107+
$varname = $qa->get_question()->field($choicenumber);
108108
$classes = ['choices', 'choice'. $choicenumber];
109109
[, $html] = $this->hidden_field_for_qt_var($qa, $varname, $value, $classes);
110110
return $html;

‎tests/backup_and_restore_test.php‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
namespace qtype_drawlines;
1818

1919
use question_bank;
20+
use core_question;
21+
use core_question\local\bank\question_edit_contexts;
2022

2123
defined('MOODLE_INTERNAL') || die();
2224

@@ -45,30 +47,27 @@ public function test_restore_create_qtype_drawlines_mkmap_twolines(): void {
4547
// Create a course with one drawlines question in its question bank.
4648
$generator = $this->getDataGenerator();
4749
$course = $generator->create_course();
48-
$contexts = new \core_question\local\bank\question_edit_contexts(\context_course::instance($course->id));
49-
$category = question_make_default_categories($contexts->all());
50+
$qbank = $generator->create_module('qbank', ['course' => $course->id]);
51+
$bankcontext = \context_module::instance($qbank->cmid);
52+
$category = question_get_default_category($bankcontext->id, true);
5053
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
5154
$drawlines = $questiongenerator->create_question('drawlines', 'mkmap_twolines', ['category' => $category->id]);
55+
$drawlines->contextid = $bankcontext->id;
5256

5357
// Do backup and restore the course.
5458
$newcourseid = $this->backup_and_restore($course);
5559

5660
// Verify that the restored question has the extra data such as options, lines.
57-
$contexts = new \core_question\local\bank\question_edit_contexts(\context_course::instance($newcourseid));
58-
$newcategory = question_make_default_categories($contexts->all());
61+
$newcategory = question_get_default_category($bankcontext->id);
62+
5963
$newdrawlines = $DB->get_record_sql('SELECT q.*
6064
FROM {question} q
6165
JOIN {question_versions} qv ON qv.questionid = q.id
6266
JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid
6367
WHERE qbe.questioncategoryid = ?
6468
AND q.qtype = ?', [$newcategory->id, 'drawlines']);
6569

66-
$newdrawlines->options = $DB->get_record('qtype_drawlines_options', ['questionid' => $newdrawlines->id]);
67-
$newdrawlines->lines = $DB->get_records('qtype_drawlines_lines', ['questionid' => $newdrawlines->id]);
68-
6970
$this->assertSame($newcourseid, $course->id + 1);
70-
$this->assertSame((int)$newdrawlines->id, $drawlines->id + 1);
71-
7271
$this->assertTrue($DB->record_exists('question', ['id' => $newdrawlines->id]));
7372
$this->assertTrue($DB->record_exists('qtype_drawlines_options', ['questionid' => $newdrawlines->id]));
7473
$this->assertTrue($DB->record_exists('qtype_drawlines_lines', ['questionid' => $newdrawlines->id]));

0 commit comments

Comments
 (0)