# Simple PHP Router

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.

-

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.)

-
very good point, i'll implement your suggestion. Anything else you see? Any potential problem areas? –  momo Nov 9 '12 at 2:54
@momo: Just updated with other things i considered. –  cHao Nov 9 '12 at 4:40
great feedback. i'd upvote but need more rep. thanks again. –  momo Nov 9 '12 at 16:10