Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge branch 'MDL-13629_droplow_22' of git://github.com/andyjdavis/mo…

…odle into MOODLE_22_STABLE
  • Loading branch information...
commit 60f519014979460ef645600898afd984ce6441c7 2 parents a875f82 + 90fa858
Dan Poltawski danpoltawski authored
60 lib/grade/grade_category.php
View
@@ -884,22 +884,64 @@ public function apply_limit_rules(&$grade_values, $items) {
asort($grade_values, SORT_NUMERIC);
$dropped = 0;
- foreach ($grade_values as $itemid=>$value) {
+ // If we have fewer grade items available to drop than $this->droplow, use this flag to escape the loop
+ // May occur because of "extra credit" or if droplow is higher than the number of grade items
+ $droppedsomething = true;
- if ($dropped < $this->droplow) {
+ while ($dropped < $this->droplow && $droppedsomething) {
+ $droppedsomething = false;
- if ($extraused and $items[$itemid]->aggregationcoef > 0) {
- // no drop low for extra credits
+ $grade_keys = array_keys($grade_values);
+ if (count($grade_keys) === 0) {
+ //We've dropped all grade items
+ break;
+ }
- } else {
- unset($grade_values[$itemid]);
- $dropped++;
+ $originalindex = $founditemid = $foundmax = null;
+
+ // Find the first remaining grade item that is available to be dropped
+ foreach ($grade_keys as $gradekeyindex=>$gradekey) {
+ if (!$extraused || $items[$gradekey]->aggregationcoef <= 0) {
+ // Found a non-extra credit grade item that is eligible to be dropped
+ $originalindex = $gradekeyindex;
+ $founditemid = $grade_keys[$originalindex];
+ $foundmax = $items[$founditemid]->grademax;
+ break;
}
+ }
- } else {
- // we have dropped enough
+ if (empty($founditemid)) {
+ // No grade items available to drop
break;
}
+
+ $i = 1;
+ while ($originalindex+$i < count($grade_keys)) {
+ $possibleitemid = $grade_keys[$originalindex+$i];
+ if ($grade_values[$founditemid] != $grade_values[$possibleitemid]) {
+ // The next grade item has a different grade. Stop looking.
+ break;
+ }
+
+ if ($extraused && $items[$possibleitemid]->aggregationcoef > 0) {
+ // Don't drop extra credit grade items. Continue the search.
+ continue;
+ }
+
+ if ($foundmax < $items[$possibleitemid]->grademax) {
+ // Found a grade item with the same grade and a higher grademax
+ $foundmax = $items[$possibleitemid]->grademax;
+ $founditemid = $possibleitemid;
+ // Continue searching to see if there is an even higher grademax...
+ }
+
+ $i++;
+ }
+
+ // Now drop whatever grade item we have found
+ unset($grade_values[$founditemid]);
+ $dropped++;
+ $droppedsomething = true;
}
} else if (!empty($this->keephigh)) {
65 lib/grade/simpletest/testgradecategory.php
View
@@ -429,6 +429,7 @@ function sub_test_grade_category_apply_limit_rules() {
$items[$this->grade_items[2]->id] = new grade_item($this->grade_items[2], false);
$items[$this->grade_items[4]->id] = new grade_item($this->grade_items[4], false);
+ // Test excluding the lowest 2 out of 4 grades from aggregation with no 0 grades
$category = new grade_category();
$category->droplow = 2;
$grades = array($this->grade_items[0]->id=>5.374,
@@ -440,6 +441,7 @@ function sub_test_grade_category_apply_limit_rules() {
$this->assertEqual($grades[$this->grade_items[1]->id], 9.4743);
$this->assertEqual($grades[$this->grade_items[4]->id], 7.3754);
+ // Test aggregating only the highest 1 out of 4 grades
$category = new grade_category();
$category->keephigh = 1;
$category->droplow = 0;
@@ -452,25 +454,28 @@ function sub_test_grade_category_apply_limit_rules() {
$grade = reset($grades);
$this->assertEqual(9.4743, $grade);
+ // Test excluding the lowest 2 out of 4 grades from aggregation with no 0 grades
+ // An extra credit grade item should be kept even if droplow means it would otherwise be excluded
$category = new grade_category();
$category->droplow = 2;
$category->aggregation = GRADE_AGGREGATE_SUM;
- $items[$this->grade_items[2]->id]->aggregationcoef = 1;
+ $items[$this->grade_items[2]->id]->aggregationcoef = 1; // Mark grade item 2 as "extra credit"
$grades = array($this->grade_items[0]->id=>5.374,
$this->grade_items[1]->id=>9.4743,
$this->grade_items[2]->id=>2.5474,
$this->grade_items[4]->id=>7.3754);
-
$category->apply_limit_rules($grades, $items);
$this->assertEqual(count($grades), 2);
$this->assertEqual($grades[$this->grade_items[1]->id], 9.4743);
$this->assertEqual($grades[$this->grade_items[2]->id], 2.5474);
+ // Test only aggregating the highest 1 out of 4 grades
+ // An extra credit grade item is retained in addition to the highest grade
$category = new grade_category();
$category->keephigh = 1;
$category->droplow = 0;
$category->aggregation = GRADE_AGGREGATE_SUM;
- $items[$this->grade_items[2]->id]->aggregationcoef = 1;
+ $items[$this->grade_items[2]->id]->aggregationcoef = 1; // Mark grade item 2 as "extra credit"
$grades = array($this->grade_items[0]->id=>5.374,
$this->grade_items[1]->id=>9.4743,
$this->grade_items[2]->id=>2.5474,
@@ -479,6 +484,60 @@ function sub_test_grade_category_apply_limit_rules() {
$this->assertEqual(count($grades), 2);
$this->assertEqual($grades[$this->grade_items[1]->id], 9.4743);
$this->assertEqual($grades[$this->grade_items[2]->id], 2.5474);
+
+
+ // Test excluding the lowest 1 out of 4 grades from aggregation with two 0 grades
+ $items[$this->grade_items[2]->id]->aggregationcoef = 0; // Undo marking grade item 2 as "extra credit"
+ $category = new grade_category();
+ $category->droplow = 1;
+ $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN2; // simple weighted mean
+ $grades = array($this->grade_items[0]->id=>0, // 0 out of 110. Should be excluded from aggregation.
+ $this->grade_items[1]->id=>5, // 5 out of 100
+ $this->grade_items[2]->id=>2, // 0 out of 6
+ $this->grade_items[4]->id=>0); // 0 out of 100
+ $category->apply_limit_rules($grades, $items);
+ $this->assertEqual(count($grades), 3);
+ $this->assertEqual($grades[$this->grade_items[1]->id], 5);
+ $this->assertEqual($grades[$this->grade_items[2]->id], 2);
+ $this->assertEqual($grades[$this->grade_items[4]->id], 0);
+
+ // Test excluding the lowest 2 out of 4 grades from aggregation with three 0 grades
+ $category = new grade_category();
+ $category->droplow = 2;
+ $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN2; // simple weighted mean
+ $grades = array($this->grade_items[0]->id=>0, // 0 out of 110. Should be excluded from aggregation.
+ $this->grade_items[1]->id=>5, // 5 out of 100
+ $this->grade_items[2]->id=>0, // 0 out of 6
+ $this->grade_items[4]->id=>0); // 0 out of 100. Should be excluded from aggregation.
+ $category->apply_limit_rules($grades, $items);
+ $this->assertEqual(count($grades), 2);
+ $this->assertEqual($grades[$this->grade_items[1]->id], 5);
+ $this->assertEqual($grades[$this->grade_items[2]->id], 0);
+
+ // Test excluding the lowest 5 out of 4 grades from aggregation
+ // Just to check we handle this sensibly
+ $category = new grade_category();
+ $category->droplow = 5;
+ $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN2; // simple weighted mean
+ $grades = array($this->grade_items[0]->id=>0, // 0 out of 110. Should be excluded from aggregation.
+ $this->grade_items[1]->id=>5, // 5 out of 100
+ $this->grade_items[2]->id=>6, // 6 out of 6
+ $this->grade_items[4]->id=>1);// 1 out of 100. Should be excluded from aggregation.
+ $category->apply_limit_rules($grades, $items);
+ $this->assertEqual(count($grades), 0);
+
+ // Test excluding the lowest 4 out of 4 grades from aggregation with one marked as extra credit
+ $category = new grade_category();
+ $category->droplow = 4;
+ $category->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN2; // simple weighted mean
+ $items[$this->grade_items[2]->id]->aggregationcoef = 1; // Mark grade item 2 as "extra credit"
+ $grades = array($this->grade_items[0]->id=>0, // 0 out of 110. Should be excluded from aggregation.
+ $this->grade_items[1]->id=>5, // 5 out of 100. Should be excluded from aggregation.
+ $this->grade_items[2]->id=>6, // 6 out of 6. Extra credit. Should be retained.
+ $this->grade_items[4]->id=>1);// 1 out of 100. Should be excluded from aggregation.
+ $category->apply_limit_rules($grades, $items);
+ $this->assertEqual(count($grades), 1);
+ $this->assertEqual($grades[$this->grade_items[2]->id], 6);
}
/**
Please sign in to comment.
Something went wrong with that request. Please try again.