code separation / mvc / need real-life advice

[Ezku]

I think variables are allright, but it's borderline. I'd definately say that the view should only get primitives, such as scalars, arrays and valueobjects - not complex model components.

So iterating over a result set in the view is a no-no?

[kyberfabrikken]

So iterating over a result set in the view is a no-no?

I guess it's a judgement call - if the objects contained by the resultset are primitives, I see no problem in that. Especially if the resultset implements the iterator interface.

[33degrees]

So iterating over a result set in the view is a no-no?

I don't see any harm in passing complex model objects to the view; the important thing is that the view does not in any way change the state of the object. In fact, I'd say there are benefits; certain types of logic that might get placed in the view can be taken care of by the models. For example, say if you're display a list of users, and you want to flag the ones that are underage; instead of doing the calculation (age < 18?) in the view, you can have an isUnderage() method in your model, and call that instead. That way you can limit your presentation logic to simple if equal statements, greatly reducing the posibility of business logic creeping into the view.

[Ezku]

I don't see any harm in passing complex model objects to the view––That way you can limit your presentation logic to simple if equal statements, greatly reducing the posibility of business logic creeping into the view.

I agree. The other solution would be to have a separate phase in the view to parse all the data into primitives before passing it to the template – which could be a good solution at times, but could also get tedious and be of no actual benefit at others.

[champ]

Again - you can do validation in controller or in model or in both. It's a matter of taste and the particular context. As a rule of thumb, I generally put validation in the controller, but might move it into the model.

I've done basic format checking validating in the controller, such as checking if the username or password is in the correct format, or if a number field contains only numbers. Once this superficial validation passes, I'll pass the necessary data to the model, which will perform further validation.

My question is, should the Model internally log an array of errors, and then make them available to the Controller upon request...

class Model_Account
{
    var $errors = array();

    function get_errors()
    {
        return $this->errors;
    }

    function is_unique_email($email)
    {
        // verify unique email by quering db
        // return TRUE or FALSE
    }

    function is_unique_username($password)
    {
        // verify unique password by quering db
        // return TRUE or FALSE
    }

    function register($email, $username, $password)
    {
        if (!$this->is_unique_email($email)) {
            $this->errors[] = 'email is not unique';
        }
        if (!$this->is_unique_username($username)) {
            $this->errors[] = 'username is not unique';
        }
        if (empty($this->errors)) {
            // insert user into database
            // return TRUE to controller
            return true;
        } else {
            // return FALSE to controller
            return false
        }
    }
}

class Controller_Account
{
    function process_registration()
    {
        if (!$model->register($email, $username, $password)) {
            $errors = $model->get_errors();
        }
    }
} 

Or should the Controller directly call the $model->is_unique_email() and $model->is_unique_username() model methods and only call $model->register() when no errors exist.

class Controller_Account
{
    function process_registration()
    {
        if ($model->is_unique_email($email)) {
            $errors[] = 'email is not unique';
        }
        if ($model->is_unique_username($username)) {
            $errors[] = 'username is not unique';
        }
        if (empty($errors)) {
            if (!$model->register($email, $username, $password)) {
                $errors = $model->get_errors();
            }
        }
    }
} 

It feels wrong for the Model to directly log errors internally. It also feels wrong for the Controller to call Model specific validation methods.

[kyberfabrikken]

I don't see any harm in passing complex model objects to the view ... That way you can limit your presentation logic to simple if equal statements, greatly reducing the posibility of business logic creeping into the view.

