How Could I Improve the Design of this HtmlHelper Extension?

Question

Hi Folks

note: I had originally posted a question similar to this here, but I decided to repost because I overcame the original issue and in the process, modified the design. I thought it warranted a new topic because as the design changed, the question fundamentally changed also. I just want to make it clear that I'm not trying to flood SO with the same question.

question: I've been working on a little experiement to see if I could create a helper method to serialize any of my types to any type of HTML tag I specify. I thought I'd put it to the community to help me identify code-smells or other flaws/inefficiencies in the design in order to improve the design.

Basically, I have this code that will generate a Select box (or any other Html element) with a number of options:

// the idea is I can use one method to create any complete tag of any type 
// and put whatever I want in the content area. 
// This makes the generation of all html completely testable

<% using (Html.GenerateTag<SelectTag>(Model, new { href = Url.Action("ActionName") })) { %>
     // model is type ShareClass. It contains a list of Funds
    <%foreach (var fund in Model.Funds) {%>
        <% using (Html.GenerateTag<OptionTag>(fund)) { %>
            <%= fund.Name %>
        <% } %>
    <% } %>
<% } %>

which will produce the following html output:

<select shareclassname="MyShareClass" 
        shareclasstype="ShareClass_A" 
        href="/Ctrlr/ActionName">
    <option selected="selected" id="1" name="MyFund_1">MyFund_1</option>
    <option id="2" name="MyFund_2">MyFund_2</option>
    <option id="3" name="MyFund_3">MyFund_3</option>
    <option id="N" name="MyFund_N">MyFund_N</option>
</select>

This Html.GenerateTag helper is defined as:

public static MMTag GenerateTag<T>(this HtmlHelper htmlHelper, object elementData, object attributes) where T : MMTag
{
    return (T)Activator.CreateInstance(typeof(T), htmlHelper.ViewContext, elementData, attributes);
}

Depending on the type of T it'll create one of the types defined below:

public abstract class HtmlTypeBase : MMTag
{        
    public HtmlTypeBase(ViewContext viewContext, params object[] elementData)
    {
        _tag = this.GetTag();
        base._viewContext = viewContext;
        base.MergeDataToTag(viewContext, elementData);
    }

    public abstract TagBuilder GetTag();
}

public class SelectTag : HtmlTypeBase
{
    public SelectTag(ViewContext viewContext, params object[] elementData) 
        : base(viewContext, elementData)
    {
        base._tag = new TagBuilder("select");
    }

    public override TagBuilder GetTag()
    {
        return new TagBuilder("select");
    }
}

public class OptionTag : HtmlTypeBase
{
    public OptionTag(ViewContext viewContext, params object[] elementData)
        : base(viewContext, elementData)
    {
        base._tag = new TagBuilder("option");
    }

    public override TagBuilder GetTag()
    {
        return new TagBuilder("option");
    }
}

public class AnchorTag : HtmlTypeBase
{
    public AnchorTag(ViewContext viewContext, params object[] elementData)
        : base(viewContext, elementData)
    {
        base._tag = new TagBuilder("a");
    }

    public override TagBuilder GetTag()
    {
        return new TagBuilder("a");
    }
}

and this is the definition of MMTag:

public class MMTag : IDisposable
{
    internal bool _disposed;
    internal ViewContext _viewContext;
    internal TextWriter _writer;
    internal TagBuilder _tag;

    public MMTag() {}

    public MMTag(ViewContext viewContext, params object[] elementData){ }

    protected void MergeDataToTag(ViewContext viewContext, object[] elementData)
    {
        MergeTypeDataToTag(elementData[0]);
        MergeAttributeDataToTag(elementData[1]);

        this._viewContext = viewContext;

        this._viewContext.Writer.Write(_tag.ToString(TagRenderMode.StartTag));
    }

    private void MergeAttributeDataToTag(object attributeData)
    {
        var dic = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
        var attributes = attributeData;
        if (attributes != null)
        {
            foreach (PropertyDescriptor descriptor in TypeDescriptor.GetProperties(attributes))
            {
                object value = descriptor.GetValue(attributes);
                dic.Add(descriptor.Name, value);
            }
        }

        _tag.MergeAttributes<string, object>(dic);
    }

    private void MergeTypeDataToTag(object typeData)
    {
        Type elementDataType = typeData.GetType();
        foreach (PropertyInfo prop in elementDataType.GetProperties())
        {
            if (prop.PropertyType.IsPrimitive || prop.PropertyType == typeof(Decimal) || prop.PropertyType == typeof(String))
            {
                object propValue = prop.GetValue(typeData, null);
                string stringValue = propValue != null ? propValue.ToString() : String.Empty;
                _tag.Attributes.Add(prop.Name, stringValue);
            }
        }
    }

    #region IDisposable 
    public void Dispose()
    {
        Dispose(true /* disposing */);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            _disposed = true;
            if (disposing)
            {
                _writer = _viewContext.Writer;
                _writer.Write(_tag.ToString(TagRenderMode.EndTag));
            }
        }
    }
    #endregion
}

I should also add that Fund & ShareClass are defined as:

public class Fund
{
    public int ID { get; set; }
    public string Name { get; set; }

    public Fund()
    {
        this.ID = 123;
        this.Name = "MyFund";
    }

    public Fund(int id, string name)
    {
        this.ID = id;
        this.Name = name;
    }
}

public class ShareClass
{
    public string ShareClassName { get; set; }
    public string ShareClassType { get; set; }
    public IEnumerable<Fund> Funds { get; set; }

    public ShareClass(string name, string shareClassType)
    {
        this.ShareClassName = name;
        this.ShareClassType = shareClassType;
    }
}

Answer 1

Have you considered using convention to create your tags? It looks like you are requiring a lot of repetitive code in your views. Each time you want a drop down you'd have to copy half a dozen lines.

Using opinionated input builders would drastically simplify your view, but require a bit of setup on your part. In my experience, this set up is WELL WORTH the time you save down the road!

The idea of an input builder is that you specify that there is an input element (or display or label, etc) for a property on your view model. Your input builder framework then inspects the property for it's type and attributes and determines the proper type of input to be rendered.

Here is an example of how a drop down is built in my current project:

//View Model
[OptionsFrom("Years")]
public int ContractYear{ get; set; }

public IDictionary Years
{
    get
    {
        var currentYear = DateTime.Today.Year;
        return Enumerable.Range(0, 10).ToDictionary(i => currentYear + i, i => (currentYear + i).ToString());
    }
}

//View
Html.InputFor(x => x.ContractYear);

Here is a walk-through on using MVCContrib's input builders. I know there is also some input builder support in MVC2, but I am not familiar with it. In my opinion the best convention based builders are available from FubuMVC. I have a post here about how to use them with ASP.NET MVC. I don't yet have any posts about how to configure them, although I am planning on it soon.