patternphpMinor
PHP router class
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
request.php
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
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.
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.