Yay. Looking back at my post now, I'm not sure I'll take side with my own statement. (That's a deceptive way to say, I was wrong btw.)
After having some mixed experience with OO model layers, I prefer using a rather procedural model layer, at the moment. This naturally leeds to simple model components, so having something complex ending up in the view layer sets off a warning with me.
The scenario I was thinking of is where a gateway somehow ends up in the view. Say you have an object oriented query api - you wouldn't want to expose that to your view. But this, I suppose, is more an issue of making the api between view and model as concise as possible.

[kyberfabrikken]

It feels wrong for the Model to directly log errors internally. It also feels wrong for the Controller to call Model specific validation methods.

You could implement an event system, which allows the model to notify about failures, and let the controller listen to this. A less laborious variant is a double dispatch (visitor) - pass a logger object (which might just be the controller itself), to the model.
I don't think it's inherently wrong to simply let the controller call validation methods on the model though. It depends a bit on the complexity of the model.

[Dr Livingston]

> the important thing is that the view does not in any way change the state of the
> object.

And...

> say if you're display a list of users, and you want to flag the ones that are
> underage; instead of doing the calculation (age < 18?) in the view, you can have an
> isUnderage() method in your model, and call that instead.

Isn't that where the Observer comes into it though? You have that *::update(); clause, which in my view removes the need for those conditionals in your template?

Not that I'm looking to turn this thread into another should we or shouldn't we arguement

[33degrees]

Isn't that where the Observer comes into it though? You have that *::update(); clause, which in my view removes the need for those conditionals in your template?

I'm not sure where an Observer comes into this? Care to explain?

[dan7]

Hi again..
I see this thread has evolved a bit with some very interesting code in there.

I need to get back a bit for a moment. It will be long, I hope somebody will be patient enough...

login.php

require 'class/httpcontext.php';
require 'class/session.php';
require 'class/db.php';
require 'class/loginbox.php';

$db = new Database();
$httpcontext = new HttpContext(new Session($db));
$controller = new LoginBox($db, new Translator($db), $httpcontext); 
$httpcontext->content = $controller->execute();
$httpcontext->out(); 

This is the example kyberfabrikken gave before, considering a login page.
Actually, this is never happening in my application.

Here's some relevant code from main controller

page.php

class Page {

    [...]

    function execute() {
        if ($this->isXMLHTTP()) {
            return $this->ajax();
        }
        return $this->get();
    }

    function get() {
        return $this->render("main.tpl.php");
    }
    
    function ajax() {
        $this->httpcontext->contentType = "text/json";
        return $this->render("main.json.tpl.php");
    } 

So basically, if the request is not xmlhttp then get() method is executed and html returned, otherwise, ajax() is executed.

My problem is implementation of ajax() method.
Here are 2 scenarios with xmlhttp I'm facing.
1. User is executing a given component and the expected response should contain some info from THIS PARTICULAR component only (I don't need to update any other parts of the GUI)
2. User is executing a given component and the expected response should contain some info from THIS component + info from some other components (I need to update some other GUI parts)

Now, I'm not sure how to handle both scenarios. I believe, that my ajax() method should execute all the relevant components in some particular order.

So, here are some samples
Page.php - main controller

class Page
{
    (...)

    function __construct($db, $translator, $httpcontext) {
        $this->db = $db;
        $this->translator = $translator;
        $this->httpcontext = $httpcontext;
    }

    (...)
    
    protected function getComponentOne() {
            $childcontroller = new ComponentOne($this->translator);
            return  $childcontroller->execute();
    }
    protected function getComponentTwo() {
            $childcontroller = new ComponentTwo($this->translator);
            return  $childcontroller->execute();
    }
    
    function execute() {
        if ($this->isXMLHTTP()) {
            return $this->ajax();
        }
        return $this->get();
    }

    function get() {
        return $this->render("main.tpl.php");
    }
    
    function ajax() {
        // I need to execute the relevant components 
        // some of them already ouput data itself and exit (component one),
        // but some return data (component two)
        $return = array_merge ( 
              $this->getComponentOne(),
              $this->getComponentTwo()
        );

        $encoder = new Services_JSON();
        $this->httpcontext->contentType = "text/json";
        return $encoder->encode($result);
    }

} 

componentOne.php - coontroller of component one

class componentOne
{
    function __construct($translator, $httpcontext) {
        $this->translator = $translator;
        $this->httpcontext = $httpcontext;
    }

    function execute() {
        if ($this->isXMLHTTP()) {
            return $this->ajax();
        }
        $httpMethod = $_SERVER['REQUEST_METHOD'];
        return $this->{$httpMethod}();
    }

    function ajax() {
        $login = new LoginModel($this->db);
        $result = $login->tryLogin(
            $_GET['username'],
            $_GET['password']
        );

        $encoder = new Services_JSON();
        $this->httpcontext->contentType = "text/json";
        $this->httpcontext->out($encoder->encode($result));
    }

    function get() {
        return $this->render("loginbox.tpl.php");
    }

} 

componentTwo.php - controller of component two

class componentTwo
{
    function __construct($translator, $httpcontext) {
        $this->translator = $translator;
        $this->httpcontext = $httpcontext;
    }

    function execute() {
        if ($this->isXMLHTTP()) {
            return $this->ajax();
        }
        $httpMethod = $_SERVER['REQUEST_METHOD'];
        return $this->{$httpMethod}();
    }

    function ajax() {
        $result = array('foobar');

        return $result;

    }

    function get() {
        return $this->render("something.tpl.php");
    }

} 

Is this the way to go to handle the described problems? If something is unclear, please let me know and I'll try to explain my problem better.

Thanks a lot

[kyberfabrikken]

Calling $this->httpcontext->out() from within your controller is definately a no-no. The purpose of the httpcontext is exactly to insulate the controller from the global context. If you call out() from within the controller, you might aswell have used echo, header and exit.
That said, I understand why you end up doing it.

The problem is that if you use a hierarchical controller design, there is no easy way for a deeply nested controller to "break out" of the outer controllers. There is some degree of coupling between controller and view. Normally, this isn't a problem, because that's exactly the behaviour you want, but when you mix different types of requests (html and ajax), the game changes a bit.
One way to solve it, is to make separate hierarchies for the two types of request - effectively making them two separate applications. For some, this may be the answer. For example if you're building a web-api of sorts, I think this model would do better than trying to weave it in with the html site.
With ajax-requests however, this is often not the best answer. The ajax request is tightly bound with the html request, so it is desirable to deal with the ajax-request alongside the html, as we have also been doing so far.
To do this, you need a way to let a controller deeper down the hierarchy break of the rendering chain. There are several ways to do this.
One way is to separate the request-processing from the rendering of view into two methods. You would then execute the hierarchy two times, and in the first pass, any one controller can signal that it needs to be the root of the second pass.
Another, less dramatic solution could be to introduce a lock on the httpcontext, which can be set at any time. The downside of this approach is that it then becomes the responsibility of each controller to check if the lock has been set, before continuing execution. This is especially bothersome when you're rendering childcontrollers, since you'd have to explicitly check the httpcontext after each of them have finished their execution.
The solution I'm using in konstrukt is a bit different. I basically exploit exceptions to break the normal execution-stack. If a deep controller wants to prevent outer rendering, it'll create an instance of k_http_Response, assign contents to it and throw it (k_http_Response extends Exception). The top level dispatcher then catches it and calls it's out() method. Purists would call it exception-abuse, and they'd be right too, None the less, it works very well, and I haven't really encountered any backsides to it yet. The important issue for me, is that the scope of each controller is contained, and since exceptions can be caught, that holds true.

[dan7]

Calling $this->httpcontext->out() from within your controller is definately a no-no.

Yeah I know... Just wanted to show what I wanted to achieve...

For example if you're building a web-api of sorts, I think this model would do better than trying to weave it in with the html site.

Yes, but it's not a web-api. This is just a top layer, that makes some things easier, but below it, is a pure html solution. So, I want to achieve as small separation as possible between both types of requests.

Another, less dramatic solution could be to introduce a lock on the httpcontext, which can be set at any time. The downside of this approach is that it then becomes the responsibility of each controller to check if the lock has been set, before continuing execution. This is especially bothersome when you're rendering childcontrollers, since you'd have to explicitly check the httpcontext after each of them have finished their execution.

That's almost exactly what I was thinking about after I posted my message. But I thought, that this lock-checking could be done in the main controller. So, I execute childcontrollers only, if there's no lock on httpcontext. What do you think about such solution?
As all the controllers extend the same abstract class, I could also build some lock-checking method there to run automagically, but I'm not sure if it's a good solution.
I have to think about it.

Thanks for the tips! Helped a lot

[kyberfabrikken]

But I thought, that this lock-checking could be done in the main controller. So, I execute childcontrollers only, if there's no lock on httpcontext. What do you think about such solution?
As all the controllers extend the same abstract class, I could also build some lock-checking method there to run automagically, but I'm not sure if it's a good solution.

Yeah - The problem is with the Page->getLoginBox method of the running example.
What if LoginBox wanted to be king ? In order to allow this, you will have to call the child controllers before calling render(). But then the controller has to know which components the view is going to need.
If you accept the possible overhead, you could let main.tpl.php render as normal, and then check if the context has been sealed meanwhile, before assigning the contents. You will have to suck up some rendering that is never going to be used with this technique, but in most cases the overhead will probably be bearable.

[dan7]

Yeah - The problem is with the Page->getLoginBox method of the running example.
What if LoginBox wanted to be king ? In order to allow this, you will have to call the child controllers before calling render(). But then the controller has to know which components the view is going to need.

Yeah, that's the problem. I need to review my code and see if I this "I want to be a king" issue is the case here.

Thanks!

[lvismer]

Hi Daniel,

I am also just another traveller trying to figure out the matrix Having tried to follow the thread and keeping true to what I at least could pick up as initial needs. How about the following sort of approach...

index.php

$locator = new Locator();
$locator->set('request', new Request());
$locator->set('response', new Response());

$auth = new Authenticator();
$auth->execute();

$locator->set('auth', $auth);

$page = new MainPage();
$page->execute($locator);
$response = $locator->get('response');
$response->out(); 

page.php

class MainPage possibly_extends Something
{
    protected $locator;
    public function execute(Locator $locator)
    {
        $this->locator = $locator;
        ...do some other setup stuff
    }

    public function getContent()
    {
        $controllerName = $this->locator->request->get('controller');
        if ( $this->controllerExists($controller) ) {
            $controller = new $controllerName;
            return $controller->execute($this->locator);
        }
    }

    public function getMenu()
    {
        if ( $this->locator->auth->hasAccess() ) {
            ... display propper menu and welcome message etc.
        } else {
            $this->locator->request->set('controller', 'login');
        }
    }

    public function getFooter()
    {
        ...
    }
} 

main.tpl.php

<html>
<body>
<div id="menu-horizontal"><?php echo $this->getMenu(); ?></div>
<div id="content"><?php echo $this->getContent(); ?></div>
<div id="footer"><?php echo $this->getFooter(); ?></div>
...some other modules/plugins/controllers here...
</body>
</html>

then something like index.php?controller=index would render the index page and if the user has been authenticated he gets the page with menu's and all the frills, else just the login for barring frills.

Obviously the meat of Authenticator and the Index controller are left out.

--
lv

[Dr Livingston]

> The downside of this approach is that it then becomes the responsibility of each
> controller to check if the lock has been set, before continuing execution.

Not had the opportunity to follow this thread in it's completeness, however one idea that has just cropped up to get around the stated problem, would be to have independent contexts, no?

Share nothing, in other words

[dan7]

Hi Daniel,

I am also just another traveller trying to figure out the matrix Having tried to follow the thread and keeping true to what I at least could pick up as initial needs. How about the following sort of approach...

lvismer, thanks a lot. I don't know what to say about it now. I think I have to get some idea what is the responsibiilty of Locator first

[dan7]

Not had the opportunity to follow this thread in it's completeness, however one idea that has just cropped up to get around the stated problem, would be to have independent contexts, no?

Share nothing, in other words

Hi Dr
There are separate contexts, but I'm not sure what you mean about 'share nothing'...

[lvismer]

I think I have to get some idea what is the responsibiilty of Locator first

It pretty much is the same as the Registry/Service Locator idea that Kyber spoke about.

You could actually have the Service Locator create (lazy load) the request, response etc. objects as and when you need them.

--
lv

[kyberfabrikken]

Not had the opportunity to follow this thread in it's completeness, however one idea that has just cropped up to get around the stated problem, would be to have independent contexts, no?

Share nothing, in other words

I think that about sums up the purpose of a context in the first place. The question is how.

Off Topic:

This is about when, I was expecting you to jump in and suggest a visitor to do the job

[dan7]

hi again

I have a little problem with a View and different request types

Let's say I have a component, which outputs a list of links.

The simple controller would look like this (based on previous code)

class Controller {

    [...]

    function execute() {
        if ($this->isAjax()) {
            return $this->ajax();
        }
        return $this->get();
    }

    function get() {
        return $this->render("main.tpl.php");
    }
    
    function ajax() {
        return $this->render("main.json.tpl.php");
    } 

Ok, so if request is made through xmlhttp, ajax() is called. Otherwise get() is called

Now, let's say main.tpl.php looks like that

<ul>
    <li><a href="/1" title="1">1</a></li>
    <li><a href="/2" title="2">2</a></li>
    <li><a href="/3" title="3">3</a></li>
</ul> 

ajax() should return a php array with the content, so in this case, something like this:

array (
    array (
        'href'=>'/1',
        'title'=>'1',
        'value'=>'1',
    ),
    [...] 

The question is, how to handle both cases here? The main problem for me is to properly construct a JSON response (view). It's easy to make a HTML one, just creating additional procedural template... So far I was trying to avoid creating separate View classes, but I'm a bit lost with JSON view in the such scenario.

P.S. json encoding is being made later, just before the output, because more than 1 component may create the output. So, the main controller merges all output arrays from the components and then the merged array is encoded).

Thanks

[kyberfabrikken]

The question is, how to handle both cases here? The main problem for me is to properly construct a JSON response (view). It's easy to make a HTML one, just creating additional procedural template... So far I was trying to avoid creating separate View classes, but I'm a bit lost with JSON view in the such scenario.
(...)
P.S. json encoding is being made later, just before the output, because more than 1 component may create the output. So, the main controller merges all output arrays from the components and then the merged array is encoded).

You're suggesting to return an array structure for JSON type responses, rather than a primitive (string), as you do for HTML type responses.
This is possible, but it's a more complex view strategy than a simple template view (I believe what Fowler calls a two-step view). The intermediate (assoc array) will need to be transformed into a final response (serialized to a string) at some point. Thus, somewhere in your chain of execution, you must have a component which is capable of doing this. This could be the root controller, or it could be a controller further down the chain.

Note that the confusion stems from the fact that you want to use a two-step view for JSON type requests, while you want a plain template view for HTML type requests. As such, there is nothing preventing you from using a template view for JSON responses. Just as there's nothing preventing you from using a two-step view for HTML responses either. I would suggest that you choose one strategy and use that for all types of requests to simplify things.

[dan7]

You're suggesting to return an array structure for JSON type responses, rather than a primitive (string), as you do for HTML type responses.
This is possible, but it's a more complex view strategy than a simple template view (I believe what Fowler calls a two-step view). The intermediate (assoc array) will need to be transformed into a final response (serialized to a string) at some point. Thus, somewhere in your chain of execution, you must have a component which is capable of doing this. This could be the root controller, or it could be a controller further down the chain.

Yes, exactly. I want to encode the array in the root controller, after all the child controllers has been executed. This way, I can merge all the responses into one array, encode it and return for the output. Never tried to output more objects at a time.

Note that the confusion stems from the fact that you want to use a two-step view for JSON type requests, while you want a plain template view for HTML type requests. As such, there is nothing preventing you from using a template view for JSON responses. Just as there's nothing preventing you from using a two-step view for HTML responses either. I would suggest that you choose one strategy and use that for all types of requests to simplify things.

Yes, I guess that's the point. But is it actually possible, to make a json response without a View class? I have no clue, how I could build that response without additional View class and without breaking the MVC pattern. So, I'll probably stick with the additional View class.

Do you think I should use such class also for components, that don't output json at all, to simplify things? I'm not sure, but maybe rendering in one place would be better... (now I'm rendering through controllers, using $this->render('template.php'))

If I stick with additional view class, I'll lose the access to the controller. Should I pass it to the view then, in the constructor?

Thanks

[kyberfabrikken]

Yes, exactly. I want to encode the array in the root controller, after all the child controllers has been executed. This way, I can merge all the responses into one array, encode it and return for the output.

I'm still not sure why you want to do it that way. I could understand if you were building an XML document, since it needs a header at the top level of the document. But JSON can easily be built as string fragments, which can be stitched together - just like HTML. You can use the same techniques as with HTML, or you can use a helper in the view for serializing native PHP data to JSON.

But is it actually possible, to make a json response without a View class? I have no clue, how I could build that response without additional View class and without breaking the MVC pattern.

You can still use a procedural view for JSON responses, even if you return native PHP data (assoc array), rather than a string.

Do you think I should use such class also for components, that don't output json at all, to simplify things?
I'm not sure, but maybe rendering in one place would be better...

That might be a good idea, but I think it depends on the application. If it's mainly an HTML application, and the JSON is merely a bit of sugar on the top, I think it would be a mistake to let the JSON dictate the application structure. You can't choose a design which is optimal for all occasions, so you should aim at making it work smooth in most cases, while not getting too much in the way for the remaining cases.

If I stick with additional view class, I'll lose the access to the controller. Should I pass it to the view then, in the constructor?

Yes, you could do that. When you use include ($this->render()) to render the view, you basically have two components - a controller and a view. The controller is an object and the view is a procedural file. What changes, if you use a view class, is that the view component changes from a procedural file to an object. Apart from the style of implementation (object vs. procedural file), there is no difference. And as you know, the procedural file has access to the controller (through $this), so an object view could have the same.

All aside, an object is a more powerful concept than a procedural file. You get inheritance and an easy way to declare functions to use. But in general I think procedural files are more suitable for HTML views. This is probably because the logic needed in views is fairly simple. In the few cases, where I need complex logic, I would then have to put it in separate utilitarian objects (helpers)

[dan7]

I'm still not sure why you want to do it that way. I could understand if you were building an XML document, since it needs a header at the top level of the document. But JSON can easily be built as string fragments, which can be stitched together - just like HTML. You can use the same techniques as with HTML, or you can use a helper in the view for serializing native PHP data to JSON.

I assume you're right. I've just never tried it - so I'll make some test now (I'm using a custom library for handling xmlhttp requests, so I need to check, how it would handle it)

You can still use a procedural view for JSON responses, even if you return native PHP data (assoc array), rather than a string.

Yes, but I'm not sure how to do it properly. Considering, the template examples above, html rendering looks like this:

    protected function render($template) {
        ob_start();
        include($template);
        return ob_get_clean();
    } 

So, considering that my json template consists only of $response = array(...);, then how would rendering look like, something like this:?

    protected function renderJSON($template) {
        include($template);
        return json_encode($response);
    } 

This way I'm sticked to some name convention. Have you got a better idea?
Also, this way, if I want to add something to the view, I'm forced to update both templates. It would be probably easier, if I have a links array predefined and iterated over in the templates, instead of hardcoding. But to do it this way I would have to build a View class, right? But would such links array belong to the view object at all? What if such links are dependent on some permissions? So, user can see links he is allowed to.

That might be a good idea, but I think it depends on the application. If it's mainly an HTML application, and the JSON is merely a bit of sugar on the top, I think it would be a mistake to let the JSON dictate the application structure. You can't choose a design which is optimal for all occasions, so you should aim at making it work smooth in most cases, while not getting too much in the way for the remaining cases.

It's a bit more complicated. The internal part of the site will be mostly Ajax, but the public will be 100% HTML + ajax sugar. Both parts are similar in size. Maybe the internal part is a bit bigger. So maybe I'll drop view class for the internal part then... I don't know for now.

Thanks