CODEWITHSUNDEEP
asp.net-mvcrazorasp.net-mvc-5
MisaelGaray
MisaelGaray

Reputation: 75

Is it bad practice to have logic in view

I have a table and each item on it has a status or state and depending on this status you can either buy an item or set it in a waiting list.

To do that I have an @if statement in my Razor view where I check the status and then conditionally choose which button to render for the appropriate actions.

I have some similar logic in other views for sessions, and roles.

Is it a good practice? Are there other ways to do it?

@if( @item.status == 1 ) {
     <a class="btn btn-default" href="Items/Buy/@item.id">Buy</a>
} else if ( item.status == 2 ) {
    <a class="btn btn-default" href="Items/SetList/@item.id">Add To List</a>
}

Upvotes: 4

Views: 2554

Answers (3)

SvenAelterman
SvenAelterman

Reputation: 1662

While I fundamentally agree with the fact that what you're doing is technically rendering logic, determining whether or not something can be bought or added could be considered business logic.

Also, you might have items other than the buttons that depend on that logic to be displayed or active or something.

I would suggest the following approach:

  1. Define one or more bool properties on your view, such as CanBuy.

  2. The value of the property can be set in the controller or in the property definition in the view, like so

    public bool CanBuy
{
    get { return (status == 1); }
}

  3. Then in your view you would simplify the if statement like this:

    @if(@item.CanBuy ) {
    <a class="btn btn-default" href="Items/Buy/@item.id">Buy</a>
} else if ( item.CanAdd ) {
    <a class="btn btn-default" href="Items/SetList/@item.id">Add To List</a>
}

In addition, Bobby Caldwell's suggestion to abstract this in a HtmlHelper is valuable and could be combined with this method.

Upvotes: 0

David Tansey
David Tansey

Reputation: 6023

There isn't anything wrong with the way you've done this.

When you hear or read about the idea that it is 'bad practice to include logic in your views' it is typically referring to business logic.

There is nothing that prevents one from passing a model into a view that has all kinds of methods available on it, that can then be executed from code in a view -(but this is what you should avoid -- instead getting this done in the controller action).

The logic that you have shown is really rendering logic -- you are conditionally choosing the appropriate HTML to include in your output. No problem at all in my opinion.

If this logic gets too cumbersome in one or more views you can use HTML helpers, PartialViews, and functions in Razor to help with that problem.

Upvotes: 3

Bobby Caldwell
Bobby Caldwell

Reputation: 150

It is not the worst thing in the world, since it is being used to display elements in the view. However I would make an extension method for the HtmlHelper to render the action links you are trying to create. Something like:

public static class HtmlHelperExtensions
{
    public static HtmlString RenderStatus(this HtmlHelper helper, ItemModel item)
    {
        if (item.status == 1)
        {
            return helper.ActionLink("Buy", "Buy", "Items", new { @class = "btn btn-default" });
        }

        if (item.status == 2)
        {
            return helper.ActionLink("Add To List", "SetList", "Items", new { id = item.Id }, new { @class = "btn btn-default" });
        }

        return new MvcHtmlString("");
    }
}

Then in your view, all you would need to do is call

@Html.RenderStatus(item)

Upvotes: 5

Related Questions