ASP.NET MVC View Best Practices – Keep logic out of your views

When it comes to views in ASP.NET MVC, you won’t be short of options how you decide how to create them.

One of the easiest mistakes is to implement logic in their views.

It’s no surprise developers such as Rob Conery have advised against having conditional code in ASP.NET MVC views. Before you know it a view that was simple and easy to understand becomes a complex monster intertwined with html and logic.

The code used in this post can be downloaded here.

The fact is ASP.NET MVC views which contain logic will ultimately end up being a nightmare to maintain and test.

Here is an example of what I mean. When displaying product information, our client wants to show to the number of units in stock. The background colour should reflect the stock level as shown below (along with the css class name)

Number In Stock Colour should be Css class name
0 Red stockLevelNone
1-20 Yellow stockLevelLow
Over 20 Green stockLevelHigh



This could be implemented by creating the view like this

<div class="stockLevel
   <%=Model.UnitsInStock == 0 ? " stockLevelNone" : String.Empty%>
   <%=Model.UnitsInStock > 0 && Model.UnitsInStock <= 20 ? " stockLevelLow" : String.Empty%>
   <%=Model.UnitsInStock > 20 ? " stockLevelHigh" : String.Empty%>
   ">
    <span>Number Of Units In Stock: <%=Model.UnitsInStock %></span>
</div>

Because there is logic in the ASP.NET MVC view, you have to go through the laborious process of creating front end tests or creating a product, setting the number in stock and then checking the correct class is used by viewing the page source for each condition. And if that wasn’t difficult enough, imagine doing that every time you do a release.

So what if I said there are alternative methods which could:

  • reduce the chance of typos in your html
  • clean up your view
  • be testable
  • be reused so you can follow the “Don’t Repeat Yourself (DRY)” principle which will save you so much time!

Option1 – Move the logic for choosing the css class into a helper method

The conditional logic in the ASP.NET MVC view can be replaced by a method which takes in the number of units in stock and returns the appropriate css class as shown below.

using System;

namespace AspNetMvcViewBestPractices.Helpers
{
    public static class ProductCssHelper
    {
        public const string STOCK_LEVEL_NONE_CSS_CLASS = "stockLevelNone";
        public const string STOCK_LEVEL_LOW_CSS_CLASS = "stockLevelLow";
        public const string STOCK_LEVEL_HIGH_CSS_CLASS = "stockLevelHigh";

        public static string GetCssClassForUnitsInStock(int unitsInStock)
        {
            if (unitsInStock == 0)
            {
                return STOCK_LEVEL_NONE_CSS_CLASS;
            }
            if (unitsInStock > 0 && unitsInStock <= 20)
            {
                return STOCK_LEVEL_LOW_CSS_CLASS;
            }
            if (unitsInStock > 20)
            {
                return STOCK_LEVEL_HIGH_CSS_CLASS;
            }
            return String.Empty;
        }
    }
}

In the view the number of units in stock is passed to the css helper method

<div class="stockLevel <%=ProductCssHelper.GetCssClassForUnitsInStock(Model.UnitsInStock)%>">        
    <span>Number Of Units In Stock: <%=Model.UnitsInStock %></span>
</div>

And the css helper method can now be unit tested as shown below

using System;
using AspNetMvcViewBestPractices.Helpers;
using NUnit.Framework;

namespace AspNetMvcViewBestPractices.Tests
{
    [TestFixture]
    public class ProductCssHelperTests
    {
        [Test]
        public void Will_Return_StockLevelNone_Css_Class_If_There_Are_No_Units_In_Stock()
        {
            // Arrange
            int unitsInStock = 0;

            // Act 
            string cssClass = ProductCssHelper.GetCssClassForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(ProductCssHelper.STOCK_LEVEL_NONE_CSS_CLASS,cssClass);
        }

