How much code should be in a CodeIgniter view?

First steps are done, some pages are coded. And after looking to the result I开发者_开发问答've got the question... how much code is there should be in templates (View)?

For example, here is a template file:

<?php $this->load->view('header'); ?>
<?php $this->load->view('banner'); ?>   

<div id="items">
<?php
for($i=0; $i<count($main); $i++) {
    echo '<div class="item">
        <div class="name">'.$main[$i]['name'].'</div>';
    if($main[$i]['icq']=='') { }
        else { echo '<div class="phone">'.$main[$i]['phone'].'</div>'; }
    echo '</div>';
}
?>
</div>

<?php $this->load->view('footer'); ?>

Do you think there is too much code in this template or it is normal?

The first answer was actually spot on, but the user deleted it (probably due to peer pressure). Basically, you do not want any logic in your template. In an ideal world, you had a tag for all the model data, but since we are in an HTML world, there isn't, so you either have to use XSLT or use ViewHelpers.

Let's focus on the ViewHelper approach.

This is hard to maintain:

<div id="items">
<?php
for($i=0; $i<count($main); $i++) {
    echo '<div class="item">
        <div class="name">'.$main[$i]['name'].'</div>';
    if($main[$i]['icq']=='') { }
        else { echo '<div class="phone">'.$main[$i]['phone'].'</div>'; }
    echo '</div>';
}
?>
</div>

And it won't get any better if you replace the PHP with Smarty. This is easy to maintain:

<div id="items">
    <?php echo itemlist($items, 'template.htm') ?>;
</div>

Below the now deleted question, there was a commentor objecting this "code isn't easy to maintain for non-coders because now they don't know where itemlist is defined and what it does." but that's complete gibberish. Think about it for a second.

On the one hand, they claim non-coders will have issues with a simple function call but on the other hand they expect them to understand the mishmash of PHP code mixed with HTML. A designer does not care about the presentation logic, but just the actual presentation. The output.

A single function call clearly says: "here be an item list", which is much easier to grasp than "here is a for that echoes a div and maybe something else if icq is given". A function call is as good as a tag. It has clearly defined input and output. How it achieves that output is irrelevant to anyone but the developer.

Your ViewHelper encapsulates the presentation logic. It's a snippet you can reuse across all of your Views. That is much more maintainable than copy and pasting all that logic over and over again when needed. In the example above, there is two arguments to the helper:

• $items is an array or other traversable type holding your actual item data
• 'template.htm' is the filename of the template used to render the data

I'll make the second one optional, because I'd assume it's always the same template anyway. But since the commentor complained the non-coder wouldn't know where to look, I felt it necessary to show how easy it is to tell the non-coder where to look.

function itemlist($items, $template = './templates/itemlist.htm') {
    ob_start();
    foreach($items as $item) {
        include $template;
    }
    return ob_get_flush();
}

There might be more efficient approaches to solve the inclusion of the template. The main idea should be clear though. Hide the presentation logic from the actual template. Your "template.htm" would then look like this:

<div class="item">
    <div class="name"><?php echo $item['name'] ?></div>
    <?php echo contact($item, 'icq' 'phone'); ?>
</div>

No if and elses. No string concatenations or curly braces. The logic to decide how to the user can be contacted is hidden in a ViewHelper as well. All the non-coder has to know now is the arguments to the ViewHelpers and that's as easy as knowing which attribute to write to a tag. Give them a cheat sheet if necessary.

Related information:

• http://martinfowler.com/eaaCatalog/templateView.html

EDIT

Due to the two comments below I decided to expand on this answer. The above is not abstraction for abstraction's sake. It is for reusability and maintainability. I've already pointed that out above but let me explain that here again.

Actually, I find it odd to object to using ViewHelpers because you'll have "have presentation in two places" but not complain about separating header, banner and footer. It's the same thing. You isolate parts that are reusable and put them into their own templates. Separating the logic from the template in that step is just the natural next step to achive even more maintainability.

