ASP.Net MVC helps with refactoring

Question

I am new to both ASP.Net and MVC.

On my main page, I got the following code:

<div id="main-menu" class="menu"> <% var items = (IList<CompanyName.Framework.Web.MenuItem>)ViewData["MainMenu"]; if (items.Count > 0) { %><ul><% foreach (var item in items) { if (!string.IsNullOrEmpty(item.RequiredRole) && !System.Threading.Thread.CurrentPrincipal.IsInRole(item.RequiredRole)) continue; %><li><a href="<%= item.Uri %>"><%= item.Title %></a></li><% } %></ul><% } %> </div> 

Can I move the code to another file or process the code in any way?

change

My ApplicationController, which is produced by all controllers:

 public class ApplicationController : Controller { List<MenuItem> _mainMenu = new List<MenuItem>(); List<MenuItem> _contextMenu = new List<MenuItem>(); protected IList<MenuItem> MainMenu { get { return _mainMenu; } } protected IList<MenuItem> ContextMenu { get { return _contextMenu; } } protected string PageTitle { get; set; } protected override void OnResultExecuting(ResultExecutingContext filterContext) { ViewData["PageTitle"] = PageTitle; ViewData["MainMenu"] = MainMenu; ViewData["ContextMenu"] = ContextMenu; base.OnResultExecuting(filterContext); } } 

Answer 1

Here are some suggestions:

Improvement number 1 : use view models and strongly typed representations instead of ViewData p>

 public ActionResult Index() { // TODO: Fetch this data from a repository var menus = new[] { new MenuItem(), new MenuItem() }.ToList(); return View(menus); } 

and then, in your opinion:

 <div id="main-menu" class="menu"> <% if (Model.Count > 0) { %><ul><% foreach (var item in Model) { if (!string.IsNullOrEmpty(item.RequiredRole) && !System.Threading.Thread.CurrentPrincipal.IsInRole(item.RequiredRole)) continue; %><li><a href="<%= item.Uri %>"><%= item.Title %></a></li><% } %></ul><% } %> </div> 

Still a terrible and completely unreadable soup tag.

---

Improvement number 2 : use editor / display templates:

In ~/Views/Home/DisplayTemplates/MenuItem.ascx :

 <%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<CompanyName.Framework.Web.MenuItem>" %> <% if (!string.IsNullOrEmpty(Model.RequiredRole) && System.Threading.Thread.CurrentPrincipal.IsInRole(Model.RequiredRole)) { %> <li> <a href="<%= Model.Uri %>"><%= Model.Title %></a> </li> <% } %> 

And then in your main view:

 <div id="main-menu" class="menu"> <ul> <%= Html.DisplayForModel() %> </ul> </div> 

---

Improvement number 3 . Avoid coding business rules in a view. Therefore, in your view model add the property:

 public bool IsLinkVisible { get { return !string.IsNullOrEmpty(RequiredRole) && Thread.CurrentPrincipal.IsInRole(RequiredRole); } } 

so that your display template now looks like this:

 <%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<CompanyName.Framework.Web.MenuItem>" %> <% if (Model.IsLinkVisible) { %> <li> <a href="<%= Model.Uri %>"><%= Model.Title %></a> </li> <% } %> 

---

Improvement number 4 . Write your own HTML helper to render this anchor, because the C # notation in the view is still ugly and unstable:

 public static class HtmlExtensions { public static MvcHtmlString MenuItem(this HtmlHelper<MenuItem> htmlHelper) { var menuItem = htmlHelper.ViewData.Model; if (!menuItem.IsLinkVisible) { return MvcHtmlString.Empty; } var li = new TagBuilder("li"); var a = new TagBuilder("a"); a.MergeAttribute("href", menuItem.Uri); a.SetInnerText(menuItem.Title); li.InnerHtml = a.ToString(); return MvcHtmlString.Create(li.ToString()); } } 

and finally your display template:

 <%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<CompanyName.Framework.Web.MenuItem>" %> <%= Html.MenuItem() %> 

Answer 2

Yes, you can just put this block in a .ascx file and use:

 <% html.RenderPartial("myPartialFile.asx"); %> 

The above assumes that myPartialFile.ascx is in the same folder as your main page, usually the Views / Shared folder.