Abstracting views

Hi,

I’ve developed most of my controllers this way for the most part:


class IndexController extends AppController implements ControllerObserver {
	private $index; //model
	private $results;
	
	public function __construct() {
		$this->index = new Index();
	}
	
	public function update(array $info) {
		$this->query = $info['query'];
		$this->controller = $info['controller'];
	}
	
	public function render() {
		if (isset($this->query['a'])) {
			$action = $this->query['a'];
			
			if (method_exists($this, $action)) {
				$this->$action();
			}
		}
		require_once "views/{$this->controller}/index.php";
	}
	
}

This works fine when the page doesn’t change much within it self (same page with different results).

But as I start to work with projects that don’t necessarily create a new controller and instead it generates a different view, this method does not work well.

A basic example would be:
url: example.com/experimental?v=recent

… Would direct the user to the Experimental controller but with a portion of the view changed out (recent news/posts/etc would be shown only)


<div class="wrapper">
  <ul>
    <li><a href="?">All news</a></li>
    <li><a href="?v=recent">Recent News</li>
    <li><a href="?v=old">Old News</li>
  </ul>
  
  <div class="posts">
  <?php if ($this->query['v'] == 'recent') { ?>
    <!-- a different view -->
  <?php } elseif ($this->query['v'] == 'old') { ?>
    <!-- //another view -->
  <?php } else { ?>
    <!-- show all posts (recent, old, etc) -->
  <?php } ?>
  </div>
</div>

Do you see the problem? As I start to add additional filters, this page gets bloated with if/elseif statements.

What are some of the approaches that are considered best practice in this situation? Any code example would be great and or links would be great.

Thanks.

Hey guys can we let go of the bracer thing?

@TomB

I’ve implemented the model as a HAS-A relation.

I can easily do the following:


class IndexController extends AppController implements ControllerObserver {
    private $index; //model
    private $results;
    
    public function __construct() {
        $this->index = new Index();
    }
    
    public function update(array $info) {
        $this->query = $info['query'];
        $this->controller = $info['controller'];
        
        //this would return an array
        //i can easily traverse through the results in the view now
        $this->results = $this->index->getNewsPosts($this->query['v']);
    }
    
    public function render() {
        if (isset($this->query['a'])) {
            $action = $this->query['a'];
            
            if (method_exists($this, $action)) {
                $this->$action();
            }
        }
        require_once "views/{$this->controller}/index.php";
    }
} 

@Michael Morris

Still waiting on you :slight_smile:

Sure I can especially if its: <?php } ?> and, as TomB pointed out, moving your cursor to } causes the matching { to high lite.

And I’m pretty sure I wasn’t suggesting that you use a full IDE to proof a post (though if you do look at the color coding in your post another rather significant error does jump out). I was merely suggesting that using a color coded editor would help during normal php development. Perfectly fine with me if you chose not to use one.

I am NOT going to proof a quick vbulletin post in a full IDE. :rolleyes: And even with color syntaxing braces get lost. You cannot argue to me that

<?}?>

is easier to spot and grok than

<? endforeach ?>

You don’t have a model in the MVC sense, which is why you’re struggling to implement MVC :stuck_out_tongue:

Possibly not, but you lose bracket matching functionality in the IDE which makes it far more difficult to find the matching start tag.

It will be a little while. It’s about 300 lines of code along with another 50 lines relavant to it in Core (decoupling is nice and all, until you have to write the same 5 lines of boilerplate code 300 times).

Alright people, can we let go of this brace things please?
proxi asked a valid question and I’d like it if we all discuss that, and not start a heavy off-topic debate on the topic of curly brackets vs bracketless code.
It’s an interesting topic, but if you want to discuss that I invite you to start a new thread on the subject. Heck, you could even throw in a poll :wink:

Better from a code clarity point of view but your controller is still doing too much imho, none of that controller logic is reusable.

First you have to find the } . It’s just hard as hell to find a brace amongst a forest of html tags. It’s not as bad in straight PHP because there’s less of a visual distraction.

YMMV of course. The other reason I use the things is to give the template files a feel distinct from the rest of the code.

Proxi: If there’s need to decide, you must use switch or ifs anyway.

Render method belongs to view, not controller (IMHO).

Ok folks, if you’re going to have php interspersed with html learn how to use braceless syntax please. Braces in HTML make my eyes bleed.

Example:


