Jump to content

Archived

This topic is now archived and is closed to further replies.

redbullmarky

plugins

Recommended Posts

i have a dispatcher class with a dispatch method. The chunk in question is:

 

// ...do the stuff with whatever we have from the router/request object
$class = $this->getClassName($this->rq->getParams());
$controller = new $class($this->rq);
$controller->init();

if (method_exists($controller, $this->rq->action)) {
$action = $this->rq->action; // could be 'viewAction'
$p = $this->rq->getParams();
$out = call_user_func_array(array($controller, $action), $p['args']);
}

echo $out;

 

I don't like that.

 

Standardize, standardize, standardize. I can understand why you do it this way -- because PHP lets you, but, reading that is a mess and a half.

 

First this line

$class = $this->getClassName($this->rq->getParams());
$controller = new $class($this->rq);
$controller->init();

 

These should be updated to use an Abstract Factory pattern [1]. The idea is to create the object for you and to return a 'known' type based on input. What if, $this->rq->getParams() was tainted and $class doesn't exist? I would prefer to see it like this:

 

<?php

interface Controller {
    public function init();
    public function generate();
    public function afterLoad();
    public function afterRender();
    public function setContext(Context $context);

}

class ActionController {
    private $args;
    private $context = null;

    public function __construct($args) { $this->args = $args; }
    public function setContext(Context $context) { $this->context = $context; }
    public function init() {
        //i assume this sets local variables, etc
    }

    public function afterRender() {

    }

    public function afterLoad() {

    }

    public function generate() {
        $this->afterLoad();
        $tpl = new Template();
        $tpl->setAll($this->context->getParams($page));
        $tpl->setOutput($tpl->render($this->context->getPageTemplate()));
        $output = $this->afterRender();
        return $output;
    }
}

class ControllerFactory {

    public static function getClass($type, $args) {
        switch ($type) {
            case 'viewAction':
                $a = new ActionController($args);
                $a->init();
                return $a;
            case 'viewSomethingElse':
                $p = new SomethingElseController($args);
                $p->init();
                return $p;
            default:
                throw new ActionNotFoundException('Could not find requested Action');
        }

    }

}


// client code
try {
    $class = ControllerFactory::getClass($this->rq->action, $this->rq->getParams());
    $output = $class->generate();
    if ($output == NULL) //do something ;
    echo $output;
} catch (ActionNotFoundException) {
    //generate some error
}


?>

 

 

 

 

[1]: http://en.wikipedia.org/wiki/Abstract_Factory

Share this post


Link to post
Share on other sites

hmm possibly another thought - adding everything to a context, including the plugins?

 

public function testAction()
{
   $c = new Context($this); // context can access getRequest, getUser, etc methods if required by plugins

   $page = Page::loadByRequest($this->rq);

   $c->setParams($page->getParams()); // pass page vars (title, body, etc) to context - Django-ish, I suppose...
   $c->addPlugin('test'); // add a default plugin that ALWAYS gets used
   $c->addPlugins($page->getPlugins()); // add plugins that are attached to a page.
   $c->addView($this->tpl); // set the view object to use to render page

   return $c->renderContext('page.tpl');
}

 

the thought being that my context object is left to handle plugin calls, variable setting, filters, etc and its render method uses a View class to render the final HTML response.

 

thoughts?

 

As long as the Context is not initializing anything, I don't see any harm. But, you should be careful about *what* you want to expose. Exposing too much is not a good thing :)

Share this post


Link to post
Share on other sites

going back to earlier - as it's not always necessary to attach a plugin to the controller (by $this->addPlugin('my_plugin')), is it considered bad practice to have a sort of registry purely for plugins, so I can just pass in what I need as required?

 

For example (I'll call it PluginManager for less of a better name for now)

 

public function testAction() {
   $page = Page::loadByRequest($this->rq);

   PluginManager::addPlugin('nav'); // add nav by default
   PluginManager::addPlugins($page->getPlugins()); // add those associated with my page

   // when editing a page, I might want to do this...
   $form = $page->getEditForm();
   $form = PluginManager::getEditForm($form); // makes a magic method call, which in turn calls the method of same name in each of the plugins (if exists)


   // when viewing a page, I might do something like this instead...
   $params = PluginManager::beforeRender($page->getParams()); // manipulate page vars

   // the page is constructed as normal
   $out = $this->tpl->render('page.tpl', $params);
   $out = PluginManager::afterRender($out);
}

 

as my need is specific, and plugins are essentially not just reacting to and changing page properties but all sorts of things, is this acceptable, or am I really going off tangent now? I still like the context idea and, to some extent, helps with a lot of my issues - however, when it comes to extra processing like the form that's generated by the model, I can't see it working as i'd like it to.

 

(nb - the code above is just scratchy as an example/proof of concept, rather than tried/testing/working code or anything resembling what the final code may be like)

Share this post


Link to post
Share on other sites

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.