CODEWITHSUNDEEP
phpoopurlroutespathinfo
Stephan-v
Stephan-v

Reputation: 20309

How to fix this router class to include 404 errors/pages?

I'm working on a simple router for php practice, it's already evolved quite a bit more but I'm posting the more simplistic version.

My router class check a given url vs the current server path and returns content if they match. This works fine but the problem is that the router runs 3 times because I'm calling the router get() method 3 times in my routes.php file.

If I for example want to return a 404 error when the route does not match it will return it 3 times. 1 time for each route not found. What is the best way to fix this?

As always your help is much appreciated.

Router class:

<?php
class Router {
    public $currentRoute = '/';
    public function __construct()
    {
        $this->getCurrentRoute();
    }
    public function getCurrentRoute()
    {
        if(isset($_SERVER['PATH_INFO'])) {
            $this->currentRoute = $_SERVER['PATH_INFO'];
        }
    }
    public function get($route, $content)
    {
        if($route == $this->currentRoute) {
            echo $content();
        }
    }
}

Routes file:

<?php

$router->get('/', function()
{
    return 'Index page';
});

$router->get('movies', function()
{
    return Cinematix\View::make('movies');
});

$router->get('users', function() 
{
    return 'The users collection page';
});

Upvotes: 2

Views: 3292

Answers (2)

aergistal
aergistal

Reputation: 31209

First be warned that your approach is not recommended. Looking up the classic MVC programming pattern is a good way to start.

Read on if you really want to do it the way you started using anonymous callbacks.

Router class:

class Router {
    public $currentRoute = '/';
    private $routes;

    public function __construct($routes)
    {
        $this->routes = $routes;
        $this->getCurrentRoute();
    }
    public function getCurrentRoute()
    {
        if(isset($_SERVER['PATH_INFO'])) {
            $this->currentRoute = $_SERVER['PATH_INFO'];
        }
    }
    public function route()
    {
        $route = $this->currentRoute;

        if (!isset($this->routes[$route])) {
            $route = '/404';
        }

        if (is_callable($this->routes[$route])) 
        {
            echo $this->routes[$route]();  
        }    
    }
}

routes file:

$routes = array(
    '/' => function()
    {
        return 'Index page';
    },
    '/movies' => function()
    {
        return 'Movies';
    },
    '/users' => function() 
    {
        return 'The users collection page';
    },
    '/404' => function() 
    {
        return '404';
    }
);

$router = new Router($routes);
$router->route();

Upvotes: 1

Greg Kelesidis
Greg Kelesidis

Reputation: 1054

A simple solution may be to declare a static flag in your class. Say 

<?php
class Router {
    static $found = 0;
    public $currentRoute = '/';

Then, in routes.php you go like:

$router->get('/', function()
{
    Router::$found = 1;
    return 'Index page';
});

$router->get('movies', function()
{
    Router::$found = 1;
    return Cinematix\View::make('movies');
});

$router->get('users', function() 
{
    Router::$found = 1;
    return 'The users collection page';
});

if (!Router::$found) {
    $router->get('404', function()
    {
        return '404 page';
    });
}

Now, the '404' has to be a declared 'path' in the Router class.

Upvotes: 1

Related Questions