        [Test]
        public void Will_Return_StockLevelLow_Css_Class_If_Units_In_Stock_Is_Between_One_And_Twenty(
            [Range(1, 20)] int unitsInStock)
        {
            // Arrange        

            // Act 
            string cssClass = ProductCssHelper.GetCssClassForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(ProductCssHelper.STOCK_LEVEL_LOW_CSS_CLASS, cssClass);
        }


        [Test]
        public void Will_Return_StockLevelHigh_Css_Class_If_Units_In_Stock_Is_Over_Twenty()
        {
            // Arrange        
            int unitsInStock = 21;

            // Act 
            string cssClass = ProductCssHelper.GetCssClassForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(ProductCssHelper.STOCK_LEVEL_HIGH_CSS_CLASS, cssClass);
        }

        [Test]
        public void Will_Return_EmptyString_If_Units_In_Stock_Is_Less_Then_Zero()
        {
            // Arrange        
            int unitsInStock = -1;

            // Act 
            string cssClass = ProductCssHelper.GetCssClassForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(String.Empty, cssClass);
        }
    }
}

Option 2 – Create a method to build the html for displaying the number of units in stock

I first saw this approach in Mike Hadlows’ Suteki Shop. Here an html helper method takes in the number of units in stock and returns an html string

using System;

namespace AspNetMvcViewBestPractices.Helpers
{
    public static class ProductHtmlHelper
    {
        public const string STOCK_LEVEL_NONE_CSS_CLASS = "stockLevelNone";
        public const string STOCK_LEVEL_LOW_CSS_CLASS = "stockLevelLow";
        public const string STOCK_LEVEL_HIGH_CSS_CLASS = "stockLevelHigh";
        public const string UNITS_IN_STOCK_HTML = @"<div class=""stockLevel {0}""><span>Number Of Units In Stock: {1}</span></div>";

        public static string GetHtmlForUnitsInStock(int unitsInStock)
        {
            if (unitsInStock == 0)
            {                
                return String.Format(UNITS_IN_STOCK_HTML,STOCK_LEVEL_NONE_CSS_CLASS,unitsInStock);
            }
            if (unitsInStock > 0 && unitsInStock <= 20)
            {
                return String.Format(UNITS_IN_STOCK_HTML, STOCK_LEVEL_LOW_CSS_CLASS, unitsInStock);
            }
            if (unitsInStock > 20)
            {
                return String.Format(UNITS_IN_STOCK_HTML, STOCK_LEVEL_HIGH_CSS_CLASS, unitsInStock);
            }
            return String.Format(UNITS_IN_STOCK_HTML, String.Empty, unitsInStock);
        }
    }
}

In the view a call is made to the html helper method passing the number of units in stock

<%=ProductHtmlHelper.GetHtmlForUnitsInStock(Model.UnitsInStock)%>

The html helper can be unit tested like this

using System;
using AspNetMvcViewBestPractices.Helpers;
using NUnit.Framework;

namespace AspNetMvcViewBestPractices.Tests
{
    [TestFixture]
    public class ProductHtmlHelperTests
    {
        [Test]
        public void Will_Return_StockLevelNone_Html_If_There_Are_No_Units_In_Stock()
        {
            // Arrange
            int unitsInStock = 0;
            string expectedResult = @"<div class=""stockLevel stockLevelNone""><span>Number Of Units In Stock: 0</span></div>";

            // Act 
            string html = ProductHtmlHelper.GetHtmlForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(expectedResult,html);
        }

        [Test]
        public void Will_Return_StockLevelLow_Html_If_Units_In_Stock_Is_Between_Zero_And_Twenty(
            [Range(1, 20)] int unitsInStock)
        {
            // Arrange        
            string expectedResult = String.Format(@"<div class=""stockLevel stockLevelLow""><span>Number Of Units In Stock: {0}</span></div>", unitsInStock);

            // Act             
            string html = ProductHtmlHelper.GetHtmlForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(expectedResult, html);
        }


