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'm building a router for my MVC Framework. How can I make it better? What am I doing right/wrong?

I have no idea about how best to handle errors. I would prefer to use exceptions - is this the right way?

=> routes.php (Register all Routes)

$route->get('/', 'HomeController@index');
$route->get('/profile/{id}', 'HomeController@show');

$route->post('/', 'HomeController@store');

=> Route.php (Router class)

class Route {

    /**
     * Store all routes.
     */
    private $routes = ['get' => [], 'post' => []];

    /**
     * Store all parameters.
     */
    private $parameters = [];

    /**
     * Requestet uri.
     */
    private $uri;

    /**
     * Store possible matched parameters from regex.
     */
    private $matches;

    /**
     * Unleash and run the router.
     */
    public function run()
    {
        $this->uri = isset($_GET['url']) ? '/' . $_GET['url'] : '/';

        foreach($this->routes[$this->requestMethod()] as $pattern => $callback) {
            list($controller, $action) = explode('@', $callback);

            $pattern = '/' . trim($pattern, '/');

            if($this->match($pattern)) {
                require '../app/controllers/' . $controller . '.php';
                $controller = new $controller();

                // remove the element for complete uri.
                array_shift($this->matches);

                // Store possible matched parameter in a flat array 
                // for call it in call_user_func_array().
                foreach($this->matches as $params => $value) {
                    $this->parameters[] = $value[0];
                }

                return call_user_func_array([$controller, $action], $this->parameters);
            }
        }
    }

    /**
     * Match the URI against the {pattern}.
     */
    private function match($pattern)
    {
        $pattern = preg_replace('#\{[^/-]+\}#', '(.+)', $pattern);

        return preg_match_all('#^' . $pattern . '$#', $this->uri, $this->matches);
    }


    /**
     * Add routes for get requests.
     */
    public function get($pattern, $callback)
    {
        $this->routes['get'][$pattern] = $callback;
    }

    /**
     * Add routes for post requests.
     */
    public function post($pattern, $callback)
    {
        $this->routes['post'][$pattern] = $callback;
    }

    /**
     * Return the routes.
     */
    public function getRoutes($requestMethod = null)
    {
        return $requestMethod ? $this->routes[$requestMethod] : $this->routes;
    }

    /**
     * 
     */
    private function requestMethod()
    {
        return strtolower($_SERVER['REQUEST_METHOD']);
    }

}
share|improve this question
    
Welcome to CR. This question can be reviewed however you shouldn't ask about code you haven't yet written. Next time please try to include the best shot you could implement to solve your problem. Unless you edit your question people only will be able to review the code you provided. –  Bruno Costa Sep 11 at 8:34

Your Answer

 
discard

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

Browse other questions tagged or ask your own question.