2

I've always thought that in typical MVC design it is bad practice to build & concatenate HTML strings in any PHP file or class that isn't strictly a template.

I'm working on a large project with strict guidelines in place, and while it is forbidden to put any HTML code in the PHP controller, it is fair game to have PHP sub-objects that might generate lots of HTML code strings which then get passed into the template.

These "sub-objects" contain additional logic that could change the HTML depending on conditions.

Thus my 2 main questions are:

  1. Is this considered MVC?
  2. Are these objects considered part of the View, or are they a frankenstein of the Model and the View?

Example:

<?php
/* controller.php */
class KittenController {

    protected $view = 'view.html';

    public function render (Request $request) {
        $data = $this->getDynamicData(); // get some data
        $object = $this->getObject();  // get a PHP object
        $this->setField('foo', $data['bar']);
        $this->setField('baz', $object->render());
    }
}

 

<?php
/* object.php */

class Object {
    public function render () {
        $html = '';
        $html .= '<div class="my-cool-html">';
        if ($this->hasDuck()) {
            $html .= '<p>I have a duck...</p>';
        }
        else {
            $html .= '<p>No duck here...</p>';
        }
        $html .= '</div>';

        return $html;
    }
}

 

<!-- an html template -->
<html>
<body>
<div class="blah">
    [[foo]]
</div>
<footer>
    [[baz]]
</footer>
</body>
</html>
Improve this question
4
  • 3
    Burning someone's house down is wrong. Choosing whether to embed HTML in strings is a preference.
    – GrandmasterB
    Commented Apr 8, 2014 at 6:40
  • @GrandmasterB Don't you think putting MVC's Partial Viewes inside the other Classes is just Code Smell?
    – Mahdi
    Commented Apr 8, 2014 at 13:40
  • @GrandmasterB Even burning someones house down might be the right thing to do under certain extraordinary circumstances.
    – Philipp
    Commented Apr 8, 2014 at 13:55
  • Ignoring the model containing a render method, having the controller fetch the model and pass data to the view is not MVC anyway. In MVC the view has direct access to the model.
    – Tom B
    Commented Apr 9, 2014 at 9:03

2 Answers 2

Reset to default
3

The whole idea of MVC is Separation of Concern and by introducing any method that builds/generates any form of mark-up like what you are doing in your example, even without directly outputting them, you're violating that. So this is not MVC anymore.

In your example changing the desing/mark-up by a designer needs modification in your object.php by a programmer. Also you might end up with a mess like mark-up is everywhere in your project classes and your actual Views are just the general layout.

If you are not building anything on top of MVC, then of course you can build different layers, structure you application differently and also have some mark-up generated by classes like what you have in your example; However the later approach is not generally recommended.

What I recommend is to re-factor your Object class to something like the following, where you still have the separation of concern applied; Your PHP code inside the render method will take care of the business logic and the proper Partial View will be loaded and returned in the end.

class PetHelper {

    private $view = null;

    public function render ( Child $child ) {

        if ( true === (bool) $child->hasPuppy() ) {
            $this->view = 'walk-the-puppy.html';
        } else {
            $this->view = 'play-with-kitten.html';
        }

        return $this->view;
    }
}
Improve this answer
2

You need to look at these sub-objects and what logic they contain.

Something has to decide how to mark up the text. As your question makes clear, the controller should not (and in your case, is not) the place where this occurs.

Those sub-objects may be considered part of the view, in that they contain code responsible for assembling the dynamic parts of the view.

If you have data objects that are part of the model, and they emit HTML? That is a violation of the separation of MVC. Some object performs decisions not strictly related to the view (e.g. calculating how many rows in a table) and also emit HTML? Also a violation of the separation.

Sometimes you may find that you are unable to apply MVC or any other pattern 100% due to constraints outside of your control such as language or framework. It is okay to fudge it a little to apply MVC in that case, just document it and try to localize the ugliness to one class or small set of classes. I am not sure if that is what is happening here, however. Just keep in mind that you should try to stick to MVC as much as possible if that is the pattern you have chosen, but it is okay to deviate slightly if there is a good enough reason. It comes down to understanding the principles and how to apply them, then reviewing the code as a team to determine how well it adheres to any particular design pattern. If it does not adhere strictly to MVC, the important question becomes is it worth it to refactor to make it "better" or is the code manageable as-is? But that would be a separate question.

Improve this answer
3
  • -1 In the end there is no strict set of rules that say "this is proper MVC" or "this is not.". Actually there is. In such a radical change it's not MVC anymore.
    – Mahdi
    Commented Apr 8, 2014 at 5:41
  • 1
    I reworded my answer to explain what I meant better.
    – user22815
    Commented Apr 8, 2014 at 13:28
  • Just removed the down-vote.
    – Mahdi
    Commented Apr 8, 2014 at 13:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.