Jump to content

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

Link to comment
https://forums.phpfreaks.com/topic/111992-plugins/page/2/#findComment-576887
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 :)

Link to comment
https://forums.phpfreaks.com/topic/111992-plugins/page/2/#findComment-576889
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)

Link to comment
https://forums.phpfreaks.com/topic/111992-plugins/page/2/#findComment-576980
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • 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.