c# xml code improvement suggestions

P

Peted

Using Vs 2008 C#

Im using this code segment bellow, which works and does what im after,
but im wondering if it is actually good code, or just inefficient and
if there is a better way to achieve the same result.

Essentially
1. recieve and xml string
2. check multiple <PCSServiceCode> nodes to see if at least 1
<ServiceCode> node exists
3. if not then remove that whole <PCSServiceCode> from the xml
string/doc
4.pass the modified xml string to be deserialized.

If anyone has any suggests or improvements, comments are welcome

regards
Peted


public static PCSBillingEventDataType Deserialize(string xml)
{
try
{

XmlDocument xmlDoc = new XmlDocument();
xmlDoc.LoadXml(xml);

foreach (XmlNode pcsServiceCodeNode in
xmlDoc.SelectNodes("/PCSBillingEventData/ProblemPageList/PCSProblemPage/ServiceCodeList/PCSServiceCode"))

{
bool pcsNodeRemove = true;

foreach (XmlNode node in
pcsServiceCodeNode.ChildNodes)
{
if (node.Name.ToString() == "ServiceCode")
pcsNodeRemove = false;
}

if (pcsNodeRemove)
{

pcsServiceCodeNode.ParentNode.RemoveChild(pcsServiceCodeNode);
}

}


// assign new xml data for deserialization.
xml = xmlDoc.InnerXml;

/* deserialize xml */
XmlSerializer serializer = new
XmlSerializer(typeof(PCSBillingEventDataType));

using (StringReader sr = new StringReader(xml))
{

PCSBillingEventDataType data =
(PCSBillingEventDataType)serializer.Deserialize(sr);
return data;
}
}
catch (Exception ex) { throw; }
}
}
 
A

Alberto Poblacion

If anyone has any suggests or improvements, comments are welcome
[...]
foreach (XmlNode node in
pcsServiceCodeNode.ChildNodes)
{
if (node.Name.ToString() == "ServiceCode")
pcsNodeRemove = false;
}
[...]

Instead of looping through all the nodes to see if any of them is
"ServiceCode", you can use an XPATH query to get this information:

XmlNode xn = pcsServiceNode.SelectSingleNode("ServiceCode");
if (xn!=null) pcsNodeRemove = false;

In fact, if you know well enough your XPATH (which I do not), you can
probably write an XPATH query that will directly find all of the
PCSServiceCode nodes which do not contain any ServiceCode children, all in a
single line of source code using SelectNodes(). You would end up with
something similar to this:

foreach (XmlNode pcsServiceCodeNode in
xmlDoc.SelectNodes("/PCSBillingEventData/ProblemPageList/PCSProblemPage/ServiceCodeList/PCSServiceCode[not
(ServiceCode)]"))
{
pcsServiceCodeNode.ParentNode.RemoveChild(pcsServiceCodeNode);
}
 
D

Dathan

If anyone has any suggests or improvements, comments are welcome
[...]
                   foreach (XmlNode node in
pcsServiceCodeNode.ChildNodes)
                   {
                       if (node.Name.ToString()== "ServiceCode")
                           pcsNodeRemove = false;
                   }
[...]

    Instead of looping through all the nodes to see if any of them is
"ServiceCode", you can use an XPATH query to get this information:

XmlNode xn = pcsServiceNode.SelectSingleNode("ServiceCode");
if (xn!=null) pcsNodeRemove = false;

    In fact, if you know well enough your XPATH (which I do not), youcan
probably write an XPATH query that will directly find all of the
PCSServiceCode nodes which do not contain any ServiceCode children, all in a
single line of source code using SelectNodes(). You would end up with
something similar to this:

                foreach (XmlNode pcsServiceCodeNode in
xmlDoc.SelectNodes("/PCSBillingEventData/ProblemPageList/PCSProblemPage/ServiceCodeList/PCSServiceCode[not
(ServiceCode)]"))
{
    pcsServiceCodeNode.ParentNode.RemoveChild(pcsServiceCodeNode);

}

The XPath query you provide should have the correct results. Or you
could use "/PCSBillingEventData/ProblemPageList/PCSProblemPage/
ServiceCodeList/PCSServiceCode[count(ServiceCode)=0]", though the one
you provide will most likely execute more quickly.

~Dathan
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top