Skinny controller in ASP.NET MVC 4
- by thangchung
Rails community are always inspire a lot of best ideas. I really love this community by the time. One of these is "Fat models and skinny controllers". I have spent a lot of time on ASP.NET MVC, and really I did some miss-takes, because I made the controller so fat. That make controller is really dirty and very hard to maintain in the future. It is violate seriously SRP principle and KISS as well. But how can we achieve that in ASP.NET MVC? That question is really clear after I read "Manning ASP.NET MVC 4 in Action". It is simple that we can separate it into ActionResult, and try to implementing logic and persistence data inside this. In last 2 years, I have read this from Jimmy Bogard blog, but in that time I never had a consideration about it. That's enough for talking now.
I just published a sample on ASP.NET MVC 4, implemented on Visual Studio 2012 RC at here. I used EF framework at here for implementing persistence layer, and also use 2 free templates from internet to make the UI for this sample.
In this sample, I try to implementing the simple magazine website that managing all articles, categories and news. It is not finished at all in this time, but no problems, because I just show you about how can we make the controller skinny. And I wanna hear more about your ideas.
The first thing, I am abstract the base ActionResult class like this: public abstract class MyActionResult : ActionResult, IEnsureNotNull
{
public abstract void EnsureAllInjectInstanceNotNull();
}
public abstract class ActionResultBase<TController> : MyActionResult where TController : Controller
{
protected readonly Expression<Func<TController, ActionResult>> ViewNameExpression;
protected readonly IExConfigurationManager ConfigurationManager;
protected ActionResultBase (Expression<Func<TController, ActionResult>> expr)
: this(DependencyResolver.Current.GetService<IExConfigurationManager>(), expr)
{
}
protected ActionResultBase(
IExConfigurationManager configurationManager,
Expression<Func<TController, ActionResult>> expr)
{
Guard.ArgumentNotNull(expr, "ViewNameExpression");
Guard.ArgumentNotNull(configurationManager, "ConfigurationManager");
ViewNameExpression = expr;
ConfigurationManager = configurationManager;
}
protected ViewResult GetViewResult<TViewModel>(TViewModel viewModel)
{
var m = (MethodCallExpression)ViewNameExpression.Body;
if (m.Method.ReturnType != typeof(ActionResult))
{
throw new ArgumentException("ControllerAction method '" + m.Method.Name + "' does not return type ActionResult");
}
var result = new ViewResult
{
ViewName = m.Method.Name
};
result.ViewData.Model = viewModel;
return result;
}
public override void ExecuteResult(ControllerContext context)
{
EnsureAllInjectInstanceNotNull();
}
}
I also have an interface for validation all inject objects. This interface make sure all inject objects that I inject using Autofac container are not null. The implementation of this as below public interface IEnsureNotNull
{
void EnsureAllInjectInstanceNotNull();
}
Afterwards, I am just simple implementing the HomePageViewModelActionResult class like this public class HomePageViewModelActionResult<TController> : ActionResultBase<TController> where TController : Controller
{
#region variables & ctors
private readonly ICategoryRepository _categoryRepository;
private readonly IItemRepository _itemRepository;
private readonly int _numOfPage;
public HomePageViewModelActionResult(Expression<Func<TController, ActionResult>> viewNameExpression)
: this(viewNameExpression,
DependencyResolver.Current.GetService<ICategoryRepository>(),
DependencyResolver.Current.GetService<IItemRepository>())
{
}
public HomePageViewModelActionResult(
Expression<Func<TController, ActionResult>> viewNameExpression,
ICategoryRepository categoryRepository,
IItemRepository itemRepository)
: base(viewNameExpression)
{
_categoryRepository = categoryRepository;
_itemRepository = itemRepository;
_numOfPage = ConfigurationManager.GetAppConfigBy("NumOfPage").ToInteger();
}
#endregion
#region implementation
public override void ExecuteResult(ControllerContext context)
{
base.ExecuteResult(context);
var cats = _categoryRepository.GetCategories();
var mainViewModel = new HomePageViewModel();
var headerViewModel = new HeaderViewModel();
var footerViewModel = new FooterViewModel();
var mainPageViewModel = new MainPageViewModel();
headerViewModel.SiteTitle = "Magazine Website";
if (cats != null && cats.Any())
{
headerViewModel.Categories = cats.ToList();
footerViewModel.Categories = cats.ToList();
}
mainPageViewModel.LeftColumn = BindingDataForMainPageLeftColumnViewModel();
mainPageViewModel.RightColumn = BindingDataForMainPageRightColumnViewModel();
mainViewModel.Header = headerViewModel;
mainViewModel.DashBoard = new DashboardViewModel();
mainViewModel.Footer = footerViewModel;
mainViewModel.MainPage = mainPageViewModel;
GetViewResult(mainViewModel).ExecuteResult(context);
}
public override void EnsureAllInjectInstanceNotNull()
{
Guard.ArgumentNotNull(_categoryRepository, "CategoryRepository");
Guard.ArgumentNotNull(_itemRepository, "ItemRepository");
Guard.ArgumentMustMoreThanZero(_numOfPage, "NumOfPage");
}
#endregion
#region private functions
private MainPageRightColumnViewModel BindingDataForMainPageRightColumnViewModel()
{
var mainPageRightCol = new MainPageRightColumnViewModel();
mainPageRightCol.LatestNews = _itemRepository.GetNewestItem(_numOfPage).ToList();
mainPageRightCol.MostViews = _itemRepository.GetMostViews(_numOfPage).ToList();
return mainPageRightCol;
}
private MainPageLeftColumnViewModel BindingDataForMainPageLeftColumnViewModel()
{
var mainPageLeftCol = new MainPageLeftColumnViewModel();
var items = _itemRepository.GetNewestItem(_numOfPage);
if (items != null && items.Any())
{
var firstItem = items.First();
if (firstItem == null)
throw new NoNullAllowedException("First Item".ToNotNullErrorMessage());
if (firstItem.ItemContent == null)
throw new NoNullAllowedException("First ItemContent".ToNotNullErrorMessage());
mainPageLeftCol.FirstItem = firstItem;
if (items.Count() > 1)
{
mainPageLeftCol.RemainItems = items.Where(x => x.ItemContent != null && x.Id != mainPageLeftCol.FirstItem.Id).ToList();
}
}
return mainPageLeftCol;
}
#endregion
}
Final step, I get into HomeController and add some line of codes like this [Authorize]
public class HomeController : BaseController
{
[AllowAnonymous]
public ActionResult Index()
{
return new HomePageViewModelActionResult<HomeController>(x=>x.Index());
}
[AllowAnonymous]
public ActionResult Details(int id)
{
return new DetailsViewModelActionResult<HomeController>(x => x.Details(id), id);
}
[AllowAnonymous]
public ActionResult Category(int id)
{
return new CategoryViewModelActionResult<HomeController>(x => x.Category(id), id);
}
}
As you see, the code in controller is really skinny, and all the logic I move to the custom ActionResult class. Some people said, it just move the code out of controller and put it to another class, so it is still hard to maintain. Look like it just move the complicate codes from one place to another place. But if you have a look and think it in details, you have to find out if you have code for processing all logic that related to HttpContext or something like this. You can do it on Controller, and try to delegating another logic (such as processing business requirement, persistence data,...) to custom ActionResult class.
Tell me more your thinking, I am really willing to hear all of its from you guys.
All source codes can be find out at here.
Tweet
!function(d,s,id){var js,fjs=d.getElementsByTagName(s)[0];if(!d.getElementById(id)){js=d.createElement(s);js.id=id;js.src="http://weblogs.asp.net//platform.twitter.com/widgets.js";fjs.parentNode.insertBefore(js,fjs);}}(document,"script","twitter-wjs");
