Skip to main content
Remove functions as controllers in dispatcher? Started by emanuele · · Read 2128 times 0 Members and 1 Guest are viewing this topic. previous topic - next topic

Remove functions as controllers in dispatcher?

At the moment the dispatcher supports in several way the use of functions as controllers, instead of the pair controller/method.
Usually the function is used as fallback when the method does not exist in the controller.

And this is implemented in a slightly odd way: using a function as controller,  because it first instantiate the default controller, and then if the "possible method" doesn't exist it is checked as a function...pseudo code:
Code: [Select]
$possible_method = 'myMethod';
$controller = new MyController();
if (method_exists($controller, $possible_method))
    $controller->{$possible_method}();
elseif (function_exists($possible_method))
    $possible_method();
else
{
    $controller = new BoardIndex_Controller();
    $controller->action_index();
}
What I'm proposing is to deprecate and immediately remove (from 1.1) the function_exists block.
Leading to something like:
Code: [Select]
$possible_method = 'myMethod';
$controller = new MyController();
if (method_exists($controller, $possible_method))
    $controller->{$possible_method}();
else
{
    $controller = new BoardIndex_Controller();
    $controller->action_index();
}

Then, I think that since we have in 1.1 an autoloader that takes care of controllers, we could even use:
Code: [Select]
$possible_method = 'myMethod';
$possible_controller = 'MyController';
if (method_exists($possible_controller, $possible_method))
{
    $controller = new $possible_controller();
    $method = $possible_method;
}
else
{
    $controller = new BoardIndex_Controller();
    $method = 'action_index';
}
$controller->{$method}();

Do you see anything broken with that?

ETA: forgot to add that ElkArte itself uses only classes as controllers, so the function version would be used only by addons.
Last Edit: October 11, 2014, 08:54:29 am by emanuele
Bugs creator.
Features destroyer.
Template killer.

Re: Remove functions as controllers in dispatcher?

Reply #1

I would go one step further. Just remove the else and make it fail if it doesn't exist. I think that makes it better for testing and finding issues.

Re: Remove functions as controllers in dispatcher?

Reply #2

I was thinking that this could be one of the spots for a 404 more than an error.
Bugs creator.
Features destroyer.
Template killer.

Re: Remove functions as controllers in dispatcher?

Reply #3

Either way, it is an error. Though, you are already going to do an isset($actions[$route]). So, if it isn't set, that's when you would fail to a 404. You want that logged so you can tell where you might have broken links.

Re: Remove functions as controllers in dispatcher?

Reply #4

https://github.com/elkarte/Elkarte/pull/1979
Bugs creator.
Features destroyer.
Template killer.