        [Test]
        public void Will_Return_StockLevelHigh_Html_If_Units_In_Stock_Is_Over_Twenty()
        {
            // Arrange        
            int unitsInStock = 21;
            string expectedResult = @"<div class=""stockLevel stockLevelHigh""><span>Number Of Units In Stock: 21</span></div>";

            // Act 
            string html = ProductHtmlHelper.GetHtmlForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(expectedResult, html);
        }

        [Test]
        public void Will_Return_Html_With_NoStockLevelIndicator_If_Units_In_Stock_Is_Less_Then_Zero()
        {
            // Arrange        
            int unitsInStock = -1;
            string expectedResult = @"<div class=""stockLevel ""><span>Number Of Units In Stock: -1</span></div>";

            // Act             
            string html = ProductHtmlHelper.GetHtmlForUnitsInStock(unitsInStock);

            // Assert
            Assert.AreEqual(expectedResult, html);
        }
    }
}

Option 3 – Use a Product data transfer object with a css class name property

Instead of passing the product model into the view, we could map it into a data transfer object that has a property for the css class name. For this to work the controller needs to perform the mapping before returning a model to the view. I will be covering the why you should avoid using domain models directly into views in a future blog post.

The view model/data transfer object is shown below

namespace AspNetMvcViewBestPractices.Models
{
    public class ProductDto
    {
        public Product Product { get; set; }
        public string UnitsInStockCssClassName { get; set; }
    }
}

The mapping class looks like this

using AspNetMvcViewBestPractices.Helpers;
using AspNetMvcViewBestPractices.Models;

namespace AspNetMvcViewBestPractices.Mappers
{
    public static class ProductDtoMapper
    {
        public static ProductDto Map(Product product)
        {
            ProductDto productDto = new ProductDto();
            productDto.Product = product;
            productDto.UnitsInStockCssClassName =  ProductCssHelper.GetCssClassForUnitsInStock(product.UnitsInStock);
            return productDto;
        }
    }
}

And can be unit tested like this

using AspNetMvcViewBestPractices.Mappers;
using AspNetMvcViewBestPractices.Models;
using NUnit.Framework;

namespace AspNetMvcViewBestPractices.Tests
{
    [TestFixture]
    public class ProductDtoMapperTests
    {
        [Test]
        public void Can_Map_Product_to_ProductDto()
        {
            // Arrange
            Product product = new Product();
            product.UnitsInStock = 10;

            // Act 
            ProductDto productDto = ProductDtoMapper.Map(product);

            // Assert
            Assert.AreEqual(product, productDto.Product);
        }
    }
}

The controller uses the Product mapper as shown below

public ActionResult DisplayProductUsingMapper()
{
    Product product = new Product();
    Random random = new Random();
    product.UnitsInStock = random.Next(0, 30);

    ProductDto productDto = ProductDtoMapper.Map(product);
    return View(productDto);
}

Now the view does not need to make any method calls and only uses properties in the view model

<div class="stockLevel <%=Model.UnitsInStockCssClassName%>">        
    <span>Number Of Units In Stock: <%=Model.Product.UnitsInStock %></span>
</div>

Option 4 – When you need to show all css options in the view

I created this option after a comment I received. If you’re working with designers and/or want to show the css classes in the view this option will be to go for. Similar to first option we can use a css helper but this time we pass in the the number of units in stock and the css classes as shown below

using System;

namespace AspNetMvcViewBestPractices.Helpers
{
    public static class ProductCssHelper
    {
        public static string SelectCssClassForUnitsInStock(int unitsInStock, string stockLevelNone, string stockLevelLow, string stockLevelHigh)
        {
            if (unitsInStock == 0)
            {
                return stockLevelNone;
            }
            if (unitsInStock > 0 && unitsInStock <= 20)
            {
                return stockLevelLow;
            }
            if (unitsInStock > 20)
            {
                return stockLevelHigh;
            }
            return String.Empty;
        }
    }
}

In the view the number of units in stock and the css classes are passed to the css helper method

