How can I avoid unmaintanable code in asp.net mvc views

Question

I find more and more of my asp.net mpc views are starting to look like my old crappy asp code, which I could never keep or keep clean with the integrated <%%> and html. below i have the code from which i am talking. is there any good practice or recommended way to avoid this and also keep opinions more important and readable.

<td> <% bool userRequiresApproval = false; if (!string.IsNullOrEmpty(item.loginName)) { MembershipUserCollection membership = (MembershipUserCollection)ViewData["UnapprovedUsers"]; if (membership != null && membership[item.loginName] != null) { userRequiresApproval = true; } } bool isLoggedInUserAdmin = false; if (ViewData.ContainsKey("isAdmin")) { isLoggedInUserAdmin = (bool)ViewData["isAdmin"]; } if (isLoggedInUserAdmin && userRequiresApproval) {%> <%= Html.ActionLink("View", "Details", new { id=item.Mail_ID })%>, <%= Html.ActionLink("Delete", "GotoDeleteConfirmPage", new { id = item.Mail_ID })%>, <%= Html.ActionLink("Approve", "Approve", new { id = item.Mail_ID })%> <%} else if (isLoggedInUserAdmin) {%> <%= Html.ActionLink("View", "Details", new { id = item.Mail_ID })%>, <%= Html.ActionLink("Delete", "GotoDeleteConfirmPage", new { id = item.Mail_ID })%> <%} else {%> <%= Html.ActionLink("View", "Details", new { id = item.Mail_ID })%> <%}%> </tr> <% } %> 

Answer 1

First, I will consider a specific question, and then an abstract:

Concrete

One possible way would be to create a set of HTML helpers or user controls that have some basic logic to determine if they should be visible. For example, your use may be:

 <td> Html.LinkList(", " ActionLinks.ViewDetails(item), ActionLinks.DeleteAndConfirm(item), ActionLinks.Approve(item)) </td> 

Each action contains its own logic to determine whether it should be used (for example, “I require administrator rights”), and if this action defines its own criteria, you do not need to return string.Empty :

 class ActionLinks { public static string Approve(Item item) { if(ItemRequiresApproval(item) && CurrentUserIsAdmin()) { return Html.ActionLink("Approve", "Approve", new { id = item.Mail_ID }); } else { return string.Empty; } } private static bool ItemRequiresApproval(Item item) { //determine whether item requires approval //this could be further broken into a separate utilities class } private static bool CurrentUserIsAdmin() { //this should definitely go in a separate class dedicated to //handling membership and authorization //as well as figuring out who the current user is } } 

LinkList will look something like this:

 string LinkList(string delimiter, params string[] links) { StringBuilder sb = new StringBuilder(); foreach(string link in links) { if(!string.IsNullOrEmpty(link)) { sb.Append(delimiter); sb.Append(link); } } return sb.ToString().Substring(delimiter.Length); } 

annotation

The solution to your problem is to remember SRP (the principle of shared responsibility) and SOC (Separation of problems) . In your current example, your view is a class. You have made this class responsible not only for the overall markup structure, but also for every tiniest detail of almost your entire application! Your look should not know or care about administrator rights or approval. Only approving buttons should know about approval. Only administrator-specific items should be aware of administrator rights. If you find that you are repeating certain types of checks (for example, "if admin is show x, otherwise show y"), create some common wrappers, such as the AdminPanel, that will turn on or off accordingly. For all other players, this element is simply or not - and this element is responsible for making this decision.

Answer 2

Strongly entering your views with a base class containing things like IsAdmin will help (or even easier, populate this data in a session if it is not a controller / action).

For example, all of your code above can be combined into:

 <tr><td> <% Response.Write(Html.ActionLink("View", "Details", new { id=item.Mail_ID }); if (Model.IsLoggedInUserAdmin) { Response.Write(", " + Html.ActionLink("Delete", "GotoDeleteConfirmPage", new { id = item.Mail_ID })); } if (Model.UserRequiresApproval) { Response.Write(", " + Html.ActionLink("Approve", "Approve", new { id = item.Mail_ID })); } %> </td></tr> 

Or you could put the code between <%%> in the control and pass the necessary vyres to the RenderPartial, so that your look will not be cluttered with the Response.Write code navel and your original 30 lines are combined into 3 (td, renderpartial, / td) . In any case, you are clearly clarifying your opinion.

Answer 3

Use the NHAML viewer.
This will help you in many things and save a ton of prints.

Additionally (and mainly):

• Pull any logic into a controller or model
• Use a typed model instead of ViewData li>
• Use typed views
• Use extension methods
• Using partial views