MVC Refactor, Take 2

I would say mostly no, you’re not wrong. What happened is, the original MVC was designed for desktop software (rich, interactive clients), which isn’t perfectly suited to request-response server side web applications. So the pattern that we in the PHP world call MVC is actually an adaptation of the original for the web (hence in this topic we refer to it as Web MVC). Tom believes there’s a better way to adapt the pattern, and he’s been advocating his alternative for a while, so in this topic we’re trying to compare advantages and disadvantages.

So I would say you’re not wrong because despite the discussion here, your understanding of what MVC is (what we call Web MVC) is the industry standard* meaning of MVC.

* the web industry standard

1 Like

Ah I see, so it’s like a derivative of the original MVC pattern? I’ve seen some of his explanations and it sounds like he’s saying that it’s actually ok to call the view files from the model files correct? I’ve seen how the hierarchy works and I believe it does sound like how Tom is explaining. However the way we use MVC is totally different. What made it this way may I ask?

Tom’s demonstrated concept has the model being injected into the view. It is completely different, because the controller doesn’t work with the view directly.

And from what I see, it is now the router that controls the view. The router is no longer just a router, but more a coordinator between the “MVC Triad”. So instead of having most of the coordination logic in the controller, it is now being put into the router.

I am not sure I like that.

As I see it, the domains we are juggling with here are these:

  1. The domain of the application system. How to model the components of the web to get everything to work properly. This is like working with things like the GET and POST parameters along with the need for a router to put them in the right place for further processing.

  2. The application architecture domain. How to be an application for a web based system. This what we are trying to do with web MVC, or TomBZombie MVC. This is the most abstract and highest level.

  3. The application’s own domain. How to work, so an application can do its intended purpose. This is the form example we are talking about currently. It is the most concrete and lowest level and also deals with user land.

Symfony, I feel, does a great job of point 1.

So, in the same manner, I’d like to suggest, we need to get from HTTP to MVC. We should actually include something like Symfony’s HTTPKernel in the beginning. With it, we end up with just a request object and need a response object to send back out. That is all any MVC system should really need to work with. However, we need an entry point to the MVC system. Most frameworks use the controller, as I see it, mainly to model the older page controller concept we had before such frameworks.

However, we do need to go from “web” to “MVC”.

So let’s say we have an HTTP frontend like Symfony’s and we now can have this request formulated into an object. Now we need to determine what needs to happen internally to come up with a response. Symfony does this by routing to controllers, which coordinate between the models and views.

And I agree with Tom here. Controllers, as coordinators, are doing too much.

So, should we be doing more in the HTTP stage? I think yes. The request object should be structured in such a way, that we only need 4 controllers. CREATE, READ, UPDATE and DELETE. These are the main methods HTTP can give us to “act” on.

I say, let’s still stick to controllers as the entry point to our MVC.

A READ controller would be the easiest. It takes a request, calls a view and returns a response. Although this goes against Tom’s "Why should a read request need a controller, this form of a controller is so simple and keeps the architecture uniform.

That leaves us with CREATE, UPDATE, and DELETE. These are all write actions. So why not just have a “WRITE” controller class, with CREATE, UPDATE and DELETE methods? These fire off respective model manipulations.

Once done, the view is called to do its work, for the respective request, which it can form the necessary models to build a proper response.

So, let’s take a step back and now just have a controller class with these 4 methods and at the end, the call to the view to render.

I do believe, this is a much better separation of concerns. Isn’t it?

So, basically, what I am suggesting is now we have userland only working with models and views. They concentrate strictly on business logic and visualization. They also might have some control over the HTTP’s building of the request object, like in filtering the URL properly, so it matches their SEO needs. This should be userland configuration. Important to note, this part of the HTTP domain isn’t mixed up with our MVC at all and doesn’t affect any controller actions directly.

What about authentication and authorization? The only thing we are protecting is data, right? And where is the data found? In the model. So, rightfully, the first thing any model activity will do is check to make sure it can do the work it has been ask to do for the person asking for it.

I know this was very theoretical, But, couldn’t this be feasible?

Scott

1 Like

Oh, I see. So basically, it’s CRUD mixed with the web MVC? Also, may I ask what is the difference between using namespaces in an MVC application compared to an MVC application that doesn’t use namespaces?

I ask that because you mentioned Symfony quite a few times and I know for sure their whole code style is written using namespaces. Does it matter if one doesn’t use namespaces?

Namespacing is a method in PHP to encapsulate groupings of code and has nothing to do with our discussion really. And yes, it would make sense to use namespaces, as it is currently a best practice(this linked page links to the PHP.FIG PSRs, which you couldn’t follow without namespacing.)

My suggestion is to basically create “standard” controllers and isn’t exactly what Tom is suggesting…I think, but I think a good approach to properly modularizing any user land code.

The view, is responsible solely for the return of the response. It gets its data and state from the model.

The model is responsible purely for the business logic. Attached to it would be any necessary persistence. This includes session management, database abstraction, etc.

The controllers are only there to bring the model and view together and as the entry point into the MVC triad.

Scott

