The Filter-ViewData anti-pattern

In just about every website you go to these days, its layout follows a very similar pattern:

You have some static resource logo, a large main section with information that our controller action supplies.  But we also have some other stuff, those green sections.  Things like breadcrumbs, shopping carts, login widgets, smart menus and so on.  It’s all still data driven, but its information has absolutely nothing to do with what’s going on in my main section.  This is some of the first kinds of duplication we see in MVC applications – common, data-driven sections across many pages in our application.

The usual way to get rid of this duplication is through ActionFilters.  We decorate our main section action with filters:

public class ProductController : Controller
{
    [LoginInfo]
    public ActionResult Index()
    {
        var products = _productRepository.FindAll();

        return View(products);
    }

We don’t have that duplicated login information code in all of our controllers, and we can even go so far as to decorating our controller class or layer supertype to apply our filter to a broad set of controllers.  But looking at our filter, it looks like we took a step backwards:

public class LoginInfoAttribute : ActionFilterAttribute
{
    public override void OnActionExecuted(ActionExecutedContext filterContext)
    {
        var currentUser = _userSession.GetCurrentUser();

        filterContext.Controller.ViewData.Add("currentUser", currentUser);
    }

Back again to magic strings!  With MVC Contrib, it looks a little better as we can skip the key part, and just pass the current user object in.  But beyond the magic strings issue here is the problem of linking what is essentially a view concern (how I organize the different pieces of information on a given page) with my controller structure.  What if that widget isn’t shown on all actions for a controller?  What if I have four widgets, and not all are shown on every screen?  It begins to break down the normal, filters-on-layersupertypes-pattern.  I’d rather not let my view structure, with master pages and what not, influence my main-line controller design in any way.

The other issue I run into is figuring out where the heck the information comes from if I’m looking at a view.  I see a piece being pulled out of ViewData, but not any clear indication of where that information came from:

<% Html.RenderPartial("loginView", ViewData["currentUser"]); %>

    <h2>View Products</h2>

Yes, we can put this in a master page.  But now we’ve coupled the requirements of our view structure with the structure of our controllers, not always a good thing.  In my (not-so) humble opinion, an action should be responsible for generating exactly one model.  Things like action filters fiddling with view data are three layers of indirection to get around this rule.  But in the MVC Futures assembly, we have a different option.

Tangential concerns with RenderAction

In our original view, if we dissected each piece, we had a main section and other, orthogonal sections.  The main section’s information is handled by our main action handling the request.  The other pieces are needed, but those concerns aren’t really related to the main section.  Does showing a login widget have anything to do with showing a product list?  No.  But I don’t want to couple all the different concerns in completed view with the main action.  Because those widgets, and where they’re located is a view concern, we can initiate the rendering of those pieces from the view using RenderAction.

RenderAction works like a mini-request, exercising the MvcHandler, but re-using the existing HttpContext.  It simulates a request, but writes out to the exact same response stream, at the exact right place:

Our view doesn’t look much different, it looks like a cross between a Url.Action call and RenderPartial:

<% Html.RenderAction<LoginController>(c => c.Widget()); %>

    <h2>View Products</h2>

What you don’t see is any data or model passed to RenderAction.  The LoginController is a plain controller, with a regular action on it.  The action now can be concerned with both finding the data for the model and choosing the view to render:

public class LoginController : Controller
{
    public ViewResult Widget()
    {
        var currentUser = _userSession.GetCurrentUser();

        return View(currentUser);
    }

I don’t have to jump through hoops to find where my information came from, as I only need to follow the link from the RenderAction call to the Widget action method.  Additionally, my original ProductController now has absolutely zero logic for all of the orthogonal concerns, whether it’s hidden in an action filter or not.  For each view, I can decide what master page to use, and place those RenderAction calls there.  I let my view decide what layout is correct.

Wherever I see action filters poking around with ViewData, I see a very clear path to using RenderAction instead.  It’s a much more cohesive and flexible model, and straightforward to understand where each individual piece comes from.  Controllers have enough responsibility as it is, why do they need to be burdened with the additional responsibility of organizing data for tangential concerns?  RenderAction has zero performance penalty, as it’s not a real, external request.  ViewData can be abused as a bag-o’-stuff for views to consume, but that won’t lead to maintainable, understandable markup.  Instead, we can utilize the RenderAction method and create very cohesive controllers and strongly-typed views, with explicit concerns.