# Custom HtmlHelper to render a Grid

I have created a custom HtmlHelper to render a Grid, and it's working fine, but I know for sure that it isn't written really nicely.

I'll provide you with the code that's most important for the grid.

First of all, the model which I pass to my grid is an IEnumerable<T>.

Then, the Razor syntax for the grid is as follows:

@(Html.GridFor()
.Name("PageOverviewGrid")
.WithColumns(model =>
{
model.Bind(x => x.Name);
model.Bind(x => x.DateCreated);
model.Bind(x => x.DateUpdated);
})
)


Then I do have a custom HtmlHelper to render this grid:

 public static IGridBuilder<TModel> GridFor<TModel>(this HtmlHelper<IEnumerable<TModel>> htmlHelper)
{
return new GridBuilder<TModel>(htmlHelper);
}


Then the GridBuilder class itself (it's constructed with interfaces so I can use a fluent API):

public class GridBuilder<TModel> : IGridBuilder<TModel>
{
#region Constructors

/// <summary>
/// Creates a new instance of the <see cref="GridBuilder{TModel}"/>.
/// </summary>
/// <param name="htmlHelper">The <see cref="HtmlHelper{TModel}"/> that is used to render this one.</param>
public GridBuilder(HtmlHelper<IEnumerable<TModel>> htmlHelper)
{
this.htmlHelper = htmlHelper;
this.properties = new Dictionary<string, string>();
}

#endregion

#region Properties

public Dictionary<string, string> properties;

#endregion

#region IGridBuilder Members

/// <summary>
/// Gets the name of the <see cref="IGridBuilder{TModel}"/>.
/// </summary>
public string name { get; private set; }

/// <summary>
/// The <see cref="HtmlHelper"/> that is used to build the grid.
/// </summary>
public HtmlHelper htmlHelper { get; private set; }

/// <summary>
/// Sets the name of the <see cref="IGridBuilder{TModel}"/>.
/// </summary>
/// <param name="name">The name that the <see cref="IGridBuilder{TModel}"/> should have.</param>
/// <returns>An <see cref="IGridBuilder{TModel}"/> that is used to construct the grid.</returns>
public IGridBuilder<TModel> Name(string name)
{
this.name = name;

return this;
}

/// <summary>
/// Binds an column to the grid.
/// </summary>
/// <typeparam name="TItem">The type of the column on which to bind the items.</typeparam>
/// <param name="function">The functional that will bind the control to the grid.</param>
public void Bind<TItem>(Expression<Func<TModel, TItem>> function)
{

}

/// <summary>
/// Set the columns of the model that should be bound to grid.
/// </summary>
/// <param name="action">The action that will bind all the columns.</param>
/// <returns>An <see cref="IGridBuilder{TModel}"/> that is used to construct the grid.</returns>
public IGridBuilder<TModel> WithColumns(Action<IGridBuilder<TModel>> action)
{
action.Invoke(this);
return this;
}

#endregion

#region IHtmlString Members

/// <summary>
/// Returns an HTML-encoded string.
/// </summary>
/// <returns>Returns an HTML-encoded string.</returns>
public string ToHtmlString()
{
// Additional rendering is done here.
}
}


And then now the things about which I'm concerned:

Retrieving the property display name or property name. (It's done through reflection right now, but I'm thinking that there is more friendly solution).

How can I render the data of the entity. For example, I want to have the DateCreated of a single entity in the grid.

foreach (var entity in htmlHelper.ViewData.Model as IEnumerable<GridPageFolderViewModel>)
{
// For the entity get the properties and look in the directory of properties if it should be displayed. If that's the case, render it.
}


I have the feeling that this could be optimized, but it's my very first HTML extension. I would appreciate any help.

– Jamal
Dec 30 '14 at 4:09

I like the fluent API. Nitpicks first:

• Drop the #region blocks. They only add clutter, for example #region IHtmlString Members is only confusing, since it's not immediately apparent that GridBuilder implements IHtmlString (perhaps IGridBuilder does? I'm not super familiar with these extensions myself, but I don't think it matters - comments shouldn't be misleading or confusing, period). Also #region Properties is a big fat lie; I'll get back to that.
• Vertical spacing isn't consistent. I prefer this:

public IGridBuilder<TModel> WithColumns(Action<IGridBuilder<TModel>> action)
{
action.Invoke(this);
return this;
}


Over that:

public IGridBuilder<TModel> Name(string name)
{
this.name = name;

return this;
}


Now the more important stuff.

#region Properties

public Dictionary<string, string> properties;

#endregion


This isn't a property, the comment describing the region is very, very misleading. This is a publicly exposed field - client code is completely free to reassign the Dictionary<string, string> reference to whatever it wants. Exposing public fields breaks encapsulations. Expose properties, not fields.

Your naming conventions are confusing, inconsistent and error-prone. All public members should be PascalCase - keep camelCase for local variables and private fields; I'd also recommend prefixing private fields with an _ underscore (still _camelCase for private fields) so that you can get rid of the this qualifier, and only use it when it's actually needed - i.e. when you're returning this, or passing this as a parameter to a method/delegate.

public string name { get; private set; }
public IGridBuilder<TModel> Name(string name)


Would be:

private string _name;
public string Name { get { return _name; } }
public IGridBuilder<TModel> WithName(string name);


Notice how none of these names are ambiguous in any way.

The parameter naming could be more meaningful here:

public void Bind<TItem>(Expression<Func<TModel, TItem>> function)
public IGridBuilder<TModel> WithColumns(Action<IGridBuilder<TModel>> action)


These parameters are closer to being methods, and naming them as such (still camelCase though) would be more helpful and wouldn't require relying on the XML documentation / IntelliSense to know what that function/action is supposed to be doing:

public void Bind<TItem>(Expression<Func<TModel, TItem>> propertySelector)
public IGridBuilder<TModel> WithColumns(Action<IGridBuilder<TModel>> bindAllColumns)


I'm recommending such naming, because this:

action.Invoke(this);


Could then also be written like this:

bindAllColumns(this);


..which is much more meaningful.

Lastly, this line may have a minor bug:

properties.Add(metadata.PropertyName, metadata.DisplayName ?? metadata.PropertyName);


DisplayName being a string, the null-coalescing operator isn't ideal. I'd split it in two instructions:

var displayName = string.IsNullOrEmpty(metadata.DisplayName)