Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have developed a system whereby I use an attribute to state which SelectList to use for an 'FK' property. I would appreciate some feedback mainly on by good practice, and also on any ways to do this better. Let me start my exposition with a demo view model:

public class DemoModel
{
    [DropDownList("LanguageSelect")]
    public int? LanguageId { get; set; }
    public SelectList LanguageSelect { get; set; }
}

Now when I use the Razor markup @EditorFor(m => m.LanguageId), I get a drop-down populated from the LanguageSelect list. I get this because the DropDownListAttrbute class attaches the select list name to the LanguageId model:

public class DropDownListAttribute : UIHintAttribute, IMetadataAware
{
    public DropDownListAttribute(string selectListName) : base(KnownUiHints.DropDown, KnownPresentationLayers.Mvc, selectListName)
    {
        SelectListName = selectListName;
    }
    public string SelectListName { get; set; }
    public void OnMetadataCreated(ModelMetadata metadata)
    {
        var listProp = metadata.ContainerType.GetProperty(SelectListName);
        metadata.AdditionalValues[KnowMetadataKeys.SelectListName] = SelectListName;
    }
}

All my view models derive from ViewModel, which offers a SelectListDictionary property:

private IDictionary<string, SelectList> _selectListdictionary;
public virtual IDictionary<string, SelectList> SelectListDictionary
{
    get
    {
        if (_selectListdictionary == null)
        {
            var props = GetType().GetProperties().Where(p => p.PropertyType == typeof(SelectList));
            _selectListdictionary = props.ToDictionary(prop => prop.Name, prop => (SelectList)prop.GetValue(this, null));
        }
        return _selectListdictionary;
    }
}

In my base controller, I override the View method to pull the entire select list dictionary from the view model, and insert it into the view's viewdata, making it available for the editor template:

protected override ViewResult View(string viewName, string masterName, object model)
{
    var result = base.View(viewName, masterName, model);
    if ((model is ViewModel) && (!ViewData.ContainsKey(KnowMetadataKeys.ViewDataSelectLists)))
    {
        var vm = (ViewModel)model;
        result.ViewData.Add(KnowMetadataKeys.ViewDataSelectLists, vm.SelectListDictionary);
    }
    return result;
}

The editor template then retrieves the list it requires from the select list dictionary and builds a select input element:

@using Erisia.Constants
@{
    var list = (SelectList)ViewData.ModelMetadata.AdditionalValues[ViewData.ModelMetadata.AdditionalValues[KnowMetadataKeys.SelectListName].ToString()];
    var listWithSelected = new SelectList(list.Items, list.DataValueField, list.DataTextField, Model);
}
@Html.DropDownListFor(m => Model, listWithSelected, " - select - ")

So what say you all?

share|improve this question
add comment

1 Answer

I'm assuming this code hasn't been reviewed yet because it's... beautiful - and I'm still searching for a better word. I mean, wow that's clever, I want that!!

The only thing I can see here, is in the View method:

protected override ViewResult View(string viewName, string masterName, object model)
{
    var result = base.View(viewName, masterName, model);
    if ((model is ViewModel) && (!ViewData.ContainsKey(KnowMetadataKeys.ViewDataSelectLists)))
    {
        var vm = (ViewModel)model;
        result.ViewData.Add(KnowMetadataKeys.ViewDataSelectLists, vm.SelectListDictionary);
    }
    return result;
}

I would suggest a safe cast instead of model is ViewModel followed by a cast, so it would look more like this - note that I dropped a few redundant parentheses:

protected override ViewResult View(string viewName, string masterName, object model)
{
    var result = base.View(viewName, masterName, model);
    var vm = model as ViewModel;
    if (vm != null && !ViewData.ContainsKey(KnowMetadataKeys.ViewDataSelectLists))
    {
        result.ViewData.Add(KnowMetadataKeys.ViewDataSelectLists, vm.SelectListDictionary);
    }
    return result;
}

Other than that, I agree with your naming (except maybe vm could be called viewModel, but vm is pretty descriptive in this specific context) and naming conventions, and there's nothing much left to say as far as I'm concerned - good job! :)

share|improve this answer
    
Thank you very much! –  ProfK Dec 20 '13 at 6:04
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.