principlecsharpMinor
Elegant approach to building unordered list from XML
Viewed 0 times
unorderedbuildingxmlelegantlistfromapproach
Problem
How can I improve on this rather ugly method that builds an HTML unordered list from an XML file?
I maintain a collection of ASP.NET webforms that all need to pull in the same site navigation as our main website. The forms have a master page that mimics the main site template, and we want any changes to the main navigation to flow through to the forms automatically.
The main site uses an XML document to generate the navigation, and the document uses inconsistent formatting for the links (some have "http://mysite.com" hardcoded, and some are relative to the main site "/subsite").
Here's what I have right now:
Believe it or not, this is an improvement over the original, which included a hardcoded URL for the XML file and got the values out of the XML document like this:
But it still needs a lot of work. Please have at it.
I maintain a collection of ASP.NET webforms that all need to pull in the same site navigation as our main website. The forms have a master page that mimics the main site template, and we want any changes to the main navigation to flow through to the forms automatically.
The main site uses an XML document to generate the navigation, and the document uses inconsistent formatting for the links (some have "http://mysite.com" hardcoded, and some are relative to the main site "/subsite").
Here's what I have right now:
private void LoadNavigation()
{
string urlPrefix = "http://mysite.com";
string xmlFilePath = ConfigurationManager.AppSettings["NavigationUrl"].ToString();
XmlDocument doc = new XmlDocument();
doc.Load(xmlFilePath);
XmlNodeList navigationItems = doc.DocumentElement.FirstChild.ChildNodes;
foreach (XmlNode item in navigationItems)
{
if (item.Attributes.GetNamedItem("url").Value.Contains("http://")) // link is hard coded
{
navList.InnerHtml += " " + item.Attributes.GetNamedItem("title").Value + "|";
}
else
{
navList.InnerHtml += " " + item.Attributes.GetNamedItem("title").Value; //relative link
if (item.NextSibling == null)
{
navList.InnerHtml += ""; //last item in list
}
else
{
navList.InnerHtml += "|"; //not the last item in list
}
}
}
}Believe it or not, this is an improvement over the original, which included a hardcoded URL for the XML file and got the values out of the XML document like this:
XmlNodeList nodeList = doc.ChildNodes[1].ChildNodes[0].ChildNodes;But it still needs a lot of work. Please have at it.
Solution
The part that looks most problematic in your code is the
However there's an even better way: .net already comes with the
if statement: the code in the if part and the else part is almost the same except that you prepend urlPrefix to the URL if it is relative. (Also you're only checking whether the node is the last if the URL is relative, which does not seem right). At the very least I'd factor this out into a helper method, which takes the absolute URL as a parameter to remove the code duplication.However there's an even better way: .net already comes with the
System.Uri class, which can take care of making the URL absolute for you.private void LoadNavigation()
{
Uri urlPrefix = new Uri("http://mysite.com");
string xmlFilePath = ConfigurationManager.AppSettings["NavigationUrl"].ToString();
XmlDocument doc = new XmlDocument();
doc.Load(xmlFilePath);
XmlNodeList navigationItems = doc.DocumentElement.FirstChild.ChildNodes;
foreach (XmlNode item in navigationItems)
{
Url fullUrl = new Uri(urlPrefix, item.Attributes.GetNamedItem("url").Value);
String title = item.Attributes.GetNamedItem("title").Value;
navList.InnerHtml += " " + title + "";
if (item.NextSibling == null)
{
navList.InnerHtml += ""; //last item in list
}
else
{
navList.InnerHtml += "|"; //not the last item in list
}
}
}Code Snippets
private void LoadNavigation()
{
Uri urlPrefix = new Uri("http://mysite.com");
string xmlFilePath = ConfigurationManager.AppSettings["NavigationUrl"].ToString();
XmlDocument doc = new XmlDocument();
doc.Load(xmlFilePath);
XmlNodeList navigationItems = doc.DocumentElement.FirstChild.ChildNodes;
foreach (XmlNode item in navigationItems)
{
Url fullUrl = new Uri(urlPrefix, item.Attributes.GetNamedItem("url").Value);
String title = item.Attributes.GetNamedItem("title").Value;
navList.InnerHtml += "<li> <a href=\"" + fullUrl + "\">" + title + "</a>";
if (item.NextSibling == null)
{
navList.InnerHtml += "</a></li>"; //last item in list
}
else
{
navList.InnerHtml += "</a>|</li>"; //not the last item in list
}
}
}Context
StackExchange Code Review Q#1056, answer score: 3
Revisions (0)
No revisions yet.