<div class="stockLevel <%=ProductCssHelper.SelectCssClassForUnitsInStock(Model.UnitsInStock,"stockLevelNone","stockLevelLow","stockLevelHigh")%>">        
    <span>Number Of Units In Stock: <%=Model.UnitsInStock %></span>
</div>

And can be unit tested like this

using System;
using AspNetMvcViewBestPractices.Helpers;
using NUnit.Framework;

namespace AspNetMvcViewBestPractices.Tests
{
    [TestFixture]
    public class ProductCssHelperTests
    {     
        [Test]
        public void SelectCssClassForUnitsInStock_Returns_First_Css_Class_If_There_Are_No_Units_In_Stock()
        {
            // Arrange
            int unitsInStock = 0;

            // Act 
            string cssClass = ProductCssHelper.SelectCssClassForUnitsInStock(unitsInStock,"a","b","c");

            // Assert
            Assert.AreEqual("a", cssClass);
        }

        [Test]
        public void SelectCssClassForUnitsInStock_Returns_Second_Css_Class_If_Units_In_Stock_Is_Between_One_And_Twenty(
            [Range(1, 20)] int unitsInStock)
        {
            // Arrange        

            // Act 
            string cssClass = ProductCssHelper.SelectCssClassForUnitsInStock(unitsInStock, "a", "b", "c");

            // Assert
            Assert.AreEqual("b", cssClass);
        }


        [Test]
        public void SelectCssClassForUnitsInStock_Returns_Third_Css_Class_If_Units_In_Stock_Is_Over_Twenty()
        {
            // Arrange        
            int unitsInStock = 21;

            // Act 
            string cssClass = ProductCssHelper.SelectCssClassForUnitsInStock(unitsInStock, "a", "b", "c");

            // Assert
            Assert.AreEqual("c", cssClass);
        }

        [Test]
        public void SelectCssClassForUnitsInStock_Returns_Empty_String_If_Units_In_Stock_Is_Less_Then_Zero()
        {
            // Arrange        
            int unitsInStock = -1;

            // Act 
            string cssClass = ProductCssHelper.SelectCssClassForUnitsInStock(unitsInStock, "a", "b", "c");

            // Assert
            Assert.AreEqual(String.Empty, cssClass);
        }
    }
}

Jag Reehal’s Final Thought on ‘ASP.NET MVC View Best Practices – Keep logic out of your views’

I like my views to contain html but not logic so I prefer options 1, 3 and 4. Because the html is hidden in option 2 finding out what method rendered what html could be like finding a needle in a haystack. In addition the the unit tests for the html helper method are difficult to maintain as you have to check the entire html string is correct.

If you’re working with a designer you should choose option 4.

