//Arrange Act Assert

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)

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.

No blog post can cover ASP.NET MVC controller best practices in one go. So this is the first post in a series about ASP.NET MVC controller best practices.

In this post we’re looking at why your ASP.NET MVC controllers should be ‘skinny’.

The code used in this post can be downloaded here.

I’ve heard Jeffrey Palermo say if you can’t see a ASP.NET MVC action method on a screen without having to scroll, you have a problem. In most cases the problem is the controller has multiple responsibilities.

The code in this post is adapted from a code sample I saw in an ASP.NET MVC book.

The controller looks like this

using System;
using System.Linq;
using System.Web.Mvc;
using TestingMvcControllers.Interfaces;
using TestingMvcControllers.Models;

namespace TestingMvcControllers.Controllers
{
    public class AnimalsController : Controller
    {
        public int PageSize = 4;
        private readonly IAnimalsRepository _animalsRepository;

        public AnimalsController(IAnimalsRepository animalsRepository)
        {
            _animalsRepository = animalsRepository;
        }

        public ViewResult List(string category, int page)
        {
            var animalsInCategory = (category == null)
                                        ? _animalsRepository.Animals
                                        : _animalsRepository.Animals.Where(x => x.Category == category);

            int numberOfAnimals = animalsInCategory.Count();
            ViewData["TotalPages"] = (int)Math.Ceiling((double)numberOfAnimals / PageSize);
            ViewData["CurrentPage"] = page;
            ViewData["CurrentCategory"] = category;

            return View(animalsInCategory
                            .Skip((page - 1) * PageSize)
                            .Take(PageSize)
                            .ToList()
                );
        }
    }
}

This approach will work, but what if you wanted to implement paging for another user interface? You would not be able to reuse the paging functionality in the controller and have to repeat yourself. This goes against the ‘Don’t Repeat Yourself (DRY) principle.

At this point some of you maybe be saying you ain’t gonna need it (YAGNI). Maybe… but let’s have a look at impact paging logic in the controller has when it comes to the unit testing the ASP.NET MVC controller.

using System.Collections.Generic;
using NUnit.Framework;
using Rhino.Mocks;
using TestingMvcControllers.Controllers;
using TestingMvcControllers.Interfaces;
using TestingMvcControllers.Models;

namespace TestingMvcControllers.Tests
{
    [TestFixture]
    public class AnimalsControllerTests
    {
        private IAnimalsRepository _animalsRepository;
        private AnimalsController _controller;

        [SetUp]
        public void SetUp()
        {
            _animalsRepository = MockRepository.GenerateStub<IAnimalsRepository>();
            _controller = new AnimalsController(_animalsRepository);
        }

        [Test]
        public void List_Presents_Correct_Page_Of_Animals()
        {
            // Arrange
            _controller.PageSize = 3;

            // Act
            _animalsRepository.Stub(a => a.Animals).Return(AnimalsStub.Animals);
            var result = _controller.List(null, 2);

            // Assert
            Assert.IsNotNull(result, "Didn't render view");
            var Animals = result.ViewData.Model as IList<Animal>;
            Assert.AreEqual(2, Animals.Count, "Got wrong number of animals");
            Assert.AreEqual(2, (int)result.ViewData["CurrentPage"], "Page number was wrong");
            Assert.AreEqual(2, (int)result.ViewData["TotalPages"], "Page count was wrong");
            Assert.AreEqual("Iguana", Animals[0].Name);
            Assert.AreEqual("Loin", Animals[1].Name);
        }

        [Test]
        public void List_Includes_All_Animals_When_Category_Is_Null()
        {
            // Arrange
            _controller.PageSize = 10;            

            // Act
            _animalsRepository.Stub(a => a.Animals).Return(AnimalsStub.Animals);
            var result = _controller.List(null, 1);

            // Assert
            Assert.IsNotNull(result, "Didn't render view");
            var Animals = (IList<Animal>)result.ViewData.Model;
            Assert.AreEqual(5, Animals.Count, "Should have got 5 animals");
        }

        [Test]
        public void List_Filters_By_Category_When_Requested()
        {
            // Arrnage
            _controller.PageSize = 10;

            // Act
            _animalsRepository.Stub(a => a.Animals).Return(AnimalsStub.Animals);
            var result = _controller.List("Reptile", 1);

            // Assert
            Assert.IsNotNull(result, "Didn't render view");
            var Animals = (IList<Animal>)result.ViewData.Model;
            Assert.AreEqual(3, Animals.Count, "Should have got 3 animals");
            Assert.AreEqual("Crocodile", Animals[0].Name);
            Assert.AreEqual("Snake", Animals[1].Name);
            Assert.AreEqual("Iguana", Animals[2].Name);
            Assert.AreEqual("Reptile", result.ViewData["CurrentCategory"]);
        }
    }
}

