1
\$\begingroup\$

I'm in the process of building a soon to be opensource CMS. 100% MVC. The basic idea is that a plugin should be able to add pages (read: methods) to a given controller, without needing to instantiate a new Route object, which the Router would then have to iterate over to test for a match. class_alias won't work. You can only extend 1 controller using extend won't work. You can only extend 1 controller.

After a number of hours scanning the manual and using google to try to find an answer to this problem, I was at a loss, so I started hacking. This is what I came up with, and it works, and due to the design of the application, it adds only a slight performance hit, which is due to the need to instantiate the controller twice and pass the called method to the proxied instance of the class. Bear in mind that the end user writing modules and plugins for this system needs to use minimal effort to accomplish these things, which provides the need for all of this context switching.

Base Controller class which controllers that wish to be virtually extensible must extend

namespace t;

class Controller
{
    public $callables = [];
    public $app;

    final public function __construct()
    {
        $this->app = PHP::getInstance();
        $this->app->invokePlugins('\\'.get_class($this));
        $c = $this->app->getControllerExtensions($this);//plugins register virtual methods to controllers. 
        if(!empty($c))
        {
            foreach($c as $k=>$v)
            {
                //handle the next step in the extend method to prevent vague exceptions being thrown
                $this->extend($k,$v);
            }   
        }

    }

    final public function __call($fn_name,$fn_args=[])
    {
        if(isset($this->callables[$fn_name])){
            return call_user_func_array($this->callables[$fn_name],$fn_args); //first try callables. allows overwriting built in methods.
        }
        $c2 = '\\'.get_class($this);
        $instance = new $c2; //instantiate the class again since we cant get at its private methods from this context
        if(method_exists($instance,$fn_name)){
            return $instance->proxy($fn_name,$fn_args); //use the proxy trait to successfully invoke the controller method that is private
        }

        //if we made it here, we know that the method does not exist. throw \t\Exceptions\Pass
        $this->app->pass();
    }

    final protected function extend($fn_name,$fn_callback)
    {
        if(!is_callable($fn_callback)){
            throw new \InvalidArgumentException('`extend` expects the second parameter to be a valid callable.');
        }
        $this->callables[$fn_name] = $fn_callback;
    }
}

The Proxy trait required to allow base controller to call child controllers private methods

namespace t;

trait Proxy
{
    public function proxy($fn,$args=[])
    {
        // here we are in the private context of the class using this trait, 
        // so we can call its private methods.
        return call_user_func_array([$this,$fn],$args);
    }
}

An example class. Notice how this is achieved with minimal effort to the developer.

namespace t\BuiltIns\Routes;

class Admin extends \t\Controller
{
    use \t\Proxy;

    // methods must be private to ensure that 
    // `\t\Controller::__call()` gets called in the parent class context
    private function dashboard()
    {
        $this->app->view->render('admin/dashboard.twig');
    }

    private function plugins()
    {
        $this->app->view->render('admin/plugins.twig');
    }
    // now plugins and modules may extend this class to add new routes, 
    // without requiring a costly new instance of `RouteObject` for the
    // `Router` to iterate over when matching routes.
}

Comments:

I do realize that it is technically an abuse of the language to support a feature that isn't built in. But it seems to be a solid enough implementation that it won't cause any unforeseen problems. For what it's worth, RouteObject will not invoke the __call method in any controller, so that prevents people from causing the error log to be filled up with \t\Controller::__call() missing argument exceptions. The system is based heavily on Slim 2.6.x, with fairly extensive modifications to suit this particular use case.

\$\endgroup\$
  • \$\begingroup\$ Is there a special reason for the Admin class must have private methods? You could avoid using your Proxy trait my making them public. \$\endgroup\$ – AnotherGuy Oct 30 '15 at 7:01
  • \$\begingroup\$ @AnotherGuy if the methods are public, then its impossible to "overwrite" them, since __call() is invoked only when an inaccessible or non existent function is called on an instance of a class. this is the singular reason for making controller methods private. in this way we make possible for a closure of the same name of a function may exist and be called instead of the real function. \$\endgroup\$ – r3wt Oct 30 '15 at 7:59
  • \$\begingroup\$ @AnotherGuy I'm definitely still open to an alternative approach. i'm not sure if i would be able to call child protected methods from the parent, but if i could that would be a win since it saves the instantiation of the child class from occuring, and eliminates the need for the proxy. \$\endgroup\$ – r3wt Oct 30 '15 at 22:49
  • 1
    \$\begingroup\$ I am still considering alternatives, but I am unsure if this is actually necessary. I personally avoid using magic methods and would prefer a solution without. I will write an answer if I find a (to me) sane solution \$\endgroup\$ – AnotherGuy Oct 31 '15 at 12:06
1
\$\begingroup\$

I have finally found a solution that I feel is right for this issue. To me it seems your controllers are given a responsibility, which I feel is misplaced. Overriding/extending controller methods should not be the concern of the controller itself. To me a controller should only be responsible for its core behaviours.

Therefore the solution I have found would move the concern of checking for overriding methods to before the controller is even instantiated. Consider the following example:

/*
 * I assume controller class and method names are stored somewhere in the
 * routing.
 */
$class  = "controller_class_name";
$method = "controller_method_name";
$args   = [];

/*
 * The plugins are fetched somewhere outside the controller instance. That 
 * is not important for the example.
 */
$plugins = $app->loadPlugins($class); // An example of loading plugins.

/*
 * Check if the called method is a 'virtual' controller method.
 */
if(array_key_exists($method, $plugins))
{
    $response = call_user_func_array($plugins[$method], $args);
}
else
{
    $instance = new $class();
    $response = $instance->$method($args);
}

Now you should be able to extend and override any methods declared inside a controller. You can also keep your methods public and the constructor clean for the concrete controller implementation. Having methods public also feels more appropriate to me, as a class with no public methods, which therefore can do nothing, is redundant.

If you require access to the controller instance itself this may not work, unless you require dependencies inside your callbacks. I feel like the main benefit of this is a clear code flow without any "hacks" or abuse of exceptions and magic methods, which both are quite performance heavy if you are to call them often.

Please keep in mind the example shown has no error handling and should only be used as a proof of concept.

I found it a little difficult to understand your situation completely, so I hope this can help in any way.

Happy coding!

\$\endgroup\$
  • \$\begingroup\$ I need a bit more time to think about this. I can see the advantage of your approach here but i'm very apprehensive about this approach, since it requires placing functionality belonging to a controller inside the Router class. I appreciate your response, you have given me something to think about. \$\endgroup\$ – r3wt Nov 21 '15 at 17:02
  • \$\begingroup\$ Also i need to investigate whether an exception is throw before checking if __call() is defined. if its not i see no problem, but if it is your point is certainly valid. it shouldn't be though. logically you would only throw if there was no way you could continue execution, and if __call() is defined than you can continue there of course with a neccessary recursion checks. this is how its implemented in HHVM which makes sense IMO. \$\endgroup\$ – r3wt Nov 21 '15 at 17:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.