Implementing ToMVC

I’ve been planning to try out TomB’s MVC implementation for a long time and inspired by the recent “MVC Refactor” thread I’ve decided to give it a go. I’m starting a new thread because I don’t want to engage in the ongoing discussion there at the moment - I think it’s important to be using the pattern in the real world and have some experience to be able to assess it so I’ve decided to try it out in a small project.

For now I want to focus on implementing MVC according to this article (MVC in PHP: Deploying MVC on the web with a Front Controller). At this stage I do not want to get into adding dependency injection containers (yet).

So far I have created the basic framework based on https://r.je/mvc2.phps but the code is not complete enough. I have a front controller, which is very similar:

class FrontController {
    private $controller;
    private $view;
    
    public function __construct(Router $router, $routeName, $action = null) {
    
        //Fetch a route based on a name, e.g. "search" or "list" or "edit"
        $route = $router->getRoute($routeName);
        
        $this->view = $route->getView();
        $this->controller = $route->getController();
        
        if ($this->controller) {
           //Run the controller action
            if ($action) {
                $this->controller->{$action}();
            }
        }
        
     }
    
    public function output() {
        //Finally a method for outputting the data from the view 
        //This allows for some consistent layout generation code such as a page header/footer
        $header = '<h1>Hello world example</h1>';
        return $header . '<div>' . $this->view->render() . '</div>';
    }
}

My problem is with the controller and action part:

  1. Why are the route name and action split in two parts? The weird thing for me is that the router decides which view and controller to use but not which action. In this example the action comes independently from a different argument - who then decides which action to run and why doesn’t the router decide on that? If the decision about the action comes from an outside entity (like a URL) it may happen that the code will try to run a non-existent action since the front controller doesn’t check for its existence. A more logical approach for me would be to make the router decide which action to run - either from a single URL part (like userEdit, userList, etc.) or from a two-part URL (like User/Edit, User/List, etc.), in which case the router would probably need to validate if an action exists.
  2. The essential missing part in this example is the arguments that need to be passed to each action. The front controller simply runs the action without any so the issue seems to be ignored here - certainly each action will need a different set of arguments (like request, session, id, etc.) and here the front controller does not know what to pass. Where do I decide about that?
  3. The $action parameter is optional in the code - but if there is no action then does it make any sense to instantiate the controller at all? The view doesn’t need the controller so to me it is redundant.

I would guess we’d need to see the calling code or ask Tom directly what the logic behind it is. But, yes. I don’t get the separation of action from the router object either. Usually the router object would hold the action too, as it must come from the HTTP request. The router would need to inspect the URL/ HTTP payload to figure out the requested action.

I’d say this was a very simplistic example just for the discussion Tom makes. He even says at the end.

This is a very simplistic approach to a front controller. In a real-world application it would be far more complex and robust.

The answer goes along with the answer to your second question. Since it is a basic example, a lot of stuff is missing.

I would say the only thing questionable is the injection of the action and route name in the front controller constructor. This would mean the actual front controller script would need something like.

<?php
$request = new Request();
$route = new Route($request);
$routeName = $route->getRouteName();
$action = $route->getAction();
$frontController = new FrontController($route, $routeName, $action);
$frontController->output();

$routeName and $action are superfluous.

The FrontController could look like this.

class FrontController {
    private $controller;
    private $view;
    private $route;
    private $action;
    
    public function __construct(Router $router) {
    
        $this->route = $router->getRoute();
        $this->action = $router->getAction();      
        $this->view = $route->getView();
        $this->controller = $route->getController();
        
        if ($this->controller) {
           //Run the controller action
            if ($this->action) {
                $this->controller->{$this->action}();
            }
        }
        
     }
    
    public function output() {
        //Finally a method for outputting the data from the view 
        //This allows for some consistent layout generation code such as a page header/footer
        $header = '<h1>Hello world example</h1>';
        return $header . '<div>' . $this->view->render() . '</div>';
    }
}

And now the front controller script would look something like,

<?php
$request = new Request();
$route = new Route($request);
$frontController = new FrontController($route);
$frontController->output();

Which, to me, is a bit cleaner.

Scott

It’s in this example at the bottom:

$frontController = new FrontController(new Router, $_GET['route'], isset($_GET['action']) ? $_GET['action'] : null);

It comes from the URL and seems like there are two different segments coming from it. This is not really a problem for me because I understand this can be amended depending on what kind of URL scheme we choose. I was only wondering about the usefulness.

As to the action parameters - even if this is a simplistic example I think it is essential at this stage because even if we don’t pass anything to actions now, we may want to do so later. And if an action is called in the front controller then the front controller needs to know what to pass to each action - how will it know? It would have to check for the $action value and do different action invocations, which would essentially be a sort of another router but only for actions. I don’t know it would make any sense to have the routing logic split in two classes - but Tom mentioned something about action dispatcher.

