views:

231

answers:

7

My question needs a bit of setup, so please bear with me:

I became a convert to using View Helpers for getting data from a model rather than sprinkling it all over the controllers (hat tip to Eric Clemmons). It's much more reusable and flexible there. I just love it!

What I usually do, is lay out the template in the index.phtml, and then when I need to get something from a model, put that snippet in a detail.phtml, so logic is as far out of the way as possible.

However, I start to see the need for variables that get reused. For example, the category name. Now you don't want to use a view helper to get the cat name from a model over and over again. Although you can cache it, it's obviously way too big of a hassle.

So I started using a couple of lines of php in the detail.phtml to set variables. And it doesn't smell right anymore. Views shouldn't have too much logic.

So what say you all? If the var gets reused, put it in the controller? Or don't mind a couple vars set in the view?

EDIT: Alan Storm asked for an example of viewhelpers:

detail.phtml:

<ul id="productList">

<? foreach($this->getProductById($id) as $product) : ?>
    <li><?= $this->escape($product['name']) ?></li>
<? endforeach; ?>

</ul>

(bracing myself for attack of the anti-short-taggers)

ANOTHER EDIT: I see there can't be 2 right answers. Oh well...

+4  A: 

Neither the Controller nor the View are for storing application state. That's what the Model is for.

Keep in mind that a "Model" in MVC is not a database table! The Model is where you implement business logic for the application, and storing it in a database is an internal implementation detail of a Model. But you can have Models in your app that have nothing to do with a database.

Bill Karwin
I'm aware of that, but your view still needs to access data from the Model whether it's a database or a feed or whatever.
joedevon
Right, so the View can call methods or properties of a Model (in a read-only fashion) to get the data it needs to display.
Bill Karwin
So are you suggesting something like this?detail.phtml<h3>Category: <? $this->getCatName() ?></h3><ul id="productList"><? foreach($this->getProductById($id) as $product) : ?> <li><?= $this->escape($product['name']) ?></li><? endforeach; ?></ul>Showing all products for <? $this->getCatName() ?>END detail.phtml
joedevon
Sorry for the bad formatting, point being, call the view helper multiple times to pull same variable and let the model make sure it isn't an expensive request? Doesn't that seem wrong? Or maybe I'm misunderstanding you.
joedevon
Well, I'd think the View would call `$this->catModel->getName()`, and then catModel is reponsible for encapsulation. I.e. it lazy-loads the required data, and caches it in the object so that subsequent calls are quicker. The Controller injects the correct Model objects into the View, and then the View calls these Model objects.
Bill Karwin
+1  A: 

Short answer: IMHO, yes put it in the controller.

Reasons:

1) The controller passing variables into the views is more typical MVC.

2) Maintainability: When I re-visit the view again in 3 months, I don't want to have to look around in adjacent views/templates looking for variables. Now you are back to spaghetti code, having to guess where a particular variable originated from.

Lance Rushing
I definitely wouldn't advocate setting a variable in a view that gets used in any other template than the one it's set. There are definitely limits :)
joedevon
+1  A: 

Hi,

I don't really like this idea of variables : it add more code either in the view or the controller, and it doesn't feel nice.

On the other hand, I like this idea of cache... Event if you think it's too complex / overkill.

Why not find some way in the middle ? Not use some cache like file/APC/memcache, but just keep the data in memory for the execution of the script ?
you can use a static variable for that ; either in your class, or directly in the method (depending on "does it make sense to share that cache between methods of the class ?")

To illustrate this idea, here's a quick portion of code ; consider this class :

class A {
    public function test($param) {
        static $cache = array();
        if (isset($cache[$param])) {
            var_dump("cache hit : $param = {$cache[$param]}");
            return $cache[$param];
        } else {
            // Fetch from DB (here, simulated ^^ )
            $cache[$param] = mt_rand(0, 9999);
            var_dump("cache miss : $param = $cache[$param]");
            return $cache[$param];
        }
    }
}

The test method uses a static variable (there will be one, and only one instance of that variable, shared by any instances of the class) to store the data that has been fetched from the DB.

If you call this that way :

$a = new A();
$b = new A();

$a->test(10);   // miss
$a->test(15);   // miss
$b->test(10);   // hit
$b->test(25);   // miss
$a->test(25);   // hit

You'll get this :

string 'cache miss : 10 = 3745' (length=22)
string 'cache miss : 15 = 7800' (length=22)
string 'cache hit : 10 = 3745' (length=21)
string 'cache miss : 25 = 8623' (length=22)
string 'cache hit : 25 = 8623' (length=21)

Each time the method is called with a new parameter, it's a miss, and you go to the DB. But when it's called when a parameter already used once, the data is in memory -- and you don't go to the DB ;-)

Wouldn't that help ? I'm guessing, in your case, the A class is the view helper, btw ;-) ANd the mt_rand would be a DB query ^^

As a sidenote : this should not be done for too big objects, as it will use some RAM... and don't have lots of those...


Edit : as you are using Zend Framework, you might be interested in using Zend_Memory instead of that static variable : it deal with stuff like the amount of RAM taken (it can delete data from the "cache" if needed, for instance), if I remember correctly.

Also : yes, you are still calling the method many times... But it's better than doing the query... and, this way, neither the View nor the Controller has to care about any kind of "cache" : it's not their job.

And also : I've be using this technic for years without any problem (as long as I only store small objects this way, and not too many of them) ; And I'm not the only one using this ; Drupal uses this too, for instance.

Pascal MARTIN
Interesting response. Will have to mull it over a bit.
joedevon
+1  A: 