And this causes a couple of new potential issues:

  • The controller now has a dependency on $_SERVER to check X_HTTP_REQUESTED_WITH, if you’re checking that request header in the controller why check $_SERVER[‘REQUEST_URI’] in the router to work out $format to pass it in to the controller? Just read all $_SERVER variables in the controller. Of course this makes the router rather redundant.

  • Repeated code. Replace conditional with polymorphism is a tried and tested refactor, yo’ve done the exact opposite here, replacing polymorphism with a conditional ( https://sourcemaking.com/refactoring/replace-conditional-with-polymorphism ). The problem with your refactor here is that the controller and the router need to be amended when a new format is added, breaking SRP (The reason to change is adding a new format, in my example this can be done in 1 place: the router, yours needs two changes one in the preg_match in the router and one in the controller)

What you haven’t stated is why this approach is better, only pointed out that it can be done in Web MVC.

But now we’re getting into implementation details rather than architectural advantages/disadvantages. The only disadvantage of this approach your listed is an irrelevant implementation detail: “doesn’t support yaml/xml”. I disagree that it’s either relevant or true:

<route>
 <match>/new</match>
 <method>POST</method>
 <controller>CreateController</controller>
 <model>Model</model>
 <view format="html">
  <class>HtmlView</class>
  <arg>newThing.html.php</arg>
</view>
 <view format="json">
  <class>JsonView</class>
 </view>
</route>

There are dozens of ways of doing this, my point is that saying “It cannot be expressed in XML” is untrue.

Of course, but it’s significantly less coupled than web mvc where the controller is tightly coupled to the view. In this approach the view is agnostic about the model and the template is loosely coupled to the model, the difference here is I can swap out the model/template while using the same controller and view, in web mvc it’s impossible to change the template without changing the controller forfeiting reuse of the controller.

But my point is that this knowledge (effectively broken encapsulation) isn’t needed, there is no reason for the controller to know implementation details about the view (variable names, data structures). Let’s flip it around: What is the advantage of tightly coupling the controller to the view?

If you have 100 controller actions the developer has to write 100 lines that start $this->templating->render passing it a long set of arguments (These are not short or simple lines either). From a purely practical perspective refactoring this repetition out saves time. From a theoretical perspective the components are decoupled and therefore more modular.

I replaced your Template class with a View class so no, it’s not equivalent, there are fewer classes defined in my example, less work for the developer, and because controllers are reusable, less work for the developer going forward.

You didn’t answer my question: What is the benefit of constructing a controller, invoking a controller action and having that render a view rather than just rendering the view? In your example instead of writing one line of code new View($model) the developer has to write:

$controller = new NewController($model);
$response = $controller->doAction();



class NewController
{
    private $templating;

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

public function doAction()
{
    return ['headers' => [], 'body' => $this->templating->render('newThing.html.php')];
}
}

How is that better? It takes significantly more time and requires opening up two different files.

This is based on the example you provided. Saying “Web MVC can do that too” isn’t really helpful. You provided an example, I demonstrated where it could be improved, the only assumption I made was that your example is demonstrating the architecture you’re trying to defend. If that’s not the case, what is the point in the example?

One reason to change is returning a different content-type. In your example above you need to change both the router and the controller to handle this. Each time you add a new response format you must alter both the router and the controller. This is a clear SRP violation and from a practical perspective means the developer takes more time.

And the network adapter gets those TCP packets as a series of 1s and 0s, what’s your point? This is a non-sequitur. In PHP we don’t need to worry about the network stack only the HTTP Protocol. Cache-control, Gzip are certainly display logic and belong in the view. Again, I go back to asking what is the benefit of putting this in the controller?

Your post is heavy on “This can be done in Web MVC” but very light on “These are the advantages of doing it this way” so here’s a couple of direct questions:

###What is the advantage of coupling the response and view to the controller?

The disadvantages are numerous: Repeated $this->render lines and irrelevant empty controller classes when the controller is just rendering a template which makes it impossible to reuse the controller

You already have a router that has the following responsibilities:

  • Selecting a controller
  • Selecting a model
  • Selecting and calling a controller action

What is the disadvantage of expanding this slightly to also select a view?

What is the advantage of the binding logic between controllers and templates?

Having a controller extract data from a model and pass it to a view seems redundant to me and introduces binding logic which doesn’t need to exist. Why not just let views access models? This also means that a controller is necessary even if it’s just rendering a template

What are the advantages of web mvc over my proposed alternative? (Which if we’re honest is closer to MVC than “Web MVC” ever has been)

In the version I supplied development time was lower due to removal of binding logic, controllers which only render a view and being able to re-use controller code. These are clear advantages, as far as I can tell your post made no such claims for web mvc.

Let’s be clear here, the only difference in my example is that the router has one additional job: Selecting a view. Considering it already has to select a model, a controller and a controller action what is the disadvange of having it also select a view? And considering this can be automated if needed I’m not sure I understand why this seems to be such a big point of contention.

Let’s turn this around: By giving the router this one extra task, the controller and view can be decoupled, the benefits of this are numerous so what is the disadvantage of this? The router is slightly more complex, but I’m not sure this is an argument against it given that a more complex router doesn’t present any problems itself while opening up a greater level of flexibility in picking/choosing the other components. This seems to go back to the DI vs new in constructor type argument.

This is an interesting idea. Pretty much along the lines of what I was going for but a bit more HTTP specific. That said there will be a lot of redundancy between create/update as they’re pretty much the same thing.

I am completely with you on this. In fact extending the original example here, this is pretty much how I handle it. By having a proper MVC model, this actually becomes very easy:

interface RestrictedAccess {
  public function canView();
}

index.php


<?php
require_once 'controllers.php';
require_once 'view.php';

// Route
if ($_SERVER['REQUEST_URI'] === '/new' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$model = new Model();
	$view = new View('newThing.html.php', $model);	

} else if ($_SERVER['REQUEST_URI'] === '/new' && $_SERVER['REQUEST_METHOD'] === 'POST') {
	$model = new Model();
	$controller = new CreateController($model);
    $controller->createAction($_POST);
    $view = new View('newThing.html.php', $model);
} else {
    // 404; not implemented
    exit;
}


if ($model instanceof RestrictedAccess) {
    if (!$model->canView()) {
        //Show an error message and return a 401 header
        $view = new View('access-denied.html.php');
    }

}

$response = $view->render();

// Send response
// Assumes response in the form of ['headers' => [...], 'body' => ...].
foreach ($response['headers'] as $header) {
    header($header);
}
echo $response['body'];

Obviously this example will still call the controller action but you get the idea, in a proper router this would be done via configuration file and the check would be done between “create model” and “create controller” steps.

I don’t think authentication and authorization belongs in the controller at all. I prefer the way Laravel handles this using middleware facilitating DRY and composition over inheritance.

I think there is going to also be unnecessary code duplication, once we start expanding the application. If you already look at our little bit of code.

if ($_SERVER['REQUEST_URI'] === '/new' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$model = new Model();
	$view = new View('newThing.html.php', $model);	

} else if ($_SERVER['REQUEST_URI'] === '/new' && $_SERVER['REQUEST_METHOD'] === 'POST') {
	$model = new Model();
	$controller = new CreateController($model);
    $controller->createAction($_POST);
    $view = new View('newThing.html.php', $model);
} else {
    // 404; not implemented
    exit;
}

There is two “new Model()” and “new View()” calls. The router isn’t dumb and it is doing too much. Why do we need a router or coordinator to begin with? That is my question.

But then the view is coupled to the router. No advantage gained really, is there?

Actually, when I started writing my post, I was thinking along the lines of the read request going straight to the view. However, I said to myself, if we have a very generic controller, we don’t need a router. Our entry point to the MVC triad would still be the controller (still, as in, compared to current frameworks). The controller would actually only need to determine between reads and writes. Read is a direct pass on to the view to create the response. Write is a pass to the model to manipulate the new state and store it. Once that is done, then the view is again called as the last action to recall the new state.

From my position, what I am missing is how the request can get to the view, but without a thick/ fat routing mechanism and how the model can invoke the view on updated state.

My basic concern isn’t with giving a router the call to the view. It is the router itself. There is no such thing as a router in MVC, is there? I am questioning the need for one altogether. To me, the router is an invention, so MVC can allow the old style page controller pattern, and actually, if you read that article, it exactly what most PHP frameworks are trying to do.

If you think about the evolution of PHP, we went from single pages called with something like /showthread.php?t=1234 to /thread/1234. The showthread.php had all kinds of redundancy in it compared to all the other files, like calling some global.php file, calling other needed standard data, and calling some or directly sending some template or HTML. The only thing that made it different was the page logic. Each page was basically its own controller calling on some data persistence (part of the model) and some view. That is what Symfony tries to also follow, it seems, and it needs the router to get the right controller, model and view. We need to get away from that and the router is the first thing I call “foul” on. :smile:

Scott

This is what I am looking for.

Scott

We’re looking at the same code, right? The controller does not need to be amended for a new format. It has no tests nor special behavior for xml or json, which means we could add yaml or ini and the controller would’t care. The router, however, would have to be amended. As would yours.

Are the arguments different in each of the 100 templates? If so, then you too will be writing 100 distinct model classes, each providing the exact properties that one of the templates requires. Or are the arguments the same for all 100 templates? Then you can be damn sure we’ll be reusing that controller. In fact, that’s exactly what we did with the format example. Five different URLs, five different responses, one shared controller. The number of formats could have been 100 and the controller would still work the same.

  1. In Web MVC, the router does not select the model. I’m honestly not sure where you got that idea from.
  2. Selecting the controller and the action are the same thing. What we’re actually selecting is a callable. Doesn’t matter if it’s a method or a closure or a plain function.

Well there’s a loaded question if I ever heard one.

The point is you’re claiming you can do better. But your model is still coupled to the template (the model needs to provide the exact same properties the template is intending to use), and the controller is coupled to the model (the controller needs to invoke a particular set of methods for the model to initialize its distinct, template-dependent properties).

I’m comparing these approaches from a pragmatic point of view, not academic. We can’t only judge these approaches in the abstract. Ultimately the implementation details are what we will be dealing with on a day-to-day basis. If a particular approach unintentionally imposes some undesirable implementation details, that matters.

What I said was it can’t be expressed easily.

Your approach with 5 formats:

 <view format="html">
  <class>HtmlView</class>
  <arg>newThing.html.php</arg>
</view>
 <view format="json">
  <class>JsonView</class>
  <arg>newThing.json.php</arg>
</view>
 <view format="xml">
  <class>XmlView</class>
  <arg>newThing.xml.php</arg>
</view>
 <view format="yaml">
  <class>YamlView</class>
  <arg>newThing.yaml.php</arg>
</view>
 <view format="ini">
  <class>IniView</class>
  <arg>newThing.ini.php</arg>
</view>

The Web MVC approach I posted earlier with 5 formats:

'#^/new(?:\.(html|json|xml|yaml|ini))?$#'

That’s literally it. The difference is you moved logic from the controller to the router, so now your router config has to make decisions and handle tasks that it isn’t well adapted to handle.

That we know about the strategy pattern doesn’t mean if-statements are suddenly evil. We don’t need to eradicate every conditional we see. In the Web MVC controller, the if-statement appears just the once (not repeated), and the body of the conditional is a one-line invocation of other code that has already been factored out (no complicated algorithms). Polymorphism won’t gain you anything in this spot.

Knowing the arguments something takes is not broken encapsulation.

Realistically, you would still need a template class. Unless you plan to bake twig/blade/smarty/PHP template engines directly into each of your view classes. And so far, that’s exactly what you’ve done. The logic to render a PHP template is baked into your view class.

I’m saying it’s the same either way. “Six of one, half dozen of the other.” Sounds like we have two choices but they’re actually the same thing. Either the router invokes a view class that renders a template, or it invokes a controller class that renders a template. They’re functionally equivalent. All that changed is the name.

It’s simpler and more straightforward (or at least I think so). Enterprisey software is sometimes the butt of a joke, so I would hope that where we add more layers of indirection, abstraction, or separation, that we can clearly demonstrate a use case where it improves our code. Based on the comparison we’ve done so far (new/create, ajax, formats), I don’t see the improvement.

Once you start wanting to wrap the output in a consistent header/footer the central entry point becomes clearer.

The second reason is reuse. Another template can call router('/path/'); to include the output of another template after routing without needing to repeat information or provide the template with extra information.

Duplication is an implementation detail. I specifically provided configuration foe each route for simplicity, there’s no reason to assume we can’t use a convention-over-configuration as I suggested in #18 where we take /new and then load NewModel, infer the controller type from the model interface and use `new View(‘new.html.php’,$model);

Then we only need configuration where this default configuration isn’t sufficient.

Well yes the result of this is that everything is tightly coupled to the router but everything is is either completely decoupled or loosely coupled from everything else. With Jeff’s example the controller is tightly coupled to the model and view. This means that controller logic cannot be used easily with alternative views or models.

Apologies, I said route and controller when I meant route and view. By putting state in the model it can be serialized without having to provide binding logic for a new.json.php file we just use json_encode($model); which is reusable for any model. Need more control over what is serialized? Implement JsonSerializable

But that’s not the case is it? Consider two different templates that show the same information in a different way. They all have the same variables but just load a different template. If your example you have a hardcoded template name. $this->template->render('newThing.html.php', $args)

To reuse the model and controller with a different template you need to repeat this line or write manually abstract it to another method in the controller that has:

And you’ll need to provide this abstraction in each of the controllers you have multiple templates for. You will either need to add an if/else to the controller action or configure the template to use in the view and pass it as an argument to the constructor which in your view is making the router do too much so I’d assume you’d do this:

// Route
if ($_SERVER['REQUEST_URI'] === '/new' && $_SERVER['REQUEST_METHOD'] === 'GET') {
    $controller = new NewController(new Templating);
    $response = $controller->doAction($post);
}
else if ($_SERVER['REQUEST_URI'] === '/new-other-template' && $_SERVER['REQUEST_METHOD'] === 'GET') {
    $controller = new NewController(new Templating);
    $response = $controller->doActionNewTemplate($post);
}

I’m going to assume you’d abstract a lot of the logic and do this:

class CreateController
{
	private $templating;
	private $model;

	public function __construct(Template $templating, Model $model)
	{
	    $this->templating = $templating;
	    $this->model = $model;
	}

	private function insert($post, $template, $redirectUrl) {
		$violations = $this->model->validateThing($post);
	    if (empty($violations)) { // if valid
	        $this->model->insertThing($post);

	        return ['headers' => ['Location: ' . $redirectUrl], 'body' => ''];
	    }
	    return ['headers' => [], 'body' => $this->templating->render($template, ['entity' => $post, 'violations' => $violations])];
	}

	public function doAction($post)
	{
		return $this->insert($post, 'newThing.html.php', '/somewhere-else')	    
	}

	public function doActionNewTemplate($post)
	{
	    return $this->insert($post, 'newThing2.html.php', '/somewhere-other-else')	  
	}
}

The obvious problem? The controller has to have an action for each possible template and a route needs adding for each template. Alternatively the controller needs to read from $_SERVER and do its own routing to make the different templates appear on their own URLs.

Which brings me to my next point:

  1. In Web MVC, the router does not select the model. I’m honestly not sure where you got that idea from.

I’m only working from the example we have been using in this thread. However, as you say it usually doesn’t which itself removes flexibility. Let’s update the controller to do this:

class CreateController
{
	private $templating;
	private $model;

	public function __construct(Template $templating)
	{
	    $this->templating = $templating;
	    $this->model = new Model;
	}

	private function insert($post, $template, $redirectUrl) {
		$violations = $this->model->validateThing($post);
	    if (empty($violations)) { // if valid
	        $this->model->insertThing($post);

	        return ['headers' => ['Location: ' . $redirectUrl], 'body' => ''];
	    }
	    return ['headers' => [], 'body' => $this->templating->render($template, ['entity' => $post, 'violations' => $violations])];
	}

	public function doAction($post)
	{
		return $this->insert($post, 'newThing.html.php', '/somewhere-else')	    
	}

	public function doActionNewTemplate($post)
	{
	    return $this->insert($post, 'newThing2.html.php', '/somewhere-other-else')	  
	}
}

Yes it might go off to some kind of Service Locator to do this that constructs the model but for the sake of argument let’s just have the controller construct the model. This is no different to $this->model = $this->container->get('Model') as both will result in a fully constructed instance of the Model class.

Big problem! Now the model is hardcoded and cannot be substituted. What if I want to use the same from and write to a CSV rather than a Database? Well I need to do this:


class CreateController
{
	private $templating;

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

	private function insert($post, $template, $redirectUrl, $model) {
		$violations = $model->validateThing($post);
	    if (empty($violations)) { // if valid
	        $model->insertThing($post);

	        return ['headers' => ['Location: ' . $redirectUrl], 'body' => ''];
	    }
	    return ['headers' => [], 'body' => $this->templating->render($template, ['entity' => $post, 'violations' => $violations])];
	}

	public function doAction($post)
	{
		return $this->insert($post, 'newThing.html.php', '/somewhere-else', new Model);  
	}

	public function doActionNewTemplate($post)
	{
	    return $this->insert($post, 'newThing2.html.php', '/somewhere-other-else', new Model);
	}

	public function doActionInsertIntoCsv() {
		return $this->insert($post, 'newThing.html.php', '/somewhere-other-else', new ModelInsertsCsv);
	}
}

Of course to use ModelInsertsCsv with newThing2.html.php then I need another controller action. What if I want to use an Ajax call that returns Json with ModelInsertsCsv? Then I really need to do some work as I need to either copy/paste the insert method or re-write the abstracted insert() method.

And the obvious problem here is that I need to alter the insert method the first time I want to use a different model, and I need to create the insert method the first time I find that the original controller doesn’t offer enough flexibility. This is all considerably more work than having a reusable controller in the first place.

All of this breaks the open/closed principle in that it’s impossible to extend the behaviour of the class without modifying it. Using my approach the controller is defined, has a model injected (which can write to a CSV, database or if you like, post the results on twitter), the controller doesn’t care and doesn’t know anything about the template or view being used (or the format of that response).

Essentially your controllers are not reusable, at all without modification. The controllers need to know about all the ways in which the data can be stored (or retrieved from) as well as all the templates which might be used.

But your model is still coupled to the template (the model needs to provide the exact same properties the template is intending to use), and the controller is coupled to the model (the controller needs to invoke a particular set of methods for the model to initialize its distinct, template-dependent properties).

The difference, of course, is that the controller is not required in my example. I can provide two different models:

class NewsModel {
	public function getNews() {
		return $this->orm->sort('date desc')->limit(5);
	}
}
class NewsModel {
	private $results; 

	public function search($search) {
		$this->results = $this->orm->filter(['title' => $search]);
	}

	public function getNews() {
		return $this->results;
	}
}

One needs a controller one doesn’t. To do this in your example you need to add a controller action for each, selecting the same template but populating the results from a different place.

And realistically you’ll need to write a template wrapper or your controller action can only work with a specific template system. if ($this->template) is part of the twig library you will find it difficult to re-use the controller logic with a different template system. And this brings us back to the “coupling” you mentioned. By giving the template access to the model, rather than pushing data-structures to the template, any template system can work with any specific model, that is not the case if you’re calling $template->render(...); and two template systems have different APIs, you need to change the method call for each system.

But you’re making assumptions here, it’s easy enough to make $foo.json trigger the relevant view in the router using a convention-over-configuration approach and remove the router rules entirely.

But again, you’re either adding a single line to the router: $view = new View('x.tpl.php'); (Or using a convention-over configuration approach as I described in #18 and only supplying the template) or you’re writing a whole new class and potentially altering the router:

class MyNewTemplateController {
	private $templating;

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

	public function doAction() {
		$this->templating->render('x.html.php');
	}
}

Exactly how is that “simpler”?

I don’t see the improvement.

Writing less code is an improvement, surely?

The output can be more consistent. because of a central entry point? I know what you mean, but if you actually take that at face value, it makes little sense.

I would think the view should build the page template from a central file. Within the template (and through the templating sub-system) there would be includes for any extraneous components needed for the page’s rendering. Any subsequent calls to any other models could also be done.

I still think we can do without a router. I am working on a bit of example code and I am actually making it to model this.

Scott

I’d be interested in seeing this, but I don’t see how this is possible. Somewhere you need new View() and new Controller and some information about which HTTP request requires which controller and view.

I wanted to add a new simpler example that highlights the problem with redundant controllers causing duplicate code when there’s no user interaction.

Consider the following Web MVC system based on Jeff’s most recent posts. All it does is display the time from the model. The developer will need to add the following lines to the following files:

clock.html.php


The time is <strong><?= $time->format('H:i:s'); ?></strong>

and a model that gets the time:

clockmodel.php

class ClockModel {
	public function getTime() {
		return new DateTime();
	}
}

The controller looks like this:

clockcontroller.php


class ClockController {
	private $template;

	public function __construct(Template $template) {
		$this->template = $template;
	}
	
	public function viewAction() {
		$model = new ClockModel;
		return ['headers' => '', 'body' => $this->template->render('clock.html.php', ['time' => $model->getTime()]);
	}
}

index.php

(showing only lines that were added)


else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$controller = new ClockController(new Template);
    $response = $controller->viewAction();
}

Using ToMVC™ you get the following:

clockmodel.php

class ClockModel {
	public function getTime() {
		return new DateTime();
	}
}

clock.html.php


The time is <strong><?= $model->getTime()->format('H:i:s'); ?></strong>

index.php

(showing only lines that were added)

else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$view = new View('clock.html.php', new ClockModel);
}

Again, I’m just trying to show the amount of work done in userland. There is a lot more code to write to do this in Web MVC.

Ok so what about making changes? Let’s add a version that shows the date on /date. Using Web MVC (again showing only lines added)

date.html.php


The date is <strong><?= $time->format('d/m/Y'); ?></strong>

clockcontroller.php

	public function viewDateAction() {
		$model = new ClockModel;
		return ['headers' => '', 'body' => $this->template->render('date.html.php', ['time' => $model->getTime()]);
	}


index.php


else if ($_SERVER['REQUEST_URI'] === '/date' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$controller = new ClockController(new Template);
    $response = $controller->viewDateAction();
}

And in ToMVC (again showing only lines which need to be added to make this work):

date.html.php


The date is <strong><?= $model->getTime()->format('d/m/Y'); ?></strong>

index.php

else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$view = new View('date.html.php', new ClockModel);
}

Ok let’s say I want a variant that gets the time from an NTP server instead of just using the inbuilt datetime object

ntpmodel.php


class NtpModel {
	public function getTime() {
		//connect to NTP server and return a DateTime instance
	}
}

In Web MVC I now need to make the following changes:

clockcontroller.php

	public function viewDateNtpAction() {
		$model = new NTPModel;
		return ['headers' => '', 'body' => $this->template->render('date.html.php', ['time' => $model->getTime()]);
	}

	public function viewTimeNtpAction() {
		$model = new NTPModel;
		return ['headers' => '', 'body' => $this->template->render('clock.html.php', ['time' => $model->getTime()]);
	}

index.php

else if ($_SERVER['REQUEST_URI'] === '/date-ntp' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$controller = new ClockController(new Template);
    $response = $controller->viewDateNtpAction();
}
else if ($_SERVER['REQUEST_URI'] === '/clock-ntp' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$controller = new ClockController(new Template);
    $response = $controller->viewTimeNtpAction();
}

Whereas using ToMVC, you once again have fewer changes:

ntpmodel.php

(the same)

class NtpModel {
	public function getTime() {
		//connect to NTP server and return a DateTime instance
	}
}

index.php

else if ($_SERVER['REQUEST_URI'] === '/date-ntp' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$view = new View('date.html.php', new NtpModel);
}
else if ($_SERVER['REQUEST_URI'] === '/clock-ntp' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$view = new View('clock.html.php', new NtpModel);
}

Hopefully this example is a bit clearer.

Yes, the view is called in the HTTP Converter. It passes the request object to the view. The view then extracts the requested (base) object from the request object (which is normally found in the URL i.e. post, thread, blog, user, dog, cat, movie, etc., etc.). The requested base object determines which model needs to be called to fill the view with data and also which view needs to be generated.

Edit: The controller is also called in HTTP Converter, if the request method is anything other than a GET.

I’ll be honest and say, I am flying by the seat of my pants here. So, my code/ idea could be very possibly a load of crapola. I hope to finish it this evening.

Scott

And to take this example further still, let’s take the same idea and extend it so that the user can select a city from a <select> and it will display the time in that city.

Using Web MVC:

clock.html.php


	<form action="/clock" method="post">
	<label>Select a city</label>
	<select name="timezone" onchange="this.form.submit();">
	<?php foreach ($timezones as $timezone => $cityName) {
	?>
	<option value="<?= $timezone; ?>" <?= (isset($city) && $city === $cityName ? 'selected="selected"' : ''); ?><?= $city; ?></option>
	<? } ?>
	</select>
	</form>

<?php
if (isset($city)) {
	?>
	The time in <em><?= $city; ?></em> is <strong><?= $time; ?></strong>
	<?php
}
?>

clockmodel.php

class ClockModel implements FormModel {
		
	public $timezones = ['Europe/London' => 'London', 
						 'America/New_York' => 'New York', 
						 'America/Los_Angeles', 'Los Angeles'];

	public function getTime($timezoneStr) {
		$timezone = new DateTimeZone($timezone);
		$date = new DateTime;
		$date->setTimeZone($timezone);
		return $date;
	}

	public function isValid($timezone) {
		return isset($this->timezones[$timezone]);
	}

	public function getCity($timezone) {
		return $this->timezones[$timezone];
	}
}

clockcontroller.php


class ClockController {
	private $template;

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

	public function viewAction() {
		$model = new ClockModel;
		return ['headers' => '', 'body' => $this->template->render('clock.html.php', ['timezones' => $model->timezones]);
	}

	public function submitAction($post) {
		$model = new ClockModel;

		if ($model->isValid($post)) {
			return ['headers' => '', 'body' => $this->template->render('clock.html.php', 
													['timezones' => $model->timezones, 
													'city' => $model->getCity($post['timezone']), 
													'time' => $model->getTime($get['timezone'])]);
		}
		else return $this->viewAction();
		
	}
}

index.php

else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$controller = new ClockController(new Template);
    $response = $controller->viewAction();
}
else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'POST') {
	$controller = new ClockController(new Template);
    $response = $controller->submitAction($_POST);
}

By Comparison, in TomVC

I’m going to assume the framework provides this interface and class, they’re basically the ones I presented earlier but previously used Jeff’s less generic function names.

interface FormModel {
	public function submit($data);
	public function isValid($data);
	public function success();
}


class FormController {
	private $model;

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

	public function submit($data) {
		if ($this->model->isValid($data)) {
			if ($this->model->submit($data)) {
				$this->model->success();
			}
		}
	}
}

So the developer would provide:

class ClockModel implements FormModel {
		
	public $timezones = ['Europe/London' => 'London', 
						 'America/New_York' => 'New York', 
						 'America/Los_Angeles', 'Los Angeles'];

	public $timezone;
	public $submitted = false;
	

	public function isValid($post) {
		return isset($this->timezones($post['timezone']));
	}

	public function submit($data) {
		$this->timezone = $data['timezone'];
	}

	public function success() {
		$this->submitted = true;
	}

	public function getTime() {
		$timezone = new DateTimeZone($this-timezone);
		$date = new DateTime;
		$date->setTimeZone($timezone);
		return $date;
	}

	public function getCity() {
		return $this->timezones[$this->timezone];
	}
}

clock.html.php


	<form action="/clock" method="post">
	<label>Select a city</label>
	<select name="timezone" onchange="this.form.submit();">
	<?php foreach ($model->timezones as $timezone => $city) {
	?>
	<option value="<?= $timezone; ?>" <?= $model->timezone === $timezone ? 'selected="selected"' : ''); ?><?= $city; ?></option>
	<? } ?>
	</select>
	</form>

<?php
if ($model-submitted) {
	?>
	The time in <em><?= $model->getCity(); ?></em> is <strong><?= $time; ?></strong>
	<?php
}
?>

index.php

else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'GET') {
	$model = new ClockModel;
	$controller = new ClockController($model);
    $view = new View('clock.html.php', $model);
}
else if ($_SERVER['REQUEST_URI'] === '/clock' && $_SERVER['REQUEST_METHOD'] === 'POST') {
	$model = new ClockModel;
	$controller->submitAction($post);
    $view = new View('clock.html');
}

And just to be clear, the router could be a lot smarter than this since there’s a finite number of reusable controllers:


$name = trim($_SERVER['REQUEST_URI'], '/');

if (class_exists($name . 'model')) {
	$model = new ($name . 'model');
	if ($model instanceof FormModel) $controller = new FormController($model);
	if (file_exists($name . '.html.php')) $view = new View($name . '.html.php', $model);
}

if ($controller instanceof FormController) {
	if ($_SERVER['REQUEST_METHOD'] == 'POST') {
		$controller->submitAction($_POST);
	}
}


And only require explicit configuration when convention isn’t used.

In OOP imagine this being set up to be extensible using something like

$router->registerType('FormController', 'FormModel', function() {
	if ($_SERVER['REQUEST_METHOD'] == 'POST') {
		$controller->submitAction($_POST);
	}
});

In which case, the user wouldn’t even need to supply a route. I realise this is possible with web mvc as well but just wanted to demonstrate that it’s not exclusive to it.

Ok, here goes nothing! :blush:

First, to get our entry point, I changed index.php to be app.php and set up my webserver (nginx php-fpm) accordingly, so all calls go to app.php. I also used Tom’s last clock example for the example, to make sure it works.

app.php

<?php

/*
 * Generic entry point/ front controller.

 */

require_once('HTTPConverter.php'); #


$httpConverter = new HTTPConverter();
$request = $httpConverter->createRequest();
$response = $httpConverter->handleRequest();
$httpConverter->sendResponse();

Simple enough. This is similar to what Symfony does for a front controller, but we won’t be going into the depth that Symfony does. As you can see, we need the httpConverter. So here it is.

HTTPConverter.php

<?php


/*
 * A converter to convert HTTP to a request object and handle it to get a response
 *
 */

require_once ('View.php');
require_once ('Controller.php');
require_once ('Request.php');
require_once ('Model.php');

class HTTPConverter
{

    private $request;
    private $response;
    private $controller;
    private $model;


    public function __construct()
    {

        // prepare anything necessary

    }


    public function createRequest()
    {
        // create the request object
        $this->request = new Request();

    }

    public function handleRequest()
    {

        $this->model = new Model($this->request);

        // if the request is a write request (i.e. a post or put), do the controller first
        if ( $this->request->isWrite() ) {
            $this->controller = new Controller($this->request, $this->model);
        }

        // you'll always need a response, so call the view to get it.
         $this->response = new View($this->request);
    }

    public function sendResponse()
    {
        echo  $this->response->render($this->model);

    }

}

Ok, this is a basic (front) controller and it should stay that way! :smiley: But, we are also calling to build a request object. The request object is important, because it is needed to hold the data for all the other object’s needs. We’ll see what I mean in a second.

Request.php

<?php

/*
 * Create the Request object from global variables.
 */

class Request
{
    public $object;
    public $method;
    public $path;
    public $postData;

    public function __construct()
    {
       $this->object = trim($_SERVER['REQUEST_URI'], '/');
       $this->method = $_SERVER['REQUEST_METHOD'];
       $this->filterPostData();
       $this->setMethod();

    }


    public function isWrite()
    {

        return $this->method == 'update' OR $this->method == 'create' OR $this->method =='delete' ? true : false;

    }

    public function filterPostData()
    {
        //no filtering yet, but you get the point
        $this->postData = $_POST;
        return $this->postData;
    }

    public function setMethod()
    {
        switch ($this->method) {
            case 'POST':
                $this->method = 'update';
                break;

            case 'PUT':
                $this->method = 'create';
                break;

            case 'DELETE':
                $this->method = 'delete';
                break;

            case 'GET' :
                $this->method = 'read';
                break;

            default: 'read';
        }
    }



}

Ok, here we build the request object. Obviously, this isn’t the way it should be done, but here is where we could put logic from the client, as to how to extract the HTTP data, so it fits for rest of the framework. i.e. the SEO friendly URL format.

So, now we end up back in the httpConverter and need to control our MVC framework.
(snippet from HTTPConverter.php above)

    // if the request is a write request (i.e. a post or put), do the controller first
    if ( $this->request->isWrite() ) {
        $this->controller = new Controller($this->request, $this->model);
    }

    // you'll always need a response, so call the view to get it.
     $this->response = new View($this->request);
}