Comments

  • http://serialseb.blogspot.com Sebastien Lambla

    I take objection to putting the CSS class in the controller.

    I suppose it depends what you see your controller responsibility as being, but for me it’s the gateway between http and your system. As such, templating concerns should stay within the template, and the controller should have no idea about it, and no involvement in it.

  • http://blog.webdistortion.com Paul Anthony

    As you’ve mentioned – rendering HTML in the DLL is a no-no – and makes life difficult for a design team. HTML strings in a build should be avoided like the plague.

  • http://www.mikehenry.name/ Mike Henry

    Thanks for sharing this. What are your thoughts about putting the logic from option 1 into your view model, e.g. Model.GetCssClassForUnitsInStock()? Or do you think it’s better to keep the view model a strict DTO?

  • Richard Miller

    I think you are doing a great thing highlighting the use of view models any why they should be used more. I think the issue raised by Sebastien is a big one though. The views you have written have just become un-maintainable from a designers perspective.

    What a designer should be doing is controlling the css and placing the data (from a view model) appropriately within the tags. All the options presented don’t allow this and force the designer down the route of contacting a developer for a re-compile if they want to alter a class.

    Yes the business logic of 3 levels should be controlled in code not the view as you highlighted, but isn’t that where the view model comes in? In this case it’s something we want to do a lot so we can delegate the detail from the view model to a helper as you have done but I feel the key is not to hide anything the designer may want to change.

    Why not have a method on the view model with the following signature string StockLevelCss(string noneCss, string lowCss, string highCss) This would make the decision about which (css) string to return based on the stock level known within the view model. Tests would be simple, straightforward and long lasting.

    Within the view the designer now has control to alter the css as they wish, ok so there is the chance of typos within the view but that’s an acceptable price to pay as the toolset can easily help you there.

    If the business logic changes to have 4 levels then the method can either gracefully degrade to return one of 3 levels. Or more preferable fail MVC view compilation, highlighting the issue for the designer to fix.

    My vote is for option 4 keep css and tags out of code.

  • Samual Jenkins

    I vote for option 3 because the view model should be a strict DTO and NOT contain methods.

  • http://www.arrangeactassert.com Jag Reehal

    Hi Richard,

    Thanks for your comment.

    I have updated the post with you’re suggestion. I would not add a method onto the view model to do this but that’s down to personal choice.

    Just be aware if your workplace requires developers and designers to work together you might not be able to use some of the templating features coming in ASP.NET MVC 2 such as UIHints.

    Also if you’re compiling your ASP.NET MVC views make sure this is only done on your CI server not on your machine because it increases build time. Check out this DimeCast about Performing Static Page Checking in MVC.

  • http://weblogs.asp.net/jaimedelpalacio Jaime

    What are your thoughts (and how to solve) on “authorization/display/hide controls” logic in the view. For example, let’s say I have a view with a bunch of interaction controls/buttons that need to be enable/disable or shown/hidden depending on some business rules. The controllers fills up a view model with a bunch of “CanAddXXX” or “CanChangeYYY” properties that it got from the business service. So what are the options:

    1. Have a bunch of if’s in the view to control that gets rendered…. Back to the logic in the view, granted is “view” logic, but untestable, hard to maintain, etc.

    2. Return different views depending on the combination of shown/hidden controls… this could get really messy really fast if you have many combination

    3. Have a helper class emit the correct html… testable but we are back to the “who render that html”

    Any other thoughts?

  • http://www.arrangeactassert.com Jag Reehal

    Hi Jamie,

    You’re right to question what to do here because as soon as you open the ‘code smell’ Pandora’s box it’s difficult if not impossible to close.

    Would it be possible to add boolean properties to your view model, such as CanAddUserVisible?

    If so you could use an html helper in the view model to determine if the display value of the button/control should be block/inline or none it should be hidden.

    This approach would be testable and could be done using a single view without any conditional logic.

    I’ll try a write a blog post in the next couple of days with an example of the above.

    In the meantime, if you come up with an alternative solution let me know.

    Cheers,

    Jag

  • rgavrilov

    Note: When I say “should”, I mean “ideally should”
    View’s responsibility is to render given information.
    Controller’s responsibility is to pass data to View for rendering .
    Using Product DTO is the closest to the ideal solution. Except method “UnitsInStockCssClassName”.ViewModel should not be (ideally) aware of the rendering mechanism. CSS is a view specific concept, should not leak from the view.
    So better solution is to replace “string UnitsInStockCssClassName” with something that is inherently Product property, and does not make assumptions about view implementation.

    We add another property to the product UnitsInStockRange which specifies a category a given number of units belongs too. For example: public enum QuanitytRange { Low, Medium, High }
    and the method becomes:

    QuanitytRange UnitsInStockQuanityRange { get; set; }

    this way your Controller is ignorant about specifics of the view implementation.
    and it is up to your View to decide how to render the Product.

    Most elegant way I found to do this is:

    <div class="stockLevel stockLevel-”>    Number Of Units In Stock:

    Note that enum value will be rendered as a string here (e.g. stockLevel-High)

    and your CSS will look something like:

    stockLevel-Low
    {
        color: red;
    }