PHP router class

Question

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

<?php
namespace framework\core;
class Request
{ 
/**
 * Stores requested page.
 * 
 * @var array
 */
private $segments = array();
/**
 * Get requested page and explode it to segments.
 * 
 * @param string $protocol
 */
public function __construct($protocol)
{ 
 if($protocol != 'PATH_INFO' and $protocol != 'ORGI_PATH_INFO'){
 throw new InvalidArgumentException('URI protocol was not setup correctly.');
 }
 $uri = (isset($_SERVER[$protocol])) ? $_SERVER[$protocol] : '';
 $this->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

<?php
namespace framework\core;
class Router
{ 
/**
 * Prefix which will be appended to the class method.
 * 
 * @var constant 
 */
const actionPrefix = 'action';
/**
 * Router configuration data.
 * 
 * @var array 
 */
private $config = array();
/**
 * Request class object.
 * 
 * @var object
 */
private $request;
/**
 * Controller to be called.
 * 
 * @var string
 */
private $controller;
/**
 * Action to be called.
 * 
 * @var string
 */
private $action;
/**
 * Parameters which will be passed to
 * the controller.
 * 
 * @var array
 */
private $params = array();
/**
 * Store configuration and request object.
 * 
 * @param array $config
 * @param \framework\core\Request $request
 */
public function __construct($config, Request $request)
{
 $this->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()){
 call_user_func(array($obj, $this->action));
 }else{
 call_user_func_array(array($obj, $this->action), $this->params);
 } 
}
/**
 * Check if user requested specific controller, if not
 * then load the default one.
 * 
 * @return boolean
 */
private function validateController()
{
 $controller = $this->request->getController();
 if($controller != false && $controller != ''){
 $this->controller = $controller;
 return true;
 }
 $this->controller = $this->config['DEFAULT_CONTROLLER'];
}
/**
 * Check if user requested a specific action, if not
 * then load the default action.
 * 
 * @return boolean
 */
private function validateAction()
{
 $action = $this->request->getAction();
 if($action != false && $action != ''){
 $this->action = self::actionPrefix . $action;
 return true;
 }
 $this->action = self::actionPrefix . $this->config['DEFAULT_ACTION'];
}
/**
 * Check of the requested controller exists in controllers
 * directory.
 * 
 * @return boolean
 */
private function controllerExists()
{
 $path = APP_PATH . 'controllers' . DS . $this->controller . '.php';
 if(file_exists($path)){
 return true;
 }
 return false;
}
/**
 * Check to see if action is callable or not.
 * 
 * @return boolean
 */
private function actionExists()
{ 
 if(is_callable(array('application\\controllers\\' . $this->controller, $this->action))){
 return true;
 }
 return false;
}
/**
 * Figure out if we have to pass additional parameters
 * to the requested controller.
 * 
 * @return boolean
 */
private function paramsExists()
{
 if($this->request->getParams())
 {
 $this->params = $this->request->getParams();
 return true;
 }
 return false;
}
}

Answer 1

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.