As you can see, depending on what is being requested, we do either the controller first, then the view, or do just the view.

So, let’s look at the view.

View.php

<?php

/*
 * View class for creating an output
 *
 */

class View {

	private $template;
	private $model;
        public $headers;


	public function __construct(Request $request)
        {
            $this->template = $request->object . ".html.php";
	}

	public function render($model)
	{
            $headers = [];
            ob_start();
            require ( "./templates/".$this->template );

            return  ob_get_clean();
	}
}

As you can see here, we are calling on a template, but built from the object we sent though the url i.e. www.mysite.com/clock. So, the object is “clock”.

Here is the template.

clock.html.php

 <p>See what time it is!</p>

	<form action="/clock" method="post">
	<label>Select a city</label>
	<select name="timezone" onchange="this.form.submit();">
            <option value="">Pick one!</option>
	    <?php foreach ($model->clientModel->timezones as $timezone => $city) {
	    ?>
	    <option value="<?php echo $timezone; ?>" <?php echo $model->clientModel->timezone === $timezone ? 'selected="selected"' : ''; ?>><?php echo $city; ?></option>
	<?php } ?>
	</select>
	</form>

<?php
if ($model->clientModel->submitted) {
	?>
	The time in <em><?php echo $model->clientModel->getCity(); ?></em> is <strong><?php echo $model->clientModel->getTime(); ?></strong>
	<?php
}
?>