<div class="wrapper"> 
  <ul> 
    <li><a href="?">All news</a></li> 
    <li><a href="?v=recent">Recent News</li> 
    <li><a href="?v=old">Old News</li> 
  </ul> 
   
  <div class="posts"> 
  <? 
    $results = array(); 
    if ($this->query['v'] == 'recent'):
      $results = $news->findRecent(); 
    else if ($this->query['v'] == 'old'): 
      $results = $news->findOld(); 
    else: 
      $results = $news->findAll();
    endif; 
    foreach ($results as $newsitem): ?> 
    <p>$newsitem->title</p> 
    <? endif ?> 
  </div> 
</div>

And don’t give me the “short tags is deprecated” line. If you’re running mod_rewrite you can put “php_flag short_open_tag on” in the .htaccess file. It’s not hard. Even if you don’t want to use short tags, stop using brace syntax with HTML. It’s ugly. VERY ugly.

Second point. View != template. Templates in their entirety are part of the view, but there are aspects of view beyond the templates. Get the figuring out of which data block to run out of the template file. You don’t have to build a template engine like smarty - that’s overkill (not to mention smarty is the dumbest idea since the pet rock), but a class to manage the response to the client. Let it do the data prep and then load and eval the templates using something like this


protected function parseTemplate($template, $output = null) {
		if (is_array($output)) {
			extract($output);
		}
		ob_start();
		include($this->findTemplate($template));	
		return ob_get_clean();
	}

The findTemplate method is another method of the responder to resolve the template path (primarily since the framework has default templates and projects may override those templates by having a copy with the same name and path in the project template tree). The root Responder class knows how to compress data and send http 1.0 headers appropriate to the response. The HTML responder handles templates, as does the XML responder. The Javascript responder deals with replies to AJAX calls and so on.

The controller determines the appropriate response based on request type, but leaves the particulars to the responder chosen.

Basically what you’re saying is to have one big switch statement in the render method? That defeats the purpose of what I’m trying to fix.

I’m trying to decouple the view and controller as much as I can.

Consider having four template files: PostsRecent,PostsOld,PostsAll and Wrapper.

Your render function would switch on query[‘v’], render the appropriate PostTemplate storing the results in a variable then render the Wrapper template passing along the variable.

Using bracers or not is a personal preference and is not relevant to what I asked.

Could you please explain your parseTemplate method a little different? I’m not sure I’m understanding you 100%. Also, what does the findTemplate method do?

I’m not entirely sure how javascript, xml and ajax got involved.

@TomB

I do have a model. If you look closely at my controller class you will see I labeled index property as a model.

Models only handle data, persistence, etc. So passing the query to the model is out of the question really :-/

Reusable in what sense? What can’t I reuse?

What do you mean by too much? The controller is not dealing with any data just calling the resources it needs.

Yeah, I goofed in the example. Still, braces get lost SO easily in html blocks, they aren’t worth it.

What I mean is, your view is being fed its data by the controller rather than getting it from the model for itself. This causes lack of reusability.

If you want to reuse the view with a different data set you need to rewrite your process() method. It’s not immediately obvious when you have just one variable. But imagine your process method was setting $this->categorytitle, $this->datefrom, $this->dateto, etc all you’d be doing is fetching these from the model and passing them to the view.
This binding logic is made redundant querying the model from the view.

By moving the $query variable and process() method to the model you wouldn’t need to repeat this logic when you reused the view.

hmm… After thinking about it for a while. I’ve been thinking about this:


class IndexController extends AppController implements ControllerObserver {
    private $index; //model
    private $results;
    
    public function __construct() {
        $this->index = new Index();
    }
    
    public function update(array $info) {
        $this->query = $info['query'];
        $this->controller = $info['controller'];
        
        $this->process();
    }
    
    public function render() {
        if (isset($this->query['a'])) {
            $action = $this->query['a'];
            
            if (method_exists($this, $action)) {
                $this->$action();
            }
        }
        require_once "views/{$this->controller}/index.php";
    }

    private function process() {
       $view = (isset($this->query['v'])) ? $this->query['v'] : null;

       $this->results = $this->index->getNewsPosts($view);
    }
}

and then my view would look a little better:


<div class="wrapper">
  <ul>
    <li><a href="?">All news</a></li>
    <li><a href="?v=recent">Recent News</li>
    <li><a href="?v=old">Old News</li>
  </ul>
  
  <div class="posts">
  <?php if ($this->results != null) { ?>
    <!-- show the filtered posts -->
  <?php } else { ?>
    <!-- No posts to show / invalid filter -->
  <?php } ?>
  </div>
</div>

That’s a lot cleaner… :eye:

This is the cleanest solution I’ve been able to come up with while also keep it simple.

Consider using an editor with color coded syntax highlighting. Makes it easy to distinguish between php code and html.