URL router that loads dependent objects

Question

How can I write my code more efficient (not repeating same code)?

And is there a solution that simplifies the way of having to create a lot of dependency objects over and over again whenever I instantiate a new class (as seen in the load method)?

Any suggestions are welcome.

class Router
{
 private $request;
 private $cleanUri;
 # control attributes
 private $controller;
 private $action;
 private $params = [];
 public function __construct(Request $request)
 {
 $this->request = $request;
 $this->cleanUri = preg_replace(['#/+#', '(\.\./)'], '/', trim($request->getUri(), '/')); 
 $this->parseUri();
 $this->load();
 }
 private function parseUri()
 {
 $parts = explode('/', $this->cleanUri);
 $this->controller = ($this->cleanUri !== '' ? ucfirst(array_shift($parts)) : 'Index') . 'Controller';
 $this->action = array_shift($parts);
 $this->params = !empty($parts) ? $parts : null;
 }
 private function load()
 {
 if (class_exists($this->controller)) {
 if (empty($this->action)) {
 $this->action = 'index';
 } elseif ($this->action !== 'index' && method_exists($this->controller, $this->action)) {
 //$action value stays the same
 } else {
 if (class_exists('_404Controller')) {
 $controller = new _404Controller();
 $controller->index();
 } else {
 throw new Exception('404 controller not found. Page doesn\'t exist for: ' . $this->request->getUri());
 }
 }
 $controller = $this->controller;
 //create dependencies
 $db = new Database();
 $user = new User($db, $this->request);
 $menu = new MenuHelper($this->request, $user);
 $controller = new $controller($this->request, $user, $menu);
 $action = $this->action;
 $controller->$action();
 } else {
 if (class_exists('_404Controller')) {
 $controller = new _404Controller();
 $controller->index();
 } else {
 throw new Exception('404 controller not found. Page doesn\'t exist for: ' . $this->request->getUri());
 }
 }
 }
}

Answer 1

If this unit of code is a router, then it should remain a router. Have the controller handle its dependencies. You could have multiple controllers, each with its own dependencies. You wouldn't want a single router code become a monster just to keep up with the controllers.

class Router{
 private $request;
 private $cleanUri;
 private $controller;
 private $action;
 private $params = [];
 public function __construct(Request $request){
 $this->request = $request;
 $this->cleanUri = $this->sanitizeUri($request->getUri())
 $this->parseUri();
 $this->load();
 }
 // Extract sanitize. You might want to add more than just this and the constructor
 // is a bad place to put it.
 private function sanitizeUri($uri){
 return preg_replace(['#/+#', '(\.\./)'], '/', trim($uri, '/'));
 }
 private function parseUri(){
 $parts = explode('/', $this->cleanUri);
 $this->controller = ($this->cleanUri !== '' ? ucfirst(array_shift($parts)) : 'Index') . 'Controller';
 $this->action = array_shift($parts);
 $this->params = !empty($parts) ? $parts : null;
 }
 // Extract the 404
 private function send404(){
 if (class_exists('_404Controller')) {
 $controller = new _404Controller();
 $controller->index();
 } else {
 throw new Exception('404 controller not found. Page doesn\'t exist for: ' . $this->request->getUri());
 }
 }
 private function load(){
 // If action is empty, use index
 if (empty($this->action)) $this->action = 'index';
 // If either controller or method doesn't exist, 404
 if(!(class_exists($this->controller) && method_exists($this->controller, $this->action))) 
 $this->send404();
 // else execute
 $controller->$action($this->params);
 }
}

Then the controller could have:

class MyController extends Controller{
 public function __construct(){
 $this->load(array(
 'database',
 'cats',
 'unicorns'
 ));
 }
 public function index(){
 $this->database->getSomething();
 }
}

• Kid Diamond: Thanks. I really like your answer. It makes my code more modular and more easy modifiable and understandable. But let's say I will need a User object in every of my controllers so I can check if a user is logged in or not with isLoggedIn() method. Wouldn't it be a good idea to initialize / inject it in the constructor of the controller parent class? And you also forgot to store the class property in a variable first before being able to instantiate the controller and call it's method with a variable in the load method.
• Joseph: @KidDiamond My PHP's a bit rusty so apologies. If you need a class that needs to be loaded for all modules, consider having a config file, which could contain an array. You can include them in the router and feed them to the controller.
• Kid Diamond: By using dependency injection on the costructor of the router (config array) and in the abstract controller (the class(es) that need to be loaded in every controller), right?