We have one difference to Tom’s and maybe this might be over complication, but we now have an object in our model called clientModel.

Model.php

<?php

/*
 * A class for our model (building)
 *
 */

require_once('ModelFactory.php');


class Model {

        private $request;
        public $clientModel;

        public function __construct(Request $request)
        {
            $this->request = $request;
            $this->buildClientModel();

        }

        public function buildClientModel()
        {
            $className = ucfirst($this->request->object)."Model";
            $this->clientModel = ModelFactory::build($className);
            return $this->clientModel;
        }


	public function create()
	{
	    // create something new
	}

        public function modify()
	{
	    $this->clientModel->modify($this->request->postData);
	}

        public function delete()
        {
            // delete somthing
        }

} 

We now have a factory to create the client model. The model file the factory instantiates is given by the client coder, obviously, and is formed through the same object property we passed in the Request object.

ClientModelFactory.php

<?php

/*
 * A simple factory to build our clientModels
 *
 */


class ClientModelFactory
{
    public static function build($class) {

        require_once ("models/". $class .".php");
        if (class_exists($class)) {
            return new $class();
        }
        else {
            throw new Exception("Invalid object given.");
        }
    }
}

Now, our screen is displaying.

So now you pick a city and then we start over again, but now we are POSTing. And because we are posting, we call the controller.

