Simple PHP Router

Question

Based off simple controller implementations like those seen in micro-frameworks like Slim, Silex, F3, Laravel, Tonic, Flight, Klein, etc.

/EDIT tried to link the above sources, but as a new user I only get 2 links...

Any potential issues? Improvements?

class Router {   

    private static $routes = array();

    private function __construct() {}
    private function __clone() {}

    public static function route($pattern, $callback) {
        $pattern = '/' . str_replace('/', '\/', $pattern) . '/';
        self::$routes[$pattern] = $callback;
    }

    public static function execute() {
        $url = $_SERVER['REQUEST_URI'];
        $base = str_replace('\\', '/', dirname($_SERVER['SCRIPT_NAME']));
        if(strpos($url, $base) === 0) {
            $url = substr($url, strlen($base));
        }
        foreach (self::$routes as $pattern => $callback) {
           if(preg_match($pattern, $url)){
                preg_match_all($pattern, $url, $matches);
                array_shift($matches);
                $params = array();
                foreach($matches as $match){
                    if(array_key_exists(0, $match)){
                        $params[] = $match[0];
                    }
                }
                return call_user_func_array($callback, $params);
           }
        }
    }   
}

Takes a regexp and a callback, checks the URI for the first match... e.g.,

Router::route('blog/(\w+)/(\d+)', function($category, $id){
  print $category . ':' . $id;
});
Router::execute();
// if url was http://example.com/blog/php/312 you'd get back "php:312"...

TYIA.

Answer 1

I'm not seeing why you preg_match_all, considering you (can!) really only deal with the first match. Why not something like

if (preg_match($pattern, $url, $params)) {
    array_shift($params);
    return call_user_func_array($callback, array_values($params));
}

The str_replace to build a regex feels rickety to me; if the pattern were, say, '\/', it'd get regexified wrong and give you a PHP warning. (PCRE is pretty consistent about saying that any punctuation, metacharacter or not, can be escaped without ill effects. Blindly prefixing every / with a \ breaks that.) I'm not sure how you'd fix that, but one way to sidestep the problem might be to require that $pattern already include the delimiters rather than the router trying to tack them on itself. Besides that, it'd also serve as a clear "this is a PCRE-flavored regex" indicator, so the rules are known and people are less likely to accidentally use stuff like . without escaping it.

I'm thinking that private function __clone() {} isn't strictly necessary, and in fact hints that an instance may somehow exist. (If there's no instance possible, there's nothing to clone, and thus no need to prevent cloning.)