A view template that contains logic is effectively a script and not a template. Any view template that contains the logic to assemble itself is doomed to repeat itself. This might not be an issue with small sites but if you are working on a site with a couple dozen or even hundreds of view and widgets, not abstracting these parts will lead to code duplication. Put all the logic into the template and it will quickly become a tangled mess of c&p'ed markup mixed with conditionals. For any duplication, you'll double the time it takes to change it. Add inline styles and obtrusive Javascript and you are in maintenance hell.¹

If you use OOP for the other parts of your application, then why would you go procedural in your view? If you understood that you should separate Javascript and CSS from your HTML, why would you mix PHP into your template? It doesn't make sense. The OP's code snippet is a script and not a template. As such, it is the domain of the developer. And because of that you apply all the good practise you apply to other parts of your application as well. And that includes isolating logic into functions and/or classes.

Granted, a designer looking into a script might not immediately know what's going on. But then again, she might not know that either once you start to add in native PHP functions. What's mb_strimwidth doing? And substr? What's that ?: construct? The more actual PHP you add, the harder it will be to read for non-developers.

If you want to have designers work on templates, don't give them scripts. Give them templates. And if you do, isolate the logic from it and replace it with easy to grasp function calls. Use function names that clearly communicate what the function does. So the designer only needs to know if "I use this with that input, I will always get that output. I don't care how that output comes to be. I'll leave that to the developer".

Isolating the logic into functions (or classes) also gives you the immediate advantage of being able to test the logic used to render specific parts on that page in isolation. You don't have to setup the entire environment required to create a page, but just pass in the required input and assert it's output (that's why the function buffers the string instead of outputting it btw).

¹^{For those of you who think this is not an issue, I strongly suggest to find yourself a legacy application that truly keeps all the logic in the view template. Try to change a few things. Once you have had the pleasure of eating through 2500 lines of spaghetti code with at least 500 chars each and containing a mix of repeated PHP, HTML, CSS and JavaScript you'll know what I am talking about.}

I don't think what you're trying to achieve in the view is too much logic, but I think it should be cleaned up somewhat.

Something like:

<?php foreach($main as $item): ?>
<div class="item">
    <div class="name"><?php echo $item['name']; ?></div>
    <?php if($item['icq']): ?>
        <div class="phone"><?php echo $item['phone']; ?></div>
    <?php endif; ?>
</div>
<?php endforeach; ?>

I find that a lot easier to read, and it does the same job. I also don't think it would be hard for a non-coder to modify, which is ultimately the goal of a view.

When the templates need a lot of logic is could be useful to introduce the ViewModel pattern

The logic that determines when a certain block is shown is placed in the view model.

Example
A template snippet:

<?php if($item['show_phone_number']): ?>
    <div class="phone"><?php echo $item['phone_number']; ?></div>
<?php endif; ?>

A view model snippet:

$item['show_phone_number'] = $item["icq"] == '';

short answer:

As little as is viably possible.

View is no a template.

Or at least - it should not be a template. Views are generally instance of classes, which are responsible for dealing with presentational logic. They juggle multiple templates and decide which one to use and what data pass into them.

But CI has none of this. You are basically stuck with a template, which pretends to be "View" , and and instances of ActiveRecord, which they tell you are "Models". Hell .. in proper MVC the Model isn't even a specific class but an application layer.

In long term, this situation will force both presentation logic and business logic into the controller, while at the same time, producing hard-to-maintain templates and activerecord classes.

I think as James, too much code, but you can add an array variable to the get_items() function, to control the prefix and suffix of each item, something like:

$options = Array('item_prefix' => '<div class="item">', 'item_suffix' => '</div>'...)
echo get_items();

Your views should definitely contain most of the html tags, so I do not think that a function get_items should be placed somewhere else. Just clean up your templates a little bit and work with the alternative loop syntax as suggested by Karpie. You could also think about using some existing templating engine like Twig but with that the views have to contain some references to your data as well... What you want to achieve is that non-coders can edit your html files without having to understand too much scripting logic.