Controller.php

<?php

/*
 * Our very, very, very thin controller!
 *
 */

class Controller {

	private $model;
        private $request;

	public function __construct(Request $request, Model $model) {
                $this->request = $request;
		$this->model = $model;
                $this->decideAction();
	}


        private function decideAction()
        {
            switch ($this->request->method){
                case "create":
                    $this->model->create();
                    break;
                case "update":
                    $this->model->modify();
                    break;
                case "delete":
                    $this->model->delete();
                    break;
            }

        }

}

Because we are updating and not making anything new, we are only using the modify method in our model. This is what it looks like, when a selection is made.

It works! Can’t wait to hear how this gets ripped up. :wink:

Edit: Oh, and of course, the client only supplies templates and models, which are formulated for the base object being viewed or manipulated (like “clock” in our example). No router! :smile:

Scott

I like it! this is pretty much what I was going for other than you’ve broken apart the router a lot. My biggest issue with this approach is that although I’m all for convention over configuration when you only have convention it’s rather limiting. How would I, for example:

  • Have another page on /clock-ntp that used the same template/controller and used a different model (one that connected to an NTP server to get the time)

  • Have another page /date which used a different template but said “The date in $city is d/m/Y” using the same model

Either I’d have to create date.html.php and DateModel introducing redundancy (and extra work) or I’d need to have this logic twice, once for views and once for models?

public function buildClientModel()
        {
	    if ($this->request->path === '/clock-ntp') return new NtpModel;
            $className = ucfirst($this->request->object)."Model";
            $this->clientModel = ModelFactory::build($className);
            return $this->clientModel;
        }
```php
class View {

	private $template;
	private $model;
        public $headers;


	public function __construct(Request $request)
        {

        	if ($request->path === '/date') $this->template = 'date.html.php';
            else $this->template = $request->object . ".html.php";
	}

Obviously this would be configured using an array or some other data structure… although by doing this all that’s happened is you’ve moved the router logic into two places.

edit: To answer my own question and give some food for thought, I achieve this by having ‘modules’ rather than simple controllers and models so my folder structure now changes from:

clock.html.php
ClockModel.php

to

/clock/default.html.php
/clock/defaultmodel.php

(These can go in subdirectories but let’s keep it simple)

/clock/ then loads default.html.php and defaultmodel.php while /clock/ntp looks for a /clock/ntp.html.php and /clock/ntpmodel.php, if it can’t find the files it uses default. That way /clock/date uses /clock/date.html.php and /clock/defaultmodel.php while /clock/ntp uses /clock/ntpmodel.php and /clock/default.html.php . Of course this falls down and you still need proper routing rules if you want a better URL or you want to use ntpmodel.php with date.html.php How do I handle this? Each module e.g. /clock/ has its own routes.json and provides this information when it’s necessary :slight_smile: