Tuesday, October 19, 2010

Populating Drop Down Lists in ASP.NET MVC

Here’s the shopping basket in Suteki Shop:

suteki_shop_basket

As you can see, there’s a drop-down-list of countries. The standard way of populating a drop-down-list is to pass the collection of countries as part of the view model from the controller action. This can get very tedious when you have multiple drop-downs on a form, the controller action soon gets bloated with data access for all that lookup data. It also violates the thunderdome principle: one model in, one model out.

This lookup data is not something that the controller should really have to care about. I would much rather the data just appeared in the drop-down by convention. In the case above, the basket has a property of type Country, so why couldn’t convention just say: if you want to set the country then you need a list of all the countries?

With this in mind, I’ve put together an HtmlHelper extension method, ComboFor which can create a drop-down list for any entity type that implements my INamedEntity interface, so now I just have to write:

<%= Html.ComboFor(x => x.Country) %>

Which produces this HTML:

<select id="Country_Id" name="Country.Id">
    <option selected="selected" value="1">United Kingdom</option> 
    <option value="2">France</option> 
</select>

The extension method itself looks like this:

public static string ComboFor<TModel, TLookup>(this HtmlHelper<TModel> htmlHelper, Expression<Func<TModel, TLookup>> propertyExpression)
    where TLookup : INamedEntity
{
    return htmlHelper.With<IComboFor<TLookup, TModel>, TModel>(combo => combo.BoundTo(propertyExpression));
}

My ‘With’ extension method looks up services in the IoC container so that I can do dependency injection in HtmlHelper extension methods.

The implementation of IComboFor looks like this:

public class ComboFor<TEntity, TModel> : IComboFor<TEntity, TModel>, IRequireHtmlHelper<TModel> where TEntity : class, INamedEntity
{
    readonly IRepository<TEntity> repository;
    protected Expression<Func<TEntity, bool>> WhereClause { get; set; }
    protected string PropertyNamePrefix { get; set; }

    public ComboFor(IRepository<TEntity> repository)
    {
        this.repository = repository;
    }
    public string BoundTo(Expression<Func<TModel, TEntity>> propertyExpression, Expression<Func<TEntity, bool>> whereClause, string propertyNamePrefix)
    {
        WhereClause = whereClause;
        PropertyNamePrefix = propertyNamePrefix;
        return BoundTo(propertyExpression);
    }

    public string BoundTo(Expression<Func<TModel, TEntity>> propertyExpression, Expression<Func<TEntity, bool>> whereClause)
    {
        WhereClause = whereClause;
        return BoundTo(propertyExpression);
    }

    public string BoundTo(Expression<Func<TModel, TEntity>> propertyExpression, string propertyNamePrefix)
    {
        PropertyNamePrefix = propertyNamePrefix;
        return BoundTo(propertyExpression);
    }

    public string BoundTo(Expression<Func<TModel, TEntity>> propertyExpression)
    {
        var getPropertyValue = propertyExpression.Compile();
        var propertyName = (!String.IsNullOrEmpty(PropertyNamePrefix) ? PropertyNamePrefix : "")
            + Utils.ExpressionHelper.GetDottedPropertyNameFromExpression(propertyExpression) + ".Id";

        var viewDataModelIsNull = (!typeof(TModel).IsValueType) && HtmlHelper.ViewData.Model == null;
        var selectedId = viewDataModelIsNull ? 0 : getPropertyValue(HtmlHelper.ViewData.Model).Id;
        return BuildCombo(selectedId, propertyName);
    }


    public override string ToString()
    {
        return BuildCombo(0, typeof(TEntity).Name + "Id");
    }

    protected virtual string BuildCombo(int selectedId, string htmlId)
    {
        if (string.IsNullOrEmpty(htmlId))
        {
            throw new ArgumentException("htmlId can not be null or empty");
        }

        if (HtmlHelper == null)
        {
            throw new SutekiCommonException("HtmlHelper is null");
        }
        return HtmlHelper.DropDownList(htmlId, GetSelectListItems(selectedId)).ToString();
    }

    protected IEnumerable<SelectListItem> GetSelectListItems(int selectedId)
    {
        var queryable = repository.GetAll();
        if (WhereClause != null)
        {
            queryable = queryable.Where(WhereClause);
        }
        var enumerable = queryable.AsEnumerable();

        if (typeof(TEntity).IsOrderable())
        {
            enumerable = enumerable.Select(x => (IOrderable)x).InOrder().Select(x => (TEntity)x);
        }
        
        if (typeof(TEntity).IsActivatable())
        {
            enumerable = enumerable.Select(x => (IActivatable)x).Where(a => a.IsActive).Select(x => (TEntity)x);
        }
        
        var items = enumerable
            .Select(e => new SelectListItem { Selected = e.Id == selectedId, Text = e.Name, Value = e.Id.ToString() });

        return items;
    }

    public HtmlHelper<TModel> HtmlHelper { get; set; }
}

The interesting bit is the ‘GetSelectListItems’ method which gets the select list items from the repository. You have a chance here to provide a Linq where clause to filter the items. It also understands my IOrderable and IActivatable interfaces, so any entity that implements IOrderable will be shown in the correct order in the drop down, and any that implement IActivatable will only appear if they are active.

All this works because of conventions and polymorphism. Now I have much simpler controller actions that usually have a model as the argument and return a model in the ViewResult. Nice!

12 comments:

GeeBee said...

Not sure I like this - you now have your views calling into the data layer, feels like a bit of a violation of the MVC pattern.
Assuming that you have a controller for creating, deleting etc. the deliverable countries with their shipping charges, maybe you could just use the index action on that controller instead via the RenderAction method.

Mike Hadlow said...

Hi GeeBee,

Isn't RenderAction effectively calling into the data layer via the controller? The only difference being that I'd have to write the same repetitive code out for each drop down.

The MVC pattern is not an end in itself, but just a way to separate concerns. I have some infrastructure that calls into the data layer (the ComboFor) but the view has no dependency on that.

GeeBee said...

true - however you now have two entry points for the same repository, one via the ComboFor, and the other via a controller action. I think I just feel more comfortable with the idea that there is one very defined entry path to the repositories (via a controller)

Mike Hadlow said...

Hmm, but why be so rigid? Here's a simple generic solution to repetitive boilerplate code. Now, you might not like my particular implementation, which is fair enough, but is it right to reject optimisations because you've set some architectural principal in stone?

Marco Paul said...

So when/where does the combofor class get created? is it a singleton?

one way i addressed this on a project was to decorate my view model properties with a lookup attribute, so when the action got executed, the property got filled with items accordingly.

Mike Hadlow said...

Hi Marco Paul,

The ComboFor class is resolved from the IoC container via my 'With' HtmlHelper extension method.

Joshua Lewis said...

I think I agree with GeeBee.
I would prefer calling a RenderAction. If nothing else, only because it 'feels' more 'explicit'.

It is quite an elegant solution though.

Typically in systems I deal with, the Controller wouldn't be responsible for data access, it would request a complete model (a la Thunderdome principle) from an orchestration or coordination layer, which has the responsibility of coordinating data access and building up the model.

The idea is also that the orchestration layer exposes operations which support 'business transaction' granularity - i.e. the same layer could theoretically be interogated by a web service wrapper.

Another shortcoming of the RenderAction approach is that you might want to display your model in different ways in different views. (This may be a conceptual violation of MVC in that each model should have one corresponding view).

I also suspect your approach is a violation of the Thunderdome Principle in that your view is presenting data that is not (explicitly) part of your model.

Mike Hadlow said...

Hi Josh,

That's a good approach. I usually start by calling repositories directly from the controller action, but as soon as it starts being more that a simple retrieve or update, I refactor it into a service. Once again, I think pragmatism is the way to go here. If all you are doing is getting a single entity from a repository, there's no need for a further layer.

As I understand it from JM's original post the Thunderdome Principle is about controllers. At some point you need to aggregate all your representations into a single page to present to the user. Unless you are doing AJAX, this will happen in the view.

Unknown said...

I may be late to the party on this but...

GeeBee, I think I understand why you have an uneasy feeling; You quite rightly don't want to call directly to your data layer, as it compromises your architecture but I think your uneasiness is misplaced.

I would be less inclined to worry about not going through your Controller as I think you may be reading more into the purpose of the Controller than is actually intended. The Controller is there to handle actions in the main. If you want such consistency (and the end game here is ultimately abstraction) then whats missing is a service between the presentation layer as a whole (MVC) and the repository (data layer).
However, I think you are missing the point of Mike's post here. The value in what Mike is proposing is the power of HTML helpers. This is similar to the criticism of the NerdDinners app, where the point that is being illustrated is being missed as the example is not a full-blown architecture.

I did something similar in my previous project Mike, although no where near as elegant. At the time, I was surprised there wasn't a single pattern/way of doing this and I just wish your post had been here at that time. Good work Mike.

Mike Hadlow said...

Hi Andy,

Good points. I think it's too easy to let a pattern become a regulation. Not putting logic in your views is good advice, but as you pointed out, that doesn't mean that the _only_ place for application logic is the controller. It's worth remembering that the MVC pattern is not an end in itself, but rather a mechanism you can use to help you separate concerns.

Dimitris Foukas said...

Hi Mike,

I agree that the MVC pattern is not an end in itself but we have to draw a line for the pattern to work. What if you nee cascading lookups with dependencies? If you need to have multiple lookups use in an XOR manner etc. Will you put all this logic into helpers? Don;t think so...

Perhaps we can look at InfoPath for inspiration: there is a concept of main data source which is the target of form submissions but also additional, auxiliary data sources with declarative dependencies.

My 2cs

Mike Pollitt said...

Hi Mike --

I arrived at this post after much Googling, and was amazed to find it authored by THE Mike Hadlow of yore. Satisfying to find such great company in hitting this logical dilemma. Here's the dilemma:

"Is the display of a populated combo box the responsibility of the Controller or the View?"

If I was to replace "populated combo box" with "date picker", the answer would be obvious: the View. The Controller merely cares about the date, not the method of data entry for a date. Thus it's not such a huge leap to treat any common and fairly static (or at least exegetic) drop-down (countries, languages, currencies, etc.) as simply another type of picker control.

In past projects, for this reason, I've been happy to load these in the View itself. But it has always made me feel slightly dirty. I like your solution much better.

Cheers,
Mike.