Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I want to direct all requests that haven't got a valid controller action to my custom page controller.

I.e. I want /pages/new & /users/login etc to work correctly, but a request for /foo or /foo/bar should be directed to /pages/display/$path.

The following code works, but I'm pretty certain I'm not doing something the right way(tm).

// In app/Config/routes.php
Router::connect('/', array('controller' => 'pages', 'action' => 'displayHome'));

$controllerList = App::objects('controller');
foreach ($controllerList as $controller) {
    $contstr = substr($controller, 0, strlen($controller)-10);
    $contstr = Inflector::tableize($contstr);
    if($contstr != 'app') {
        Router::connect('/'. $contstr, array('controller'=>$contstr));
        Router::connect('/'. $contstr .'/:action', array('controller'=>$contstr));
        Router::connect('/'. $contstr .'/:action/*', array('controller'=>$contstr));

        // FIXME: Check if admin routing is set
        Router::connect('/admin/'. $contstr, array('controller'=>$contstr, 'admin'=>true));
        Router::connect('/admin/'. $contstr .'/:action', array('controller'=>$contstr, 'admin'=>true));
        Router::connect('/admin/'. $contstr .'/:action/*', array('controller'=>$contstr, 'admin'=>true));
    }
}

Router::connect('/**', array('controller'=>'pages', 'action'=>'display'));

Would someone please be able to review this and tell me if this is okay/horribly wrong?

share|improve this question

1 Answer 1

Use a pattern

The general principle is fine, though it won't handle plugin routes (which I assume isn't a problem)

However instead of a loop (and therefore defining 6 routes per controller) a parameter for the controller name can be used which will be more concise and likely overall more performant. The code could be rewritten like this:

// Define a home route
Router::connect('/', array('controller' => 'pages', 'action' => 'displayHome'));

// Generate a regex-like controller name pattern (e.g. "posts|comments|users")
$controllerList = App::objects('controller');
$controllerList = array_map(function($c) {
    return Inflector::underscore(substr($c, 0, strlen($c)-10);
}, $controllerList));
$controllerList = array_filter($controllerList, function ($c) {
    return $c !== 'app'
});
$controller = implode('|', $controllerList);

// Define normal routes
$params = array('controller' => $controller);
Router::connect('/:controller', $params);
Router::connect('/:controller/:action', $params);
Router::connect('/:controller/:action/*', $params);

// Define admin routes
$params += array('admin' => true, 'prefix' => 'admin');
Router::connect('/admin/:controller', $params);
Router::connect('/admin/:controller/:action', $params);
Router::connect('/admin/:controller/:action/*', $params);

// Define catchall
Router::connect('/**', array('controller'=>'pages', 'action'=>'display'));

In this way there are 8 routes in total, no matter how many controllers there are in the application.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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