Diving into PHP's MVC - the application's core

Question

I recently started programming in PHP using the MVC-pattern, which resulted in one of best choices I made so far, but I read through many articles showing "easy" examples and some of them were quite advanced (based on my opinion).

I started learning the pattern by looking up some code and I ended up here: "php-mvc"-repository by panique

I changed some lines of code and my best approach up to this point is this:

<?php
define( 'DEFAULT_CONTROLLER', 'controller' );
define( 'DEFAULT_METHOD', 'index' );
define( 'CONTROLLERS_DIR', './application/controllers/' );
/**
 * Gets <b>$_GET['url']</b>, while assuming it will be used in the sense of <i>MVC</i>,
 * it returns information for the controller.
 * 
 * @returns string Returns $_GET['url']
 */
function getUrl()
{
 $url = filter_input( INPUT_GET, 'url', FILTER_SANITIZE_URL );
 return ( isset( $url ) && !empty( $url ) ? $url : ( defined( 'DEFAULT_CONTROLLER' ) ? DEFAULT_CONTROLLER : null ) );
}
class Application
{
 const CONTROLLER_INDEX = 0;
 const METHOD_INDEX = 1;
 const DEFAULT_CONTROLLER = DEFAULT_CONTROLLER;
 const DEFAULT_METHOD = DEFAULT_METHOD;
 public $url;
 public $controller, $method, $params;
 public function __construct()
 {
 $this->getUrl();
 $this->splitUrl();
 $this->getController();
 $this->getMethod();
 $this->getParams();
 $this->loadController();
 $this->callMethod();
 }
 /**
 * Retrieves the controller url
 */
 private function getUrl()
 {
 $this->url = getUrl();
 }
 /**
 * Splits the url for better access
 */
 private function splitUrl()
 {
 $this->url = explode( '/', rtrim( $this->url, '/' ) );
 }
 /**
 * Gets the controller from $this->url
 */
 private function getController()
 {
 $this->controller = $this->url[self::CONTROLLER_INDEX];
 unset( $this->url[self::CONTROLLER_INDEX] );
 }
 /**
 * Gets the method from $this->url
 */
 private function getMethod()
 {
 $this->method = ( isset( $this->url[self::METHOD_INDEX] ) ? $this->url[self::METHOD_INDEX] : self::DEFAULT_METHOD );
 unset( $this->url[self::METHOD_INDEX] );
 }
 /**
 * Gets the parameters from $this->url
 */
 private function getParams()
 {
 $this->params = array_values( $this->url );
 if ( empty( $this->params ) )
 {
 $this->params = NULL;
 }
 unset( $this->url );
 }
 /**
 * Loads the controller
 */
 private function loadController()
 {
 if ( !file_exists( CONTROLLERS_DIR . $this->controller . '.php' ) )
 {
 $this->controller = self::DEFAULT_CONTROLLER;
 }
 $this->controller = new $this->controller();
 }
 /**
 * Calls the method
 */
 private function callMethod()
 {
 if ( !method_exists( $this->controller, $this->method ) )
 {
 $this->method = self::DEFAULT_METHOD;
 }
 $this->controller->{$this->method}( $this->params );
 }
}

Basically, what I wanted to do, is making the code clearer and easily readable, but now I am facing a point, where I got stuck.

1. I really wanted to share my approach, because I wanted you to share some thoughts about it.
2. Secondly, when I start a project, I always try writing my own little framework, which I require by using spl_autoload_register and the namespace, and I wanted to implement namespaces in the whole project (as it is possible?).
3. Is there any advice you would like to share (based on experience, ...)? For example: references using the MVC-pattern, things definitely worth reading.

Answer 1

For the actual code review, scroll to the bottom of this article

The "single pattern to rule them all" pattern

This is a mistake a lot of starting developers are making. They read about how good MVC is. And that everyone should use it. A bit like jQuery, where jQuery is always the answer, independent of the question. It's like using a boat to plow a field because it is really good in the water and everyone like a boatride!

A pattern is not a rule, but a best practice to solve a certain problem. The MVC-pattern helps us solve some of the code-clutter and seperation of concern of domain logic, controlling logic an displaying stuff to the user.

Some even tend to say that MVC isn't a pattern but a way-of-life. Ignore those peeps.

Biggest problem of MVC

Because of the 'MVC solves everything' approach, you ended up writing an Application class that does, a bit of everything but only half and all in the constructor.

The moment you do more then assignments (plus the optional exception throwing for bad data) in a __construct, it's time to walk away from your code, take a break (or a beer) and then come back to look at it - with some fresh eyes.

Now that we've had our beer, try and explain to yourself why you should use the Application class and what problem it can help you solve. But because we are lazy, we simply write up the API for that class.

The Application class has no dependencies (empty construct) and has no public methods. You do however have access to 4 parameters: url, controller, method and params.

Aaah, so it's a kind of parameterbag. But then without any get/set methods and only public variables.

So let's fire up that parameterbag:

$app = new Application();

FATAL ERROR: controller not found / undefined or something like that

Application class gone wrong

The above example is a developer trying to use your code without reading it - you should never have to read code, the api should be enough.

KISS: Keept It Stupid Simple

Your application class is doing to much. And what happens if I want to run multiple applications? Each with a different url passed in? And what is a 'Application'? What does it represent/do? And more importantly: What problem does it solve?. Ow, and if the words describing the problem don't fit on a post-it, your class is doing to much.

---

The actual code review

construct

public function __construct()
{
 $this->getUrl();
 $this->splitUrl();
 $this->getController();
 $this->getMethod();
 $this->getParams();
 $this->loadController();
 $this->callMethod();
}

A constructor is there to instantiate the class. It shouldn't do anything. Let the developer choose when to load the controller, or filter the url, ...

Obsolete methods

private function getUrl()
{
 $this->url = getUrl();
}

What is the point here? Instead of having one line in the __construct and doing the assignement there. You added 4 lines. Remove them and add the assignment to the construct:

public function __construct()
{
 $this->url = getUrl();
}

But then there is the problem of tight coupling. A class should have as little as dependencies as possible. And all dependencies should be added in the __construct. If you need ask it:

public function __construct($url)
{
 $this->url = $url;
}

This makes testing a lot easier:

$url = 'controller/method/param1/param2';
$app = new Application($url);

And we have now succesfully decoupled the getUrl() function from our Application class. The Application class shouldn't care how the url is formed. It only tells us that it wants a url.

Misused constants

const DEFAULT_CONTROLLER = DEFAULT_CONTROLLER;
const DEFAULT_METHOD = DEFAULT_METHOD;

Setting a default controller and/or method seems like something that should be accessible as a public method of our Application:

public function setDefaultController($controller)
{
 $this->defaultController = $controller;
}

this gives us some extra sugar:

$app = new Application($url);
//If everything else failes
//we have a simple controller that
//can handle every url and prints out a small error message to the user
//and sends an email to support
$app->setDefaultController('SimpleController');
try {
 //do some heavy lifting, i.e. instantiate the database connection
 //Now that we have access to a lot of sugar
 //Our default controller can be a little more fancy
 //We now show random kittens from our database - sweet
 $app->setDefaultController('FancyController');
} catch($e)
{
 $app->run();
}

Design remarks Your Application is in fact some kind of Router. It maps urls to controller::method(params). So why not call it a Router?

---

Good reads

A very good read is how the Symfony framework handles a Request:

• Symfony http kernel

A good reads on patterns:

• Misuse

And a last one, more of a amusing one but with a good underlying message:

• Programming sucks