Pergunta

Quick question.

How would you refactor this Asp.net MVC 2 HtmlHelper? Specifically would it make sense to use the TagBuilder class in this scenario?

        public static MvcHtmlString BusinessDisplayContacts(this HtmlHelper helper, string phone, string cellPhone, 
        string fax, string website, string email, bool hideEmail) 
    {
        StringBuilder sb = new StringBuilder();

        sb.AppendLine("<ul>");
        if (!string.IsNullOrEmpty(phone)) { 
            sb.AppendLine("<li class=\"tel\">");
            sb.AppendLine("<span class=\"type\">Work</span>:");
            sb.AppendFormat("<span class=\"value\">{0}</span>",phone);            
            sb.AppendLine("</li>");
        }
        if (!string.IsNullOrEmpty(cellPhone)) { 
            sb.AppendLine("<li class=\"tel\">");
            sb.AppendLine("<span class=\"type\">Cell</span> Phone:");
            sb.AppendFormat("<span class=\"value\">{0}</span>",cellPhone);
            sb.AppendLine("</li>");
        }
        if (!string.IsNullOrEmpty(fax)) { 
            sb.AppendLine("<li class=\"tel\">");
            sb.AppendLine("<span class=\"type\">Fax</span>:");
            sb.AppendFormat("<span class=\"value\">{0}</span>",fax);
            sb.AppendLine("</li>");
        }
        if (!string.IsNullOrEmpty(website)) { 
            sb.AppendFormat("<li><a class=\"url\" href=\"{0}\">{0}</a></li>",website);
        }
        if (!hideEmail && !string.IsNullOrEmpty(email)) {
            sb.AppendFormat("<li><a class=\"email\" href=\"mailto:{0}\">{0}</a></li>",email);
        }
        sb.AppendLine("</ul>");

        if (sb.Length < 10)
        {
            return MvcHtmlString.Create("");
        }
        else {
            return MvcHtmlString.Create(sb.ToString());
        }
    }

Thanks in advance.

UPDATE:
Thank for all of the constructive comments. In the end I've decided to move the above code into a strongly typed partial view as per @queen3's suggestion.

Foi útil?

Solução

One thing I see people missing a lot, is using C# verbatim strings for stuff like that... e.g.

sb.AppendLine("<li class=\"tel\">");
sb.AppendLine("<span class=\"type\">Work</span>:");
sb.AppendLine(string.Format("<span class=\"value\">{0}</span>",phone));            
sb.AppendLine("</li>");

can be made into

sb.AppendFormat(@"
<li class=""tel"">
    <span class=""type"">Work</span>: <span class=""value"">{0}</span>
</li>
", phone);

which is way more readable.

Another thing: I would put all those strings + bool inside an object, like ContactInfo or something, changing the signatur of your helper to BusinessDisplayContacts(this HtmlHelper helper, ContactInfo info) - this way you will be able to add/remove/modify phone numbers and conditions without breaking existing code.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top