views:

4367

answers:

3

I originally asked this question on RefactorMyCode, but got no responses there...

Basically I'm just try to load an XmlNodeList into an XmlDocument and I was wondering if there's a more efficient method than looping.

Private Function GetPreviousMonthsXml(ByVal months As Integer, ByVal startDate As Date, ByVal xDoc As XmlDocument, ByVal path As String, ByVal nodeName As String) As XmlDocument
    '' build xpath string with list of months to return
    Dim xp As New StringBuilder("//")
    xp.Append(nodeName)
    xp.Append("[")
    For i As Integer = 0 To (months - 1)
      '' get year and month portion of date for datestring
      xp.Append("starts-with(@Id, '")
      xp.Append(startDate.AddMonths(-i).ToString("yyyy-MM"))
      If i < (months - 1) Then
     xp.Append("') or ")
      Else
     xp.Append("')]")
      End If
    Next

    '' *** This is the block that needs to be refactored ***
    '' import nodelist into an xmldocument
    Dim xnl As XmlNodeList = xDoc.SelectNodes(xp.ToString())
    Dim returnXDoc As New XmlDocument(xDoc.NameTable)
    returnXDoc = xDoc.Clone()
    Dim nodeParents As XmlNodeList = returnXDoc.SelectNodes(path)
    For Each nodeParent As XmlNode In nodeParents
      For Each nodeToDelete As XmlNode In nodeParent.SelectNodes(nodeName)
     nodeParent.RemoveChild(nodeToDelete)
      Next
    Next

    For Each node As XmlNode In xnl
      Dim newNode As XmlNode = returnXDoc.ImportNode(node, True)
      returnXDoc.DocumentElement.SelectSingleNode("//" & node.ParentNode.Name & "[@Id='" & newNode.Attributes("Id").Value.Split("-")(0) & "']").AppendChild(newNode)
    Next

    '' *** end ***
    Return returnXDoc
End Function
A: 

Considering that there is no answer there or here...I think it's safe to say there is no better way ;)

RedWolves
+1  A: 
Dim returnXDoc As New XmlDocument(xDoc.NameTable)
returnXDoc = xDoc.Clone()

The first line here is redundant - you are creating an instance of an XmlDocument, then reassigning the variable:

Dim returnXDoc As XmlDocument = xDoc.Clone()

This does the same.

Seeing as you appear to be inserting each XmlNode from your node list into a different place in the new XmlDocument then I can't see how you could possibly do this any other way.

There may be faster XPath expressions you could write, for example pre-pending an XPath expression with "//" is almost always the slowest way to do something, especially if your XML is well structured. You haven't shown your XML so I couldn't really comment on this further however.

samjudson
A: 

@samjudson good catch on the redundancy! also, I had no idea that // was slower, I had just been using it because it made for neater XPaths, thanks for the tip, ++ for you!

I was really hoping there was some way to just merge a nodelist into the document that I didn't know about.

travis