From 795bee3460335511168b44f11444a275f42dbdcb Mon Sep 17 00:00:00 2001 From: nicolasconnault Date: Tue, 13 Nov 2007 09:08:43 +0000 Subject: [PATCH] Managed to remove static calls from the core gradebook classes without removing support for such calls in gradebook interface code. I used a singleton pattern for this (get_instance in grade_object). --- lib/grade/grade_category.php | 6 ++- lib/grade/grade_item.php | 6 ++- lib/grade/grade_object.php | 48 +++++++++++++++++----- lib/grade/simpletest/testgradecategory.php | 13 +++--- lib/simpletest/fixtures/gradetest.php | 12 ++++++ 5 files changed, 63 insertions(+), 22 deletions(-) diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 815999d10a..3984c1b28b 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -170,7 +170,8 @@ class grade_category extends grade_object { * @return object grade_category instance or false if none found. */ function fetch($params) { - return $this->fetch_helper('grade_categories', 'grade_category', $params); + $obj = grade_object::get_instance('grade_category'); + return $obj->fetch_helper('grade_categories', 'grade_category', $params); } /** @@ -181,7 +182,8 @@ class grade_category extends grade_object { * @return array array of grade_category insatnces or false if none found. */ function fetch_all($params) { - return $this->fetch_all_helper('grade_categories', 'grade_category', $params); + $obj = grade_object::get_instance('grade_category'); + return $obj->fetch_all_helper('grade_categories', 'grade_category', $params); } /** diff --git a/lib/grade/grade_item.php b/lib/grade/grade_item.php index a0ecfd7e02..36076cb86c 100644 --- a/lib/grade/grade_item.php +++ b/lib/grade/grade_item.php @@ -310,7 +310,8 @@ class grade_item extends grade_object { * @return object grade_item instance or false if none found. */ function fetch($params) { - return $this->fetch_helper('grade_items', 'grade_item', $params); + $obj = grade_object::get_instance('grade_item'); + return $obj->fetch_helper('grade_items', 'grade_item', $params); } /** @@ -321,7 +322,8 @@ class grade_item extends grade_object { * @return array array of grade_item insatnces or false if none found. */ function fetch_all($params) { - return $this->fetch_all_helper('grade_items', 'grade_item', $params); + $obj = grade_object::get_instance('grade_item'); + return $obj->fetch_all_helper('grade_items', 'grade_item', $params); } /** diff --git a/lib/grade/grade_object.php b/lib/grade/grade_object.php index 79b1379c25..bfec6927f6 100644 --- a/lib/grade/grade_object.php +++ b/lib/grade/grade_object.php @@ -360,20 +360,46 @@ class grade_object { * The following function is a helper for making the code more testable. It allows * unit tests to mock the objects instantiated and used within method bodies. If no params are given, * a static instance is returned. + * @param string $class + * @param array $params + * @param bool $fetch + * @param bool $set Whether or not to set the instance instead of getting it */ - function get_instance($class, $params=NULL, $fetch=true) { + function get_instance($class, $params=NULL, $fetch=true, $set=false, $object=null) { + static $grade_instances = array(); $classes = array('grade_item', 'grade_category', 'grade_grade', 'grade_outcome', 'grade_scale'); - if (!in_array($class, $classes)) { - return false; - } elseif (is_null($params)) { - static $grade_instances = array(); - if (!array_key_exists($class, $grade_instances)) { - $grade_instances[$class] =& new $class; + + if ($set) { + if (!is_object($object)) { + debugging('grade_object::set_instance was given a non-object as parameter!'); + return false; + } + + if (in_array($class, $classes)) { + // Only set the static instance if it isn't already a mock object + if (!isset($grade_instances[$class]) || get_class($grade_instances[$class]) != get_class($object)) { + $grade_instances[$class] =& $object; + return true; + } else { + return false; + } + } else { + debugging("grade_object::set_instance was given an object that is not supported ($class)!"); + return false; } - $instance =& $grade_instances[$class]; - return $instance; - } else { - return new $class($params, $fetch); + + } else { + if (!in_array($class, $classes)) { + return false; + } elseif (is_null($params)) { + if (!array_key_exists($class, $grade_instances)) { + $grade_instances[$class] =& new $class; + } + $instance =& $grade_instances[$class]; + return $instance; + } else { + return new $class($params, $fetch); + } } } } diff --git a/lib/grade/simpletest/testgradecategory.php b/lib/grade/simpletest/testgradecategory.php index d165a2aa24..c06b15a976 100755 --- a/lib/grade/simpletest/testgradecategory.php +++ b/lib/grade/simpletest/testgradecategory.php @@ -69,10 +69,9 @@ class grade_category_test extends grade_test { $grade_category->lib_wrapper = new mock_lib_wrapper(); $this->assertTrue(method_exists($grade_category, 'fetch')); - $grade_category->lib_wrapper->expectOnce('get_records_select'); - $grade_category->lib_wrapper->setReturnValue('get_records_select', array($this->grade_categories[1])); - - $grade_category = $grade_category->fetch(array('id'=>$this->grade_categories[1]->id)); + $obj = grade_object::get_instance('grade_category'); + $obj->setReturnValue('fetch', $this->grade_categories[1]); + $grade_category = $obj->fetch(array('id'=>$this->grade_categories[1]->id)); $this->assertEqual($this->grade_categories[1]->id, $grade_category->id); $this->assertEqual($this->grade_categories[1]->fullname, $grade_category->fullname); } @@ -82,10 +81,10 @@ class grade_category_test extends grade_test { $grade_category->lib_wrapper = new mock_lib_wrapper(); $this->assertTrue(method_exists($grade_category, 'fetch_all')); - $grade_category->lib_wrapper->expectOnce('get_records_select'); - $grade_category->lib_wrapper->setReturnValue('get_records_select', $this->grade_categories); + $obj = grade_object::get_instance('grade_category'); + $obj->setReturnValue('fetch_all', $this->grade_categories); - $grade_categories = $grade_category->fetch_all(array('courseid'=>$this->courseid)); + $grade_categories = $obj->fetch_all(array('courseid'=>$this->courseid)); $this->assertEqual(count($this->grade_categories), count($grade_categories)); } diff --git a/lib/simpletest/fixtures/gradetest.php b/lib/simpletest/fixtures/gradetest.php index 53ad417e24..745ff9a5c2 100644 --- a/lib/simpletest/fixtures/gradetest.php +++ b/lib/simpletest/fixtures/gradetest.php @@ -46,6 +46,13 @@ Mock::generate('grade_lib_wrapper', 'mock_lib_wrapper'); Mock::generate('ADODB_' . $CFG->dbtype, 'mock_db'); Mock::generate('ADORecordSet_' . $CFG->dbtype, 'mock_rs'); +// Prepare partial mocks for the static grade object instances +Mock::generatePartial('grade_category', 'mock_grade_category_partial', array('fetch', 'fetch_all')); +Mock::generatePartial('grade_item', 'mock_grade_item_partial', array('fetch', 'fetch_all')); +Mock::generatePartial('grade_grade', 'mock_grade_grade_partial', array('fetch', 'fetch_all')); +Mock::generatePartial('grade_outcome', 'mock_grade_outcome_partial', array('fetch', 'fetch_all')); +Mock::generatePartial('grade_scale', 'mock_grade_scale_partial', array('fetch', 'fetch_all')); + /** * Here is a brief explanation of the test data set up in these unit tests. * category1 => array(category2 => array(grade_item1, grade_item2), category3 => array(grade_item3)) @@ -106,6 +113,11 @@ class grade_test extends UnitTestCase { $CFG->disablegradehistory = false; $this->real_db = fullclone($db); // $this->reset_mocks(); + grade_object::get_instance('grade_item', null, false, true, new mock_grade_item_partial($this)); + grade_object::get_instance('grade_category', null, false, true, new mock_grade_category_partial($this)); + grade_object::get_instance('grade_grade', null, false, true, new mock_grade_grade_partial($this)); + grade_object::get_instance('grade_outcome', null, false, true, new mock_grade_outcome_partial($this)); + grade_object::get_instance('grade_scale', null, false, true, new mock_grade_scale_partial($this)); } /** -- 2.39.5