Removing logic from partial views in ASP.NET MVC

Question

I am aware that views should not have code in them but in a project I am working on I have a lot of logic in the views.

My home page has

<% Html.RenderPartial("SearchResults"); %>

Now in the partial view I have an aweful lot of logic like this;

<div id="RestaurantsList">
<%if (Model.restaurantsList.Count() > 0)
{
    foreach (var item in Model.restaurantsList)
    { %>
        <% Html.RenderPartial("SearchResult", item); %>

    <%
    } %>
<%
}
else
{
    Html.RenderPartial("NoResults");

} %>

Now I could make the home controller return a different view based on the list being empty but I don't really want that as the Index view has a few things that I want displayed no matter if there are results or not.

The only other thing I can think of here is to encapsualte this in a helper method like Html.SearchResults. But then I would need the helper to call the renderPartial for each search result also. That doesn't seem like clean seperation of concerns.

I would still have to have the first if statement in the partial view though.

How would you best handle this?

Answer 1

My personal opinion is that this is okay. The logic that you've used is totally related to how the model needs to be displayed.

You just need to be aware and make sure that you're never mixing in business logic, data access logic or anything else that isn't strictly tied in to the display of the model.

Answer 2

I agree with Praveen Angyan's answer. The only thing I could say to extend his answer is to put some of the logic in the ViewModel.

For example in the ViewModel you could hide

Model.restaurantsList.Count() > 0

behind a method or property.

E.G.:

<%if (Model.HasResturant){...}%>

Answer 3

Hi Dean,

this answer has nothing to do with your question.

However, I just want to let you know that calling Html.RenderPartial() inside a loop is not efficient.
http://stackoverflow.com/questions/1331627/asp-net-mvc-for-loop-inside-renderpartial-or-outside-renderpartial

Changing it to something like below would be better.

<%if (Model.restaurantsList.Count() > 0)
{
    // render the Restaurant item right away
    foreach (var item in Model.restaurantsList) { %>
        <div>
            <%= Html.Encode(item.RestaurantName); %><br />
            <%= Html.Encode(item.Address); %>
        </div>
    <% }
}
else
{
    Html.RenderPartial("NoResults");    
} %>

Answer 4

Praveen Angyan is correct - that's view logic and it's fine that it is where it is.
But that does not change need for tidier views.

Just wanted to share small improvement.
If we attach tiny HtmlHelper method, we can shorten view to something like this:

<div id="RestaurantsList">
<% if (Model.HasRestaurants)
    Html.RenderPartialForEach("SearchResult", Model.restaurantsList);    
else    
    Html.RenderPartial("NoResults"); %>
</div>

For some - it might not look readable and nice, but it fits for me good enough.