Abstracting views

Aren’t your news views the same? if so you should probably do something closer to:


<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 
	$results = array();
	if ($this->query['v'] == 'recent')  $results = $news->findRecent();
	else if ($this->query['v'] == 'old') $results = $news->findOld();
	else $results = $news->findAll();
	foreach ($results as $newsitem) {
  ?>
	<p>$newsitem->title</p>

  <?php } ?>
  
  </div>
</div>

Essentially, though, you don’t have a real model and your state is being stored in the controller which is where you are getting stuck.

Add a model and it gets fixed:

Model


class NewsModel {
	public $query;

	public function getResults() {
		if ($this->query == 'recent')  $results = $this->news->findRecent();
		else if ($this->query == 'old') $results = $this->news->findOld();
		else $results = $this->news->findAll();
		return $results;
	}

}

View


<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 
	foreach ($model->getResults() as $newsitem) {
  ?>
	<p>$newsitem->title</p>

  <?php } ?>
  
  </div>
</div>

Controller


class Controller {

	public function update() {
		$this->model->query = $info['query']['v'];
	}

}


You now have a better separation of concerns: the state is stored in the model (instead of the controller) and the view has more reusability.

This does present a series of new problems (display logic in the model and the model now having mixed responsibilities being the biggest) but it’s the direction to go in.

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.


It’s ugly. VERY ugly.

This one is personal preference. I prefer braces because it’s consistent with the rest of the application. I happen to find braceless syntax ugly and harder to read/maintain you have to worry about closing the correct block in the correct place rather than just having the correct number of braces. Case in point, your example has “endif” where you should have “endforeach”

Go with what you prefer, not what other people say (same goes for whitespace, and brace positioning)