MDL-19077 I just found a way to reduce the coupling between the classes.

diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php
index 27758516e5cb06989ae6bbbf2e637f2a734ee9a8..6ba2456dd10357cd21c871ab8d603d584f6968e2 100644 (file)
--- a/lib/deprecatedlib.php
+++ b/lib/deprecatedlib.php
@@ -1751,16 +1751,17 @@ function make_mod_upload_directory($courseid) {
  * @deprecated Required to make the old $THEME->customcorners setting work.
  */
 class custom_corners_renderer_factory extends standard_renderer_factory {
-    /**
-     * Constructor.
-     * @param object $theme the theme we are rendering for.
-     * @param moodle_page $page the page we are doing output for.
-     */
-    public function __construct($theme, $page) {
+    public function __construct($theme) {
+        parent::__construct($theme);
         global $CFG;
-        parent::__construct($theme, $page);
         require_once($CFG->themedir . '/custom_corners/renderers.php');
-        $this->renderers = array('core' => new custom_corners_core_renderer($this->opencontainers, $this->page, $this));
+    }
+    /* Implement the subclass method. */
+    public function get_renderer($module, $page) {
+        if ($module == 'core') {
+            return new custom_corners_core_renderer($page);
+        }
+        return parent::get_renderer($module, $page);
     }
 }
 

diff --git a/lib/outputlib.php b/lib/outputlib.php
index 246fe3b440d0bc9045ec904dcadc10ec9bcb9361..61f1b303ae27e86efcb12b56166ab66ec64e6384 100644 (file)
--- a/lib/outputlib.php
+++ b/lib/outputlib.php
@@ -60,7 +60,7 @@ interface renderer_factory {
      * @param $module the name of part of moodle. E.g. 'core', 'quiz', 'qtype_multichoice'.
      * @return object an object implementing the requested renderer interface.
      */
-    public function get_renderer($module);
+    public function get_renderer($module, $page);
 }
 
 
@@ -78,50 +78,16 @@ interface renderer_factory {
  * @since     Moodle 2.0
  */
 abstract class renderer_factory_base implements renderer_factory {
-    /** The theme we are rendering for. */
+    /** @var theme_config the theme we belong to. */
     protected $theme;
 
-    /** The page we are doing output for. */
-    protected $page;
-
-    /** Used to cache renderers as they are created. */
-    protected $renderers = array();
-
-    protected $opencontainers;
-
     /**
      * Constructor.
-     * @param object $theme the theme we are rendering for.
-     * @param moodle_page $page the page we are doing output for.
+     * @param theme_config $theme the theme we belong to.
      */
-    public function __construct($theme, $page) {
+    public function __construct($theme) {
         $this->theme = $theme;
-        $this->page = $page;
-        $this->opencontainers = new xhtml_container_stack();
     }
-
-    /* Implement the interface method. */
-    public function get_renderer($module) {
-        // Cache the renderers by module name, and delegate the actual
-        // construction to the create_renderer method.
-        if (!array_key_exists($module, $this->renderers)) {
-            $this->renderers[$module] = $this->create_renderer($module);
-        }
-
-        return $this->renderers[$module];
-    }
-
-    /**
-     * Subclasses should override this method to actually create an instance of
-     * the appropriate renderer class, based on the module name. That is,
-     * this method should implement the same contract as
-     * {@link renderer_factory::get_renderer}.
-     *
-     * @param $module the name of part of moodle. E.g. 'core', 'quiz', 'qtype_multichoice'.
-     * @return object an object implementing the requested renderer interface.
-     */
-    abstract public function create_renderer($module);
-
     /**
      * For a given module name, return the name of the standard renderer class
      * that defines the renderer interface for that module.
@@ -155,22 +121,13 @@ abstract class renderer_factory_base implements renderer_factory {
  * @since     Moodle 2.0
  */
 class standard_renderer_factory extends renderer_factory_base {
-    /**
-     * Constructor.
-     * @param object $theme the theme we are rendering for.
-     * @param moodle_page $page the page we are doing output for.
-     */
-    public function __construct($theme, $page) {
-        parent::__construct($theme, $page);
-    }
-
     /* Implement the subclass method. */
-    public function create_renderer($module) {
+    public function get_renderer($module, $page) {
         if ($module == 'core') {
-            return new moodle_core_renderer($this->opencontainers, $this->page, $this);
+            return new moodle_core_renderer($page);
         } else {
             $class = $this->standard_renderer_class_for_module($module);
-            return new $class($this->opencontainers, $this->get_renderer('core'), $this->page);
+            return new $class($page, $this->get_renderer('core', $page));
         }
     }
 }
@@ -184,14 +141,13 @@ class standard_renderer_factory extends renderer_factory_base {
  * @since     Moodle 2.0
  */
 class cli_renderer_factory extends standard_renderer_factory {
-    /**
-     * Constructor.
-     * @param object $theme the theme we are rendering for.
-     * @param moodle_page $page the page we are doing output for.
-     */
-    public function __construct($theme, $page) {
-        parent::__construct($theme, $page);
-        $this->renderers = array('core' => new cli_core_renderer($this->opencontainers, $this->page, $this));
+    /* Implement the subclass method. */
+    public function get_renderer($module, $page) {
+        if ($module == 'core') {
+            return new cli_core_renderer($page);
+        } else {
+            parent::get_renderer($module, $page);
+        }
     }
 }
 
@@ -220,9 +176,9 @@ class theme_overridden_renderer_factory extends standard_renderer_factory {
      * @param object $theme the theme we are rendering for.
      * @param moodle_page $page the page we are doing output for.
      */
-    public function __construct($theme, $page) {
+    public function __construct($theme) {
         global $CFG;
-        parent::__construct($theme, $page);
+        parent::__construct($theme);
 
         // Initialise $this->prefixes.
         $renderersfile = $theme->dir . '/renderers.php';
@@ -240,18 +196,18 @@ class theme_overridden_renderer_factory extends standard_renderer_factory {
     }
 
     /* Implement the subclass method. */
-    public function create_renderer($module) {
+    public function get_renderer($module, $page) {
         foreach ($this->prefixes as $prefix) {
             $classname = $prefix . $module . '_renderer';
             if (class_exists($classname)) {
                 if ($module == 'core') {
-                    return new $classname($this->opencontainers, $this->page, $this);
+                    return new $classname($page);
                 } else {
-                    return new $classname($this->opencontainers, $this->get_renderer('core'), $this->page);
+                    return new $classname($page, $this->get_renderer('core', $page));
                 }
             }
         }
-        return parent::create_renderer($module);
+        return parent::get_renderer($module, $page);
     }
 }
 
@@ -292,9 +248,9 @@ class template_renderer_factory extends renderer_factory_base {
      * @param object $theme the theme we are rendering for.
      * @param moodle_page $page the page we are doing output for.
      */
-    public function __construct($theme, $page) {
+    public function __construct($theme) {
         global $CFG;
-        parent::__construct($theme, $page);
+        parent::__construct($theme);
 
         // Initialise $this->searchpaths.
         if ($theme->name != 'standardtemplate') {
@@ -313,7 +269,7 @@ class template_renderer_factory extends renderer_factory_base {
     }
 
     /* Implement the subclass method. */
-    public function create_renderer($module) {
+    public function get_renderer($module, $page) {
         // Refine the list of search paths for this module.
         $searchpaths = array();
         foreach ($this->searchpaths as $rootpath) {
@@ -325,7 +281,7 @@ class template_renderer_factory extends renderer_factory_base {
 
         // Create a template_renderer that copies the API of the standard renderer.
         $copiedclass = $this->standard_renderer_class_for_module($module);
-        return new template_renderer($copiedclass, $searchpaths, $this->opencontainers, $this->page, $this);
+        return new template_renderer($copiedclass, $searchpaths, $page);
     }
 }
 
@@ -352,8 +308,8 @@ class moodle_renderer_base {
      * @param $opencontainers the xhtml_container_stack to use.
      * @param moodle_page $page the page we are doing output for.
      */
-    public function __construct($opencontainers, $page) {
-        $this->opencontainers = $opencontainers;
+    public function __construct($page) {
+        $this->opencontainers = $page->opencontainers;
         $this->page = $page;
     }
 
@@ -440,22 +396,11 @@ class template_renderer extends moodle_renderer_base {
      * @param $searchpaths a list of folders to search for templates in.
      * @param $opencontainers the xhtml_container_stack to use.
      * @param moodle_page $page the page we are doing output for.
-     * @param renderer_factory $rendererfactory the renderer factory that created us.
      */
-    public function __construct($copiedclass, $searchpaths, $opencontainers, $page, $rendererfactory) {
-        parent::__construct($opencontainers, $page);
+    public function __construct($copiedclass, $searchpaths, $page) {
+        parent::__construct($page);
         $this->copiedclass = new ReflectionClass($copiedclass);
         $this->searchpaths = $searchpaths;
-        $this->rendererfactory = $rendererfactory;
-    }
-
-    /**
-     * Get a renderer for another part of Moodle.
-     * @param $module the name of part of moodle. E.g. 'core', 'quiz', 'qtype_multichoice'.
-     * @return object an object implementing the requested renderer interface.
-     */
-    public function get_other_renderer($module) {
-        $this->rendererfactory->get_renderer($module);
     }
 
     /* PHP magic method implementation. */
@@ -710,27 +655,7 @@ class moodle_core_renderer extends moodle_renderer_base {
     const END_HTML_TOKEN = '%%ENDHTML%%';
     const MAIN_CONTENT_TOKEN = '[MAIN CONTENT GOES HERE]';
     protected $contenttype;
-    protected $rendererfactory;
     protected $metarefreshtag = '';
-    /**
-     * Constructor
-     * @param $opencontainers the xhtml_container_stack to use.
-     * @param moodle_page $page the page we are doing output for.
-     * @param renderer_factory $rendererfactory the renderer factory that created us.
-     */
-    public function __construct($opencontainers, $page, $rendererfactory) {
-        parent::__construct($opencontainers, $page);
-        $this->rendererfactory = $rendererfactory;
-    }
-
-    /**
-     * Get a renderer for another part of Moodle.
-     * @param $module the name of part of moodle. E.g. 'core', 'quiz', 'qtype_multichoice'.
-     * @return object an object implementing the requested renderer interface.
-     */
-    public function get_other_renderer($module) {
-        $this->rendererfactory->get_renderer($module);
-    }
 
     public function doctype() {
         global $CFG;
@@ -1603,16 +1528,15 @@ class theme_config {
 /// added a block when anther theme was selected).
 ////////////////////////////////////////////////////////////////////////////////
 
-    /**
-     * @var string the name of this theme. Set automatically.
-     */
+    /** @var string the name of this theme. Set automatically. */
     public $name;
-    /**
-     * @var string the folder where this themes fiels are stored. $CFG->themedir . '/' . $this->name
-     */
+    /** @var string the folder where this themes fiels are stored. $CFG->themedir . '/' . $this->name */
     public $dir;
 
+    /** @var string Name of the renderer factory class to use. */
     public $rendererfactory = 'standard_renderer_factory';
+    /** @var renderer_factory Instance of the renderer_factory class. */
+    protected $rf = null;
 
     /**
      * If you want to do custom processing on the CSS before it is output (for
@@ -1637,7 +1561,7 @@ class theme_config {
      * @return theme_config an instance of this class.
      */
     public static function load($themename) {
-        global $CFG, $PAGE;
+        global $CFG;
 
         // We have to use the variable name $THEME (upper case) becuase that
         // is what is used in theme config.php files.
@@ -1660,6 +1584,25 @@ class theme_config {
         return $THEME;
     }
 
+    /**
+     * Get the renderer for a part of Moodle for this theme.
+     * @param string $module the name of part of moodle. E.g. 'core', 'quiz', 'qtype_multichoice'.
+     * @param moodle_page $page the page we are rendering
+     * @return moodle_renderer_base the requested renderer.
+     */
+    public function get_renderer($module, $page) {
+        if (is_null($this->rf)) {
+            if (CLI_SCRIPT) {
+                $classname = 'cli_renderer_factory';
+            } else {
+                $classname = $this->rendererfactory;
+            }
+            $this->rf = new $classname($this);
+        }
+
+        return $this->rf->get_renderer($module, $page);
+    }
+
     /**
      * Set the variable $CFG->pixpath and $CFG->modpixpath to be the right
      * ones for this theme.

diff --git a/lib/pagelib.php b/lib/pagelib.php
index e1693380ff84a6443d2851f3d7fb80890be88cb4..53c6447f45223384e22e3a9ff0bc8b3ed9f60321 100644 (file)
--- a/lib/pagelib.php
+++ b/lib/pagelib.php
@@ -133,6 +133,9 @@ class moodle_page {
      */
     protected $_wherethemewasinitialised = null;
 
+    /** @var xhtml_container_stack tracks XHTML tags on this page that have been opened but not closed. */
+    protected $_opencontainers;
+
     /**
      * Sets the page to refresh after a given delay (in seconds) using meta refresh
      * in {@link standard_head_html()} in outputlib.php
@@ -420,6 +423,18 @@ class moodle_page {
         return $this->_periodicrefreshdelay;
     }
 
+    /**
+     * Please do not call this method directly use the ->opencontainers syntax. {@link __get()}
+     * @return xhtml_container_stack tracks XHTML tags on this page that have been opened but not closed.
+     *      mainly for internal use by the rendering code.
+     */
+    public function get_opencontainers() {
+        if (is_null($this->_opencontainers)) {
+            $this->_opencontainers = new xhtml_container_stack();
+        }
+        return $this->_opencontainers;
+    }
+
     /**
      * PHP overloading magic to make the $PAGE->course syntax work by redirecting
      * it to the corresponding $PAGE->get_course() method if there is one, and
@@ -802,6 +817,8 @@ class moodle_page {
         $this->_theme = theme_config::load($themename);
         if ($this === $PAGE) {
             $THEME = $this->_theme;
+            // Support legacy code.
+            $this->_theme->setup_cfg_paths();
         }
     }
 
@@ -893,14 +910,9 @@ class moodle_page {
 
         if ($this === $PAGE) {
             $THEME = $this->_theme;
+            $OUTPUT = $this->_theme->get_renderer('core', $this);
+            // Support legacy code.
             $this->_theme->setup_cfg_paths();
-            if (CLI_SCRIPT) {
-                $classname = 'cli_renderer_factory';
-            } else {
-                $classname = $this->_theme->rendererfactory;
-            }
-            $rendererfactory = new $classname($this->_theme, $this);
-            $OUTPUT = $rendererfactory->get_renderer('core');
         }
 
         $this->_wherethemewasinitialised = debug_backtrace();

diff --git a/lib/simpletest/testoutputlib.php b/lib/simpletest/testoutputlib.php
index d638f9a0fa5cf490e8aa225e601dc857099573e2..509e2b4e8058c415daa9ad67cae7fb87618f52b8 100644 (file)
--- a/lib/simpletest/testoutputlib.php
+++ b/lib/simpletest/testoutputlib.php
@@ -45,12 +45,12 @@ class testable_renderer_factory extends renderer_factory_base {
     public $createcalls = array();
 
     public function __construct() {
-        parent::__construct(null, null);
+        parent::__construct(null);
     }
 
-    public function create_renderer($module) {
+    public function get_renderer($module, $page) {
         $this->createcalls[] = $module;
-        return new moodle_core_renderer(new xhtml_container_stack(), null, null);
+        return new moodle_core_renderer($page);
     }
 
     public function standard_renderer_class_for_module($module) {
@@ -103,22 +103,11 @@ class renderer_factory_base_test extends UnitTestCase {
         // Set up.
         $factory = new testable_renderer_factory();
         // Exercise SUT.
-        $renderer = $factory->get_renderer('modulename');
+        $renderer = $factory->get_renderer('modulename', new moodle_page);
         // Verify outcome
         $this->assertEqual(array('modulename'), $factory->createcalls);
     }
 
-    public function test_get_caches_repeat_calls() {
-        // Set up.
-        $factory = new testable_renderer_factory();
-        // Exercise SUT.
-        $renderer1 = $factory->get_renderer('modulename');
-        $renderer2 = $factory->get_renderer('modulename');
-        // Verify outcome
-        $this->assertEqual(array('modulename'), $factory->createcalls);
-        $this->assertIdentical($renderer1, $renderer2);
-    }
-
     public function test_standard_renderer_class_for_module_core() {
         // Set up.
         $factory = new testable_renderer_factory();
@@ -167,12 +156,12 @@ class standard_renderer_factory_test extends UnitTestCase {
     }
 
     public function test_get_core_renderer() {
-        $renderer = $this->factory->get_renderer('core');
+        $renderer = $this->factory->get_renderer('core', new moodle_page);
         $this->assertIsA($renderer, 'moodle_core_renderer');
     }
 
     public function test_get_test_renderer() {
-        $renderer = $this->factory->get_renderer('test');
+        $renderer = $this->factory->get_renderer('test', new moodle_page);
         $this->assertIsA($renderer, 'moodle_test_renderer');
     }
 }
@@ -198,12 +187,12 @@ class custom_corners_renderer_factory_test extends UnitTestCase {
     }
 
     public function test_get_core_renderer() {
-        $renderer = $this->factory->get_renderer('core');
+        $renderer = $this->factory->get_renderer('core', new moodle_page);
         $this->assertIsA($renderer, 'custom_corners_core_renderer');
     }
 
     public function test_get_test_renderer() {
-        $renderer = $this->factory->get_renderer('test');
+        $renderer = $this->factory->get_renderer('test', new moodle_page);
         $this->assertIsA($renderer, 'moodle_test_renderer');
     }
 }
@@ -315,7 +304,7 @@ class theme_overridden_renderer_factory_test extends UnitTestCase {
         $factory = new testable_theme_overridden_renderer_factory($theme, $this->page);
 
         // Exercise SUT.
-        $renderer = $factory->get_renderer('test');
+        $renderer = $factory->get_renderer('test', new moodle_page);
 
         // Verify outcome
         $this->assertIsA($renderer, 'moodle_test_renderer');
@@ -330,7 +319,7 @@ class theme_overridden_renderer_factory_test extends UnitTestCase {
         $factory = new testable_theme_overridden_renderer_factory($theme, $this->page);
 
         // Exercise SUT.
-        $renderer = $factory->get_renderer('test');
+        $renderer = $factory->get_renderer('test', new moodle_page);
 
         // Verify outcome
         $this->assertIsA($renderer, 'testrenderertheme_test_renderer');
@@ -348,7 +337,7 @@ class theme_overridden_renderer_factory_test extends UnitTestCase {
         $factory = new testable_theme_overridden_renderer_factory($theme, $this->page);
 
         // Exercise SUT.
-        $renderer = $factory->get_renderer('core');
+        $renderer = $factory->get_renderer('core', new moodle_page);
 
         // Verify outcome
         $this->assertIsA($renderer, 'parentrenderertheme_core_renderer');
@@ -368,7 +357,7 @@ class theme_overridden_renderer_factory_test extends UnitTestCase {
         $factory = new testable_theme_overridden_renderer_factory($theme, $this->page);
 
         // Exercise SUT.
-        $renderer = $factory->get_renderer('core');
+        $renderer = $factory->get_renderer('core', new moodle_page);
 
         // Verify outcome
         $this->assertIsA($renderer, 'ctheme_core_renderer');
@@ -411,7 +400,7 @@ class template_renderer_factory_test extends UnitTestCase {
         $CFG->themedir = $CFG->dataroot . '/' . $this->workspace;
         $this->foldertocleanup = $CFG->themedir;
 
-        $this->page = new stdClass;
+        $this->page = new moodle_page;
     }
 
     public function tearDown() {
@@ -532,10 +521,10 @@ class template_renderer_factory_test extends UnitTestCase {
         $this->make_theme_template_dir('mytheme', 'core');
         $this->make_theme_template_dir('parenttheme', 'test');
         $this->make_theme_template_dir('standardtemplate', 'test');
-        $factory = new testable_template_renderer_factory($theme, $this->page);
+        $factory = new testable_template_renderer_factory($theme);
 
         // Exercise SUT.
-        $renderer = $factory->get_renderer('test');
+        $renderer = $factory->get_renderer('test', $this->page);
 
         // Verify outcome
         $this->assertEqual('moodle_test_renderer', $renderer->get_copied_class());
@@ -719,11 +708,9 @@ class template_renderer_test extends UnitTestCase {
         parent::setUp();
         $this->templatefolder = $CFG->dataroot . '/temp/template_renderer_fixtures/test';
         make_upload_directory('temp/template_renderer_fixtures/test');
-        $page = new stdClass;
-        $page->course = new stdClass;
+        $page = new moodle_page;
         $this->renderer = new template_renderer('moodle_test_renderer',
-                array($this->templatefolder), new xhtml_container_stack(), $page, null);
-        $this->savedtemplates = array();
+                array($this->templatefolder), $page);
     }
 
     public function tearDown() {
@@ -781,13 +768,11 @@ class template_renderer_test extends UnitTestCase {
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class moodle_core_renderer_test extends UnitTestCase {
-    protected $containerstack;
     protected $renderer;
 
     public function setUp() {
         parent::setUp();
-        $this->containerstack = new xhtml_container_stack();
-        $this->renderer = new moodle_core_renderer($this->containerstack, null, null);
+        $this->renderer = new moodle_core_renderer(new moodle_page);
     }
 
     public function test_select_menu_simple() {