When to seperate condition/logic between controller and view?

I notice that I have some simple logic and conditions (if...else) all over my view, instead of controller. They aren't necessarily complex conditions because they just output different text based on the output.

I just don't know how to determine when to put in the controller, when not to put in the controller. Is there any pe开发者_如何学运维rformance issue?

Thanks.

Having some logic in the view is not bad per se, as long as you have the roles of the controller (what to do and what to show) and view (how to show) separated. This helps you have understandable and maintainable code. A side note: models are where all the "how to do"s are.

The controller is where the user interacts with the application and its models, and the view is what gets displayed to the user. A rule of thumb is if you have to respond to the user's input in some way in the view, then your logic's in the wrong place.

Good view logic:

<p><%= @user.name -%> <%= '(admin)' if @user.admin? -%></p>
<ul>
<% @user.documents.each do |document| %>
  <%= render "/documents/#{document.format}_list", :document => document %>
<% end %>
</ul>

This is bad:

<% @user = User.find params[:id] %>

It's not a performance issue. It's a maintenance issue. For example, if you have a logic in view, it is not easy to write a test for the logic in the system. If it is in controller, it's a snap to write a test. If you have lots of logics in view, it will be problem with webform all over again.

The issue isn't performance, it's separation of concerns. The controller should contain business logic; For example, the logic used to determine what data to fetch given some user input.

The controller then passes data to the view. The logic in the view should concern itself only with how to display the data to the user. Do certain things need to be pluralized? Should certain UI elements be displayed/hidden based on the data returned? The logic for these things are simple and should be the responsibility of the view.

Based on your question, it seems you have the right idea, so long as the simple logic in your view isn't business logic.

I actually just spent some time on my side projects factoring out if statements from my templates in an mvc (or at least aspiring to be mvc) site that I'm working on.

If you want to stick to the strictest meanings of mvc, then there shouldn't be ANY logical constructs in your view. So even when rendering a set - lets say a list - you should have a foreach loop which (from your controller) has the arguments for a piece of the template, and calls that piece over and over again. The thing I don't like about having to do this is that it can lead to very inefficient code if you aren't careful about how you aggregate your final, composite view which has all of the pieces of your view. This is especially true if the template library you're using has a render method b/c if you're doing things this way - you're likely to call "render" over and over again. Basically, you're likely to end up doing string aggregation as opposed to building your view with a single stream.

So - what I've been doing, is I have template files I've actually been labeling as "controllers". So, in here, I'll have IF/FOREACH or other logical constructs and calls to the proper subtemplates/subviews. I don't think anyone has said that your template file MUST be JUST A VIEW. I think the paradigm is that your VIEW must NOT have logical constructs. Isolating those logical constructs by making template files which are actually controllers as opposed to pieces of your view, should provide a happy medium.

Example (using google closure template library)

Mixing logic and view (not good...but you don't really find out why until you've built a complicated view this way):

/**
 *
 * Template description
 *
 * @param paramA description of paramA 
 * @param paramB description of paramB
 * @param paramC description of paramC
 *
 *
 */
{template .myTemplate}
     <div class="{$paramA}">
       {if $paramC}
         <span class="{$paramB}">Some content</span>
       {/if}
     </div>
{/template}

Separating logic and view. Note how the controller template file doesn't add anything to the view. It just calls another template.

Controller template file:

/**
 *
 * Template description
 *
 * @param condition description of condition
 * @param parameters parameters to call subtemplate with
 *
 */
{template .myTemplate}
     {if $condition}
          {call .getTemplForCondition data="$parameters" /}
     {else}
          {call .getTempl data="$parameters" /}
     {/if}
{/template}

And then template file...now with no logic in view...

/**
 *
 * Template description
 *
 * @param paramA description of paramA 
 * @param paramB description of paramB
 *
 */
{template .getTemplForCondition}
     <div class="{$paramA}">
         <span class="{$paramB}">Some content</span>
     </div>
{/template}

/**
 *
 * Template description
 *
 * @param paramA description of paramA 
 *
 */
{template .myTemplate}
     <div class="{$paramA}"></div>
{/template}

In Rails terms the model acts as a middle tier in a multi-tier app. Put you logic in the models NOT the controllers or the views. If a view or a controller needs to make a choice over what to display then ask the model if it's ok to do something and implement the function in the model.

<% if @model.sais_its_ok %>
  OK Go ahead
<% else %>
  Sorry, Not allowed!
<% end %>

The best fit for this scenario is to use view helpers but in controllers it's fine to ask the model. Indeed controllers ask the model all the time without you even realising it.

They will tell a model to save ther data. The model will run validations and if they fail they will tell the controller that it couldn't do this and the model will even tell the controller why (and the vuiews) in the errors hash.

Please if you com from another language remember that the model is the middle tier, controllers just have the job of passing data between the middle tiere (models and the views. This is what a true MVC architecture is all about.

Reply from @jimworm demonstrates this perfectly in his reply with asking the model if it is an admin?