I'm not sure the exact technique you're talking about. The following assumes you're creating methods on View Helpers that return information by wrapping calls to your model's "get data" methods. This gets you away from the Passive View pattern employed by many other PHP MVC Frameworks. The views are going directly to the models for their data. Your concers is multiple calls to the the view helper will cause the model to fetch the data twice. This is a potential performance problem that seems easily avoidable.

//example of how I'm assuming you're using view helpers
echo $this->viewHelperName->modelName->getDataIWant('key');

If that accurately describes your problem, the "if I need to use it twice, set a view variable in the controller, otherwise just use the view helper" approach is probably the wrong one. This is more my personal preference, but whichever approach you choose for getting data from the models to your view, you should stick to throughout your application.

The problem you're trying to solve here is "fetching data directly from the models has a high performance cost". This is a problem that the model implementation should fix. It's not something that should be fixed by janky coding style :)

The best solution, as you've already mentioned, is caching. Caching doesn't have to be "too big of a hassle" if you're smart about it.

public function getDataIWant($key, $clear_cache=false)
{
 if(!array_key_exists($key, $this->_cache) || $clear_cache)
 {
  $this->_cache[$key] = parent::getDataIWant[$key];
 }

 return $this->_cache[$key];
}

If caching isn't viable for the way you're using models, then I'd suggest adding a method to your model for getting the data you want, and using extract to define variables in the view scope.

class MyModel
{
 ...
 function getDataForFooView
 {
  return Array(
   'company_name'=>$this->getCompanyName
  );
 }
 ...
}

...
//top of the view file
<?php extract($this->viewHelper->modelName->getDataForFooView()) ?>
<h1><?php echo $company_name; ?></h1>

It's still kind of janky, but you get a consistency in your view files (a single line at the top) that will smell less. That said, caching is "The Right™" way to do this. Avoiding that is just trading one smell for another. One way to think about when you define a view variable (either directly in the view or by setting it in the controller) you're already using caching, just in a haphazard fashion.

Alan Storm
Rereading Bill's comments I think you are both saying the same thing?
joedevon
Yes, my post could be summed up as "The view is for reading data from models (which is what bill said), and that it's good coding practice to read the data from your models in a consistent way throughout the entire application.
Alan Storm
A: 

I am using this in my controllers: $this->view->someVariable = 1;

.... in the view

view->someVariable

lordspace
Does that code work in the view? Isn't it $this->someVariable?
joedevon
A: 

I do almost all variable assigns in the controller. Why? I have multiple views available for every action. I'm using ContextSwitch to serve up feeds for pages in ATOM, RSS and then plain HTML. In many ways i could extend this to an API (json or xml) and oEmbed handling. Now, I'm assigning in my list of model objects, since different views need different data from my models, but I'm just accessing what I've assigned.

The nice thing is I can write the controller once, then write the view script in the way I want to present the data. I'm using a few view helpers here and there for more view-based logic.

Now I guess you could do this with more complex view helpers (and use some sort of registry for data you wish to memoize/cache in the request), but it seems like then you're hiding stuff deeper than you need, but thats maybe a matter of opinion.

Justin
Interesting. The thing is, Models are meant to be portable. You can use it with different frameworks. Whereas the Controller isn't. It sounds like you have a heavy controller. Which tightly couples your code to Zend Framework. Which is OK. But a lot of people are talking about moving complex logic to the Models.A concept I like is Django's templates. You can inherit a template. That way, you don't have to write lots of code in multiple controllers to set up your views. And you don't need to repeat html in your views either. But alas, ZF doesn't have this (yet?).
joedevon
But doesnt moving 'display' type logic in the model tie your view to your model then? How is that better or different? I am using my models in 3 different contexts already (tools type scripts, REST api controllers and Web/feed controllers). Since each of those has their own requirements on how the data is manipulated for usage, that logic in no way belongs to the model. There is ALWAYS going to be some tie to the APIs between M V and C - you want to make it as light as possible, but if you were to 'switch' out one for another, there will always be SOME refactoring.
Justin
It's an interesting debate. I've seen some well regarded architects put real heavy logic in models that shocked me. They are saying that Zend purposely wants you to put it in the model to make it more portable. I dunno... But they say the controller should only be used to control the flow of the application. Which likely doesn't include setting many view variables from the model...
joedevon
A: 

So I started using a couple of lines of php in the detail.phtml to set variables. And it doesn't smell right anymore. Views shouldn't have too much logic.

Views can contain as much display logic as you like. Business logic should be in the model and/or controller, depending on if you prefer heavy or light models.

In my own work, I tend to assign all variables in the controller, unless I am using a view helper to render the navigation, ads, etc. View Helpers are really for things you'll re-use on many parts of the site.

When assigning variables in the controller to the view, and I have a recordset, I tend to loop through that recordset and push them onto an associative array. Instead of passing the actual recordset to the view, I pass it this array.

The reason for this is so that I can:

  1. Manipulate values for display in the controller, not the view (phone: 1234567890 becomes 123-456-7890)
  2. Do any joins or other fetching in the controller
  3. Keep trivial display logic out of the view (i.e. setting the css class for even & odd rows, etc)

Example Controller:

$count = 0;
$list = array();
$result = mysql_query("select * from items limit 10");
while($item = mysql_fetch_object($result))
{
   if($count % 2 ==0){ $css_class = 'even'; } else { $css_class = 'odd'; }
   $count++;
   $item->css_class = $css_class;

   if($item->first_name && $item->last_name)
   {
      $item->name = $item->first_name.' '.$item->last_name;
   }
   else
   {
      $item->name = $item->username;
   }

   $list[] = $item;
}
$this->view->list = $list;

Example View:

<?foreach($this->list as $item):?>
<tr class="<?=$item->css_class?>">
    <td><?=$this->escape($item->name)?></td>
</tr>
<?endforeach;?>
lo_fye