Because the controller is also responsible for paging, you have to write unit tests for paging logic on top of the unit tests you should be writing for the controller. This means you will either ending up not testing enough and/or too much and find the tests a nightmare to maintain in the future. See my post about How to Unit Test ASP.NET MVC Controllers.

How I would refactor

I would move the paging functionality into a new layer. Yes I know adding another layer is always the solution to any problem in software development

The animal service layer looks like this

using System;
using System.Linq;
using TestingMvcControllers.Interfaces;
using TestingMvcControllers.Models;

namespace TestingMvcControllers.Services
{
    public class AnimalsService : IAnimalsService
    {
        private readonly IAnimalsRepository _animalsRepository;
        public int PageSize = 4;

        public AnimalsService(IAnimalsRepository animalsRepository)
        {
            _animalsRepository = animalsRepository;
        }

        public AnimalsDto GetAnimals(string category, int page)
        {
            AnimalsDto animalsDto = new AnimalsDto();

            var animalsInCategory =  (category == null)
                            ? _animalsRepository.Animals
                            : _animalsRepository.Animals.Where(x => x.Category == category);

            int numberOfAnimals = animalsInCategory.Count();
            animalsDto.CurrentCategory = category;
            animalsDto.CurrentPage = page;
            animalsDto.TotalPages = (int)Math.Ceiling((double)numberOfAnimals / PageSize);
            animalsDto.Animals = animalsInCategory
                .Skip((page - 1)*PageSize)
                .Take(PageSize)
                .ToList();

            return animalsDto;
        }
    }
}

Notice how a ‘Dto’ object is returned from the service. While this should be done using an AutoMapper, it’s not the focus of this test. The total number of pages can now be returned to the view as part of the model, rather than as a dictionary entry in ViewData.

The controller now looks clean and simple

using System.Web.Mvc;
using TestingMvcControllers.Interfaces;

namespace TestingMvcControllers.Controllers
{
    public class BetterAnimalsController : Controller
    {
        private readonly IAnimalsService _animalsService;

        public BetterAnimalsController(IAnimalsService animalsService)
        {
            _animalsService = animalsService;
        }

        public ViewResult List(string category, int page)
        {
            var animalsDto = _animalsService.GetAnimals(category, page);
            return View("List",animalsDto);
        }
    }
}

The unit tests for the controller are about testing the controller and nothing more!

using MvcContrib.TestHelper;
using NUnit.Framework;
using Rhino.Mocks;
using TestingMvcControllers.Controllers;
using TestingMvcControllers.Interfaces;
using TestingMvcControllers.Models;

namespace TestingMvcControllers.Tests
{
    [TestFixture]
    public class BetterAnimalsControllerTests
    {
        private IAnimalsService _animalsService;
        private BetterAnimalsController _controller;

        [SetUp]
        public void SetUp()
        {
            _animalsService = MockRepository.GenerateStub<IAnimalsService>();
            _controller = new BetterAnimalsController(_animalsService);
        }

        [Test]
        public void The_List_Action_Returns_AnimalsDto_To_The_View()
        {
            // Arrange
            _animalsService.Stub(a => a.GetAnimals(null,0))
                .IgnoreArguments()
                .Return(new AnimalsDto());

            // Act
            var result = _controller.List(null, 0);

            // Assert
            result.AssertViewRendered().WithViewData<AnimalsDto>();
        }

        [Test]
        public void The_List_Action_Returns_List_View()
        {
            // Arrange
            _animalsService.Stub(a => a.GetAnimals(null, 0))
                .IgnoreArguments()
                .Return(new AnimalsDto());

            // Act
            var result = _controller.List(null, 0);

            // Assert
            result.AssertViewRendered().ForView("List");
        }
    }
}

Jag Reehal’s Final Thought on ‘Skinny ASP.NET MVC Controllers’

There is of course more than one way to skin a cat. This solution could have been refactored in many different ways.

The refactoring done in the post means:

• The ASP.NET MVC controller is skinnier with less responsibility which means it’s easier to read and maintain
• The controller unit tests can focus on the ‘subject under test’
• The paging functionality can be used by multiple user interfaces