HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PHP router class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
phprouterclass

Problem

I'm writing yet another MVC framework just for learning purposes and I would like you to review my code. Whenever I'm coding, I have that feeling that my code is horrible and is not the most efficient way to achieve something.

Right now the request class takes a URL path from ORIG_PATH_INFO or from PATH_INFO and explodes it into segments. Then I can easily retrieve controllers/actions/parameters from within my router class.

request.php

segments = explode('/', rtrim($uri, '/'));
    array_shift($this->segments);
}

/**
 * Return all segments.
 * 
 * @return array
 */
public function getAll()
{   
    return $this->segments;
}

/**
 * Get requested controller.
 * 
 * @return mixed
 */
public function getController()
{
    if(isset($this->segments[0])){

        return strtolower($this->segments[0]);

    }else{

        return false;
    }
}

/**
 * Get requested action.
 * 
 * @return mixed
 */
public function getAction()
{
    if(isset($this->segments[1])){

        return strtolower($this->segments[1]);

    }else{

        return false;
    }
}

/**
 * Get requested parameters.
 * 
 * @return mixed
 */
public function getParams()
{
    if(isset($this->segments[2])){

        return array_slice($this->segments, 2);

    }else{

        return false;
    }
}
}


And now the router class which matches controllers/methods automatically. (Right now it's pretty basic, but later I'm planning on adding predefined routes and maybe REST)

router.php

```
config = $config;
$this->request = $request;
}

/**
* Match url to controllers/actions and pass parameters if available.
*/
public function dispatch()
{

$this->validateController();
$this->validateAction();

if(!$this->controllerExists() || !$this->actionExists()){

require_once(APP_PATH . 'error' . DS . 'error_404.php');
exit();
}

$controller = 'application\\controllers\\' . $this->controller;

$obj = new $controller;

if(!$this->paramsExists()){

ca

Solution

From quick glance I see a lot of logic in the constructor, usually not recommended.

I would also avoid hard-coded indices for segments and use regular expressions to parse the URLs. Basically the less hard-coded strings are in the code, the better.

You can place them together with all your constants and settings in a separate Config class or JSON. Then you can quickly change those settings and the code becomes more re-usable.

Context

StackExchange Code Review Q#35478, answer score: 2

Revisions (0)

No revisions yet.