Fixed a bug caused by incorrect comparison of float values

diff --git a/mod/workshop/allocation/random/lib.php b/mod/workshop/allocation/random/lib.php
index 16f17eb68fa701c7a397b74c33396112a9236552..497c8bd71b2818836fade8b17ebc610f4fc7bbe0 100644 (file)
--- a/mod/workshop/allocation/random/lib.php
+++ b/mod/workshop/allocation/random/lib.php
@@ -426,7 +426,7 @@ class workshop_random_allocator implements workshop_allocator {
                         $targetgroup = $circlegroupid;
                     } elseif (VISIBLEGROUPS == $gmode) {
                         $trygroups = array_diff_key($squaregroupsworkload, array(0 => null));   // all but [0]
-                        $trygroups = array_diff_key($trygroups, array_flip($failedgroups));     // withou previous failures
+                        $trygroups = array_diff_key($trygroups, array_flip($failedgroups));     // without previous failures
                         $targetgroup = $this->get_element_with_lowest_workload($trygroups);
                     }
                     if ($targetgroup === false) {
@@ -516,11 +516,18 @@ class workshop_random_allocator implements workshop_allocator {
      * @return mixed int|bool id of the selected element or false if it is impossible to choose
      */
     protected function get_element_with_lowest_workload($workload) {
+        $precision = 10;
+
         if (empty($workload)) {
             return false;
         }
-        $minload = min($workload);
-        $minkeys = array_filter($workload, create_function('$val', 'return $val == ' . $minload . ';'));
+        $minload = round(min($workload), $precision);
+        $minkeys = array();
+        foreach ($workload as $key => $val) {
+            if (round($val, $precision) == $minload) {
+                $minkeys[$key] = $val;
+            }
+        }
         return array_rand($minkeys);
     }
 

diff --git a/mod/workshop/allocation/random/simpletest/testallocator.php b/mod/workshop/allocation/random/simpletest/testallocator.php
index c3451fffaf89bc9227f10068dd97029bed6789ac..c72d05ebeccb04293a6410a8b9c27a811bce1e4a 100644 (file)
--- a/mod/workshop/allocation/random/simpletest/testallocator.php
+++ b/mod/workshop/allocation/random/simpletest/testallocator.php
@@ -261,7 +261,7 @@ class workshop_allocation_random_test extends UnitTestCase {
         for ($i = 0; $i < 100; $i++) {
             $chosen = $this->allocator->get_element_with_lowest_workload($workload);
             if (!in_array($chosen, array(4, 9, 10))) {
-                $this->fail('Invalid element chosen');
+                $this->fail('Invalid element ' . var_export($chosen, true) . ' chosen');
                 break;
             } else {
                 $counts[$this->allocator->get_element_with_lowest_workload($workload)]++;
@@ -270,6 +270,31 @@ class workshop_allocation_random_test extends UnitTestCase {
         $this->assertTrue(($counts[9] > 0) && ($counts[10] > 0));
     }
 
+    /**
+     * Floats should be rounded before they are compared
+     *
+     * This should test
+     */
+    public function test_get_element_with_lowest_workload_random_floats() {
+        // fixture setup
+        $workload = array(1 => 1/13, 2 => 0.0769230769231); // should be considered as the same value
+        // exercise SUT
+        $elements = $this->allocator->get_element_with_lowest_workload($workload);
+        // verify
+        $counts = array(1 => 0, 2 => 0);
+        for ($i = 0; $i < 100; $i++) {
+            $chosen = $this->allocator->get_element_with_lowest_workload($workload);
+            if (!in_array($chosen, array(1, 2))) {
+                $this->fail('Invalid element ' . var_export($chosen, true) . ' chosen');
+                break;
+            } else {
+                $counts[$this->allocator->get_element_with_lowest_workload($workload)]++;
+            }
+        }
+        $this->assertTrue(($counts[1] > 0) && ($counts[2] > 0));
+
+    }
+
     /**
      * Filter new assessments so they do not contain existing
      */