[sorry for the broken format - does anyone know how to quote formatted code samples in replies here?]

Yes, that’s what I’m heading towards, too. But the problem is we will have to extend the call to action() with parameters. If the parameters are determined by the router then we would need to get them from the router, too. And then what the front controller does is only fetch objects from the router and optionally run the action - since all these things are determined by the router then do we need the front controller at all? It doesn’t seem to be doing anything useful now and calling the action would be better suited and simpler in the router, I think.

Wouldn’t it be problematic to instantiate the Request object before anything else? I imagine the Request would hold GET, POST, SERVER, SESSION, etc. data while they will not be needed for every request. For example, I would initialize session only for some pages and I might also want to have two separate sessions for different pages/requests. I imagine the Request object should be initialized by the router because before we know which M/V/C to call we don’t know what request data is needed?

Did you see my input to the refactoring MVC part 2 thread? My thinking is, a router isn’t even necessary.

Isn’t it true that you must know what the request is all about, in order to know which workflow the application needs to follow? I see the request as a part of the required interface needed for a web MVC architecture. It is what makes the “web” in web MVC. The other part of the interface is the response.

My contemplation is, do we really need to have a router for the the program workflow? I say, nope.

With web MVC, there are only 4 clear activities the application must do, when completing the workflow through an MVC triad. Those activities are Read, Create, Update or Delete. Reads are taken care of by the View. Creates, Updates or Deletes should be part of any model manipulation, and thus, will be taken care of by a/the controller.

Other than that, what does a web MVC application need to do at this highest level? If you come up with anything else, I bet it will fall into these 4 functions somehow. Read, Create, Update or Delete. Why are we regurgitating a ton of the same kind of code in “page” controllers? IMHO, it is unnecessary. A page “format” is actually a view activity too. And in true MVC, the controllers and views never cross each other. Why is this now necessary in most (all?) web MVC frameworks? I am trying to get away from that completely to follow MVC more carefully. I just need to build out what I have, to see how well my new web MVC can cover all web application needs.

Scott

You’re right, of course :slight_smile: For the purposes of a simple demonstration script I didn’t really want to get into the topic of breaking apart the URL so wanted to rely entirely on $_GET parameters. as you suggest, in a real-world project it would be better to generate $aciton and $controller by splitting the request uri and providing some sensible defaults if it doesn’t match up.

There’s actually some better examples of this in the other thread.

This is basically the same as 1) it’s a simple example to demonstrate the concept rather than a complete solution.

This is certainly a possibility and one I’m on the fence on. On the one hand there are 3 different processes:

  1. Decide which controller/view/model/action to use
  2. Call the action
  3. Do something with the response

While 1 and 2 can probably be merged (if one changes, the other does so it follows that they’re a single responsibility), the third one may not.

In the real world I have the Front Controller construct two MVC triads:

  1. Based on the action requested by the URL as the examples show
  2. A MVC triad for the layout. The model has a variable $content, which is set by the front controller and contains the HTML output from the other triad. The template for this triad contains all the HTML for the layout, <head> tags, <title> tags, etc.

As an aside, each view has a $css (array), $js (array) and $title (string) property which is readable by its parent view. This way the layout can be generated and include any css/js files that are required.

Firstly, it’s worth considering the difference between the variables. $_POST, $_GET and $_SERVER are variables that are populated externally whereas $_SESSION is controlled entirely by the PHP script, think of it as encapsulation: The data in $_SESSION can be considered safe, whereas the others cannot. As such, they need to be treated differently. A request object should contain only those things that come from the request $_POST, $_GET and $_SERVER and $_COOKIE and be considered read-only. Setting a cookie is part of the application logic (Store some information) and belongs in a model (probably abstracted to its own class!)


<?php
$request = new Request();
$route = new Route($request);
$frontController = new FrontController($route);
$frontController->output();

Personally I prefer to do routing based on $_SERVER only as it keeps things simpler and then keep $_GET and $_POST for data. That way the router has a dependency only on $_SERVER:

$router = new Router();
$route = $router->getRoute($_SERVER);
echo $route->getView()->render();

By constructing the MVC triad using a DIC, the controller/model can ask for the Request object (containing $_GET and $_POST) in its constructor if it requires it and the router is unaware.

The router then constructs the controller like this:

//Would really need some better routing than this, but for simplicity:
list($model, $controller, $action, $arg1, $arg2) = explode('/', $server['REQUEST_URI']);
$controller = $dic->create($controller);
$controller->{$action}($arg1, $arg2);

Then the router doesn’t care about the Request object, the controller can look like this;


class Controller {
	public function __construct(Request $request) {
		$this->request = $request;
	}
}

or like this:

class Controller {
	public function __construct(SomethingElse $somethingElse) {
		$this->somethingElse = $somethingElse;
	}
}

And the router doesn’t need to know about it. The biggest advantage of this is that the controller’s constructor arguments can change and the router doesn’t need to be reconfigured. Hopefully that’s some food for thought! Doing this without a DIC is a lot more difficult because it means you need to put DIC logic in the router (What dependencies does each class have?)

Yes, I’ve seen it and it looks like an interesting idea. I don’t know how it would play out in the long run and for now I’ll stay with a router just to keep more (manual) control over the routing scheme. Later on I’ll focus on automating the routing mechanism but for now I just want to get the basic MVC working for me. But I like your HTTPConverter class as an entry point :smile:

That’s what I felt as well - 1 and 2 are very closely related. If I understand correctly, if we merge the two and put them into the router then we would still need to keep the Front Controller just for the sake of doing something with the response (the output() method)?

So you are basically using MVC on the layout instead of a template engine? :slight_smile:

Thanks, indeed this is an important distinction, I was mislead by the fact that all of them are superglobals in PHP.

Yes, I realize the benefits of a DIC and will get to it later. For now I want to get a small project running on MVC and now I’m not worried about configuring dependencies in the router because there will not be many of them.

I fully agree - I think we are all here striving to get rid of unnecessary controllers!

…which gets me to another question regarding pages that are read-only but still displaying results based on an ID coming from GET, for example displaying user information page.

Model:

class UserModel {
    private $pdo;
    
    public function __construct(PDO $pdo) {
        $this->pdo = $pdo;
    }
    
    public function load($id) {
        $stmt = $this->pdo->prepare('SELECT * FROM user WHERE id = ?');
        $stmt->execute([$id]);
        return $stmt->fetch();
    }
}

This looks sensible to me. But then how do I get the results of load($id) in the view? The view has access to the model but doesn’t have access to $_GET['id']. I can consider $_GET['id'] as user input, which normally is fed to the controller but not to the view (?). What do I about it? I can see some possibilities:

  1. Pass Request to the view. That would allow me to do without a controller. But is the view supposed to know Request?
  2. Assign a controller action to be called for this page and have the action set a global state of the model like this $model->setUserId($request->id) and then in the view I can call a load() method without any parameters.

or something else?

Not quite, the view still uses a template engine, the front controller instantiates a second MVC triad a bit like this:

class LayoutModel {
    public $content;
}

layout.html.php

<html>
<head>
<?php
foreach ($model->content->js as $javascript) echo '<script src=' . $javascript . '></script>';
?>
</head>
<body>
<?= $model->content->render(); ?>
</body>
</html>

And then the MVC triad for the routed action. The obvious objection here is “But the model knows about the view!”, indeed it does… but for the layout view, what is the data that needs to be processed? The other view. The problem domain for the layout model/view is what needs to be put inside the layout, the domain entity for the layout is the view it’s going to include. This also allows the layout model to supply any other information the layout needs (e.g. is the user logged in or not? What is the total amount of items in their basket?)

Yes, you’ll still need a front controller to decide what to do with the output of the routed view. The router shouldn’t output the result because that reduces reusability. It’s common to want a helper function to include a view inside a view using the same sort of syntax (although perhaps with a different configuration)

This is where my MVC example in the other thread should help: The controller would call load($id) and the view would then have access to the model and call getUser():

class UserModel {
    private $pdo;
    private $user;

    public function __construct(PDO $pdo) {
        $this->pdo = $pdo;
    }
    
    public function load($id) {
        $stmt = $this->pdo->prepare('SELECT * FROM user WHERE id = ?');
        $stmt->execute([$id]);
        $this->user = $stmt->fetch();
    }

    public function getUser() {
         return $this->user;
    }
}

Name: <?= $model->getUser()->name; ?>

This way the model can be substituted and the view can be reused. Imagine if we wanted to get an employee’s manager instead of the specific user:

class UserManagerModel {
    private $pdo;
    private $user;

    public function __construct(PDO $pdo) {
        $this->pdo = $pdo;
    }
    
    public function load($id) {
        $stmt = $this->pdo->prepare('SELECT m.* FROM user u 
                                    Inner Join user m ON u.managerId = m.id
                                    WHERE u.id = ?');
        $stmt->execute([$id]);
        $this->user = $stmt->fetch();
    }

    public function getUser() {
         return $this->user;
    }
}

And reuse the same view/controller.

Indeed, I didn’t notice how it was used in the view, now it makes sense.

Tom, thanks for explaining it in such a details, it makes the concept much clearer and I can get to applying it!

Are you asking me? Or Tom? Or Both? :smile:

If you are asking me, the id would be “decoded” from the request and is part of the object parameters within the model. So, if the request holds an object name and an id, that would tell you to form a model for that specific id. ObjectXFromIDModel.php or a class better named than that. And actually, looking back at my code, I already see a need for a change in the request. But, since it is a get, the response comes directly from the View.

Scott

Anyone who has something to say on that matter so you can count yourself in :smile:

So it’s slightly different from TomB’s way of having the model constructed first and then fetching the requested data object (User) in the controller.

BTW, Tom this class:

class UserModel {
    private $pdo;
    private $user;

    public function __construct(PDO $pdo) {
        $this->pdo = $pdo;
    }
    
    public function load($id) {
        $stmt = $this->pdo->prepare('SELECT * FROM user WHERE id = ?');
        $stmt->execute([$id]);
        $this->user = $stmt->fetch();
    }

    public function getUser() {
         return $this->user;
    }
}

is not complete once constructed because getUser() actually gets the user only after load($id) has been called. Isn’t this problematic in some way? I think there was a principle that objects once instantiated should be complete but I can’t remember what it was called…

Indeed, this is actually an encapsulation problem see ( https://r.je/constructor-injection-vs-setter-injection.html ). However, in this case returning a null reference when getUser() is called is not a problem because there needs to be some logic in the view to handle that if the a record can’t be loaded using the supplied ID. Template file:

<?php
$user = $model->getUser();
if ($user) {
?>
Name: <?= $user->name; ?>
<?php
}
else {
?>
404 not found :(
<?php
}
?>



Ok, I see, thanks!

Tom, would you consider error pages to be a functionality of the core view class? I mean, if the model couldn’t successfully be built, because the request just didn’t hold the right information, then that would be a 404. Or, depending on what went wrong, other errors could be displayed, like a 500 on exceptions, with error for development and without for production. This avoids code duplication in the templates, wouldn’t it?

It is also shown in the OP above that the output() method should be in the front controller and it shows a piece of view logic, even with HTML in it. I find this a bad practice, because I feel all and any HTML design elements should be in templates. It helps the designer troubleshoot the view easier.

The hierarchy of templates (like including a header and footer template) should be a core functionality of the templating engine. The front controller should just call the view to render output and nothing else, IMHO.

Scott

I would have generic templates which could be included in other templates such as a various error pages, then you would have if ($user) {display user} else {include error.html.php}. Of course, generic errors aren’t always what you want.

For example, one place I’ve found good use for custom errors is on deleted products. A customer clicks on a link from google that takes them to a product which no longer exists. Rather than just displaying a 404, it’s better to display “Sorry, that product is no longer available, here’s some alternative similar products” and supply a 404 header so google doesn’t index the error page.

I disagree with this. The reason is that it’s useful to be able to include pages in other pages for example. I might have /news/latest which displays the latest 10 news stories in a list. It’s not unreasonable that I might want to pull this into a sidebar on another page and style it a different way. Same for contact forms and other specific reusable page elements.

Whether the layout is applied by the router or the front controller doesn’t really matter, but there needs to be somewhere that puts the routed mvc triad into a reusable layout. If it’s part of the templating engine, you then have trouble loading a template into a different layout (e.g. admins get a different layout to normal users)

Hmm…I see what you mean. I still disagree though. The inclusion of templated components like a list of top 10 news items is a design choice and belongs in the view. If you use a template engine like Twig, you can include page components (sub-templates) and they can in turn call the models they need. Actually, with Twig, an included template calls a controller…which I also totally disagree with now.

{# app/Resources/views/base.html.twig #}

    {# ... #}
    <div id="sidebar">
        {{ render(controller(
            'AppBundle:Article:recentArticles',
            { 'max': 10 }
        )) }}
    </div> 

I’d rather see.

{# app/Resources/views/base.html.twig #}

{# ... #}
<div id="sidebar">
    {{ get(Model(
        'AppBundle:Article:recentArticles',
        { 'max': 10 }
    )) }}
</div>

or something to that effect.

It just seems to make more sense. This new perspective of mine is your fault too Tom. LOL! :smile:

Scott

So how do you handle a situation where you need different layouts at different times? On one of the B2B sites I work on, a business user logs in and they get a site that is fully branded to their business. Essentially it’s the same site as everyone else gets but with different styles, a different layout, some different navigation options, etc. How would you handle that unless you can load a given piece of content into a particular layout template? Obviously a huge template with if/elses for each possible option isn’t the way forward!

Template engines can include other files so it’s easy to create a few templates, each using a different layout and all of them including the same content template. Not saying that your solution is bad, just that it’s not difficult to do with templates - unless there is something more I’m not aware of.

It depends on how different the layout would need to be. The scenario you mentioned sounds like a multi-tenant SaaS type of system. To me, the templates would then need to be totally separate for each tenant, so the template (and layout/ design) selection would be handled at system level. If tenant X calls the site, templates from directory X would be called. If tenant Y would call the site, then templates from directory Y would be called. This would allow for the highest possible customizability.

Scott

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.