Skip to content

Commit

Permalink
Fix various bits of coding style, add missing format_string
Browse files Browse the repository at this point in the history 
Also, improve some comments.
  • Loading branch information
@timhunt
timhunt committed May 2, 2024
1 parent 64dd6f7 commit 9b82705
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 77 deletions.
2 changes: 1 addition & 1 deletion classes/privacy/provider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class provider implements
* @param collection $collection The initialised collection to add items to.
* @return collection A listing of user data stored through this system.
*/
public static function get_metadata(collection $collection) : collection {
public static function get_metadata(collection $collection): collection {
$collection->add_user_preference('qtype_oumatrix_defaultmark', 'privacy:preference:defaultmark');
$collection->add_user_preference('qtype_oumatrix_penalty', 'privacy:preference:penalty');
$collection->add_user_preference('qtype_oumatrix_inputtype', 'privacy:preference:inputtype');
Expand Down
57 changes: 24 additions & 33 deletions question.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,13 @@ abstract class qtype_oumatrix_base extends question_graded_automatically {
public $incorrectfeedback;
public $incorrectfeedbackformat;

/** @var column[] The columns (answers) object. */
/** @var column[] The columns (answers) object, indexed by number. */
public $columns;

/** @var row[] The rows (subquestions) object. */
/** @var row[] The rows (subquestions) object, indexed by number. */
public $rows;

/** @var int The number of columns. */
public $numcolumns;

/** @var int The number of rows. */
public $numrows;

/** @var array The order of the rows. */
/** @var array The order of the rows, key => row number. */
protected $roworder = null;

/**
Expand Down Expand Up @@ -89,6 +83,7 @@ public function apply_attempt_state(question_attempt_step $step) {

/**
* Returns the roworder of the question being displayed.
*
* @param question_attempt $qa
* @return array|null
*/
Expand Down Expand Up @@ -413,25 +408,23 @@ public function get_num_grade_allornone(array $response): array {
foreach ($this->roworder as $rowkey => $rownumber) {
$row = $this->rows[$rownumber];
$rowrightresponse = 0;
if (isset($row->correctanswers)) {
foreach ($this->columns as $column) {
$reponsekey = $this->field($rowkey, $column->number);
if (array_key_exists($reponsekey, $response)) {
if (array_key_exists($column->number, $row->correctanswers)) {
// Add to the count of correct responses.
$rowrightresponse++;
} else {
// Check if there are too many responses selected.
// Then set it to -1 so marks are not allotted for it.
$rowrightresponse = -1;
break;
}
foreach ($this->columns as $column) {
$reponsekey = $this->field($rowkey, $column->number);
if (array_key_exists($reponsekey, $response)) {
if (array_key_exists($column->number, $row->correctanswers)) {
// Add to the count of correct responses.
$rowrightresponse++;
} else {
// Check if there are too many responses selected.
// Then set it to -1 so marks are not allotted for it.
$rowrightresponse = -1;
break;
}
}
// Check if the row has the correct response.
if ($rowrightresponse == count($row->correctanswers)) {
$numright++;
}
}
// Check if the row has the correct response.
if ($rowrightresponse == count($row->correctanswers)) {
$numright++;
}
}
return [$numright, count($this->rows)];
Expand All @@ -448,15 +441,13 @@ public function get_num_parts_grade_partial(array $response): array {
$rightresponse = 0;
foreach ($this->roworder as $rowkey => $rownumber) {
$row = $this->rows[$rownumber];
if ($row->correctanswers != '' ) {
foreach ($this->columns as $column) {
$reponsekey = $this->field($rowkey, $column->number);
if (array_key_exists($reponsekey, $response) && array_key_exists($column->number, $row->correctanswers)) {
$rightresponse++;
}
foreach ($this->columns as $column) {
$reponsekey = $this->field($rowkey, $column->number);
if (array_key_exists($reponsekey, $response) && array_key_exists($column->number, $row->correctanswers)) {
$rightresponse++;
}
$numcorrectanswers += count($row->correctanswers);
}
$numcorrectanswers += count($row->correctanswers);
}
return [$rightresponse, $numcorrectanswers];
}
Expand Down
9 changes: 4 additions & 5 deletions renderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ public function matrix_table(question_attempt $qa, question_display_options $opt
$table .= html_writer::tag('th', '', ['scope' => 'col', 'class' => 'subquestion']);
$index = 0;
foreach ($question->columns as $value) {
$colname[$index] = $value->name;
$table .= html_writer::tag('th', html_writer::span($colname[$index], 'answer_col', ['id' => 'col' . $index]),
$table .= html_writer::tag('th', html_writer::span(format_string($value->name), 'answer_col', ['id' => 'col' . $index]),
['scope' => 'col', 'class' => 'align-middle text-center']);
$index += 1;
}
Expand All @@ -165,16 +164,16 @@ public function matrix_table(question_attempt $qa, question_display_options $opt

// Adding table rows for the sub-questions.
foreach ($question->get_order($qa) as $rowkey => $rowid) {

$row = $question->rows[$rowid];
$rowname = $row->name;
$rownewid = 'row'. $rowkey;
$feedback = '';

$table .= html_writer::start_tag('tr');
$table .= html_writer::tag('th', html_writer::span($rowname, '', ['id' => $rownewid]),
$table .= html_writer::tag('th', html_writer::span(format_string($row->name), '', ['id' => $rownewid]),
['class' => 'subquestion align-middle', 'scope' => 'row']);

for ($c = 1; $c <= count($colname); $c++) {
for ($c = 1; $c <= count($question->columns); $c++) {
$inputattributes['name'] = $this->get_input_name($qa, $rowkey, $c);
$inputattributes['value'] = $this->get_input_value($c);
$inputattributes['id'] = $this->get_input_id($qa, $rowkey, $c);
Expand Down
4 changes: 2 additions & 2 deletions tests/behat/backup_and_restore.feature
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
@qtype @qtype_oumatrix
Feature: Test duplicating a quiz containing a Numerical question
Feature: Test duplicating a quiz containing an OU matrix question
As a teacher
In order re-use my courses containing Numerical questions
In order re-use my courses containing an OU matrix questions
I need to be able to backup and restore them

Background:
Expand Down
38 changes: 19 additions & 19 deletions tests/helper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function get_oumatrix_question_data_animals_single(): stdClass {
'name' => 'Birds',
],
14 => (object) [
'id' => 13,
'id' => 14,
'number' => 4,
'name' => 'Mammals',
],
Expand Down Expand Up @@ -481,20 +481,20 @@ public function make_oumatrix_question_animals_single(): qtype_oumatrix_single {
$question->shownumcorrect = 1;

$question->columns = [
11 => new column($question->id, 1, 'Insects', 11),
12 => new column($question->id, 2, 'Fish', 12),
13 => new column($question->id, 3, 'Birds', 13),
14 => new column($question->id, 4, 'Mammals', 14),
1 => new column($question->id, 1, 'Insects', 11),
2 => new column($question->id, 2, 'Fish', 12),
3 => new column($question->id, 3, 'Birds', 13),
4 => new column($question->id, 4, 'Mammals', 14),
];

$question->rows = [
11 => new row(11, $question->id, 1, 'Bee', [1 => '1'],
1 => new row(11, $question->id, 1, 'Bee', [1 => '1'],
'Flies and Bees are insects.', FORMAT_HTML),
12 => new row(12, $question->id, 2, 'Salmon', [2 => '1'],
2 => new row(12, $question->id, 2, 'Salmon', [2 => '1'],
'Cod, Salmon and Trout are fish.', FORMAT_HTML),
13 => new row(13, $question->id, 3, 'Seagull', [3 => '1'],
3 => new row(13, $question->id, 3, 'Seagull', [3 => '1'],
'Gulls and Owls are birds.', FORMAT_HTML),
14 => new row(14, $question->id, 4, 'Dog', [4 => '1'],
4 => new row(14, $question->id, 4, 'Dog', [4 => '1'],
'Cows, Dogs and Horses are mammals.', FORMAT_HTML),
];

Expand Down Expand Up @@ -538,21 +538,21 @@ public function make_oumatrix_question_food_multiple(): qtype_oumatrix_multiple
$question->shownumcorrect = 1;

$question->columns = [
21 => new column($question->id, 1, 'Chicken breast', 21),
22 => new column($question->id, 2, 'Carrot', 22),
23 => new column($question->id, 3, 'Salmon fillet', 23),
24 => new column($question->id, 4, 'Asparagus', 24),
25 => new column($question->id, 5, 'Olive oil', 25),
26 => new column($question->id, 6, 'Steak', 26),
27 => new column($question->id, 7, 'Potato', 27),
1 => new column($question->id, 1, 'Chicken breast', 21),
2 => new column($question->id, 2, 'Carrot', 22),
3 => new column($question->id, 3, 'Salmon fillet', 23),
4 => new column($question->id, 4, 'Asparagus', 24),
5 => new column($question->id, 5, 'Olive oil', 25),
6 => new column($question->id, 6, 'Steak', 26),
7 => new column($question->id, 7, 'Potato', 27),
];

$question->rows = [
21 => new row(21, $question->id, 1, 'Proteins', [1 => '1', 3 => '1', 6 => '1'],
1 => new row(21, $question->id, 1, 'Proteins', [1 => '1', 3 => '1', 6 => '1'],
'Chicken, fish and red meat containing proteins.', FORMAT_HTML),
22 => new row(22, $question->id, 2, 'Vegetables', [2 => '1', 4 => '1', 7 => '1'],
2 => new row(22, $question->id, 2, 'Vegetables', [2 => '1', 4 => '1', 7 => '1'],
'Carrot, Asparagus, Potato are vegetables.', FORMAT_HTML),
23 => new row(23, $question->id, 3, 'Fats', [5 => '1'],
3 => new row(23, $question->id, 3, 'Fats', [5 => '1'],
'Olive oil contains fat.', FORMAT_HTML),
];

Expand Down
34 changes: 17 additions & 17 deletions tests/question_single_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ public function test_update_attempt_state_date_from_old_version_bad(): void {
$newq = clone($q);
$newq->id = 456;
$newq->columns = [
21 => new column($newq->id, 1, 'Insects', 21),
22 => new column($newq->id, 2, 'Fish', 22),
23 => new column($newq->id, 3, 'Birds', 23),
1 => new column($newq->id, 1, 'Insects', 21),
2 => new column($newq->id, 2, 'Fish', 22),
3 => new column($newq->id, 3, 'Birds', 23),
];

$oldstep = new question_attempt_step();
Expand All @@ -258,26 +258,26 @@ public function test_update_attempt_state_date_from_old_version_ok(): void {
$newq = clone($q);
$newq->id = 456;
$newq->columns = [
21 => new column($newq->id, 1, 'Insects', 21),
22 => new column($newq->id, 2, 'Fish', 22),
23 => new column($newq->id, 3, 'Birds', 23),
24 => new column($newq->id, 4, 'Mammals', 24),
1 => new column($newq->id, 1, 'Insects', 21),
2 => new column($newq->id, 2, 'Fish', 22),
3 => new column($newq->id, 3, 'Birds', 23),
4 => new column($newq->id, 4, 'Mammals', 24),
];

$newq->rows = [
21 => new row(21, $newq->id, 1, 'Bee', [1 => '1'],
1 => new row(21, $newq->id, 1, 'Bee', [1 => '1'],
'Fly, Bee and spider are insects.', FORMAT_HTML),
22 => new row(22, $newq->id, 2, 'Salmon', [2 => '1'],
2 => new row(22, $newq->id, 2, 'Salmon', [2 => '1'],
'Cod, Salmon and Trout are fish.', FORMAT_HTML),
23 => new row(23, $newq->id, 3, 'Seagull', [3 => '1'],
3 => new row(23, $newq->id, 3, 'Seagull', [3 => '1'],
'Gulls and Owls are birds.', FORMAT_HTML),
24 => new row(24, $newq->id, 4, 'Dog', [4 => '1'],
4 => new row(24, $newq->id, 4, 'Dog', [4 => '1'],
'Cow, Dog and Horse are mammals.', FORMAT_HTML),
];

$oldstep = new question_attempt_step();
$oldstep->set_qt_var('_roworder', '12,23,11,14');
$this->assertEquals(['_roworder' => '22,23,21,24'],
$oldstep->set_qt_var('_roworder', '2,3,1,4');
$this->assertEquals(['_roworder' => '2,3,1,4'],
$newq->update_attempt_state_data_for_new_version($oldstep, $q));
}

Expand All @@ -288,19 +288,19 @@ public function test_has_specific_feedback(): void {
$this->assertTrue($q->has_specific_feedback());

// First row does not have feedback text.
$q->rows[11]->feedback = '';
$q->rows[1]->feedback = '';
$this->assertTrue($q->has_specific_feedback());

// First and second rows do not have feedback text.
$q->rows[12]->feedback = '';
$q->rows[2]->feedback = '';
$this->assertTrue($q->has_specific_feedback());

// First, second and third rows do not have feedback text.
$q->rows[13]->feedback = '';
$q->rows[3]->feedback = '';
$this->assertTrue($q->has_specific_feedback());

// All rows do not have feedback text.
$q->rows[14]->feedback = '';
$q->rows[4]->feedback = '';
$this->assertFalse($q->has_specific_feedback());
}
}

0 comments on commit 9b82705

Please sign in to comment.