FxCop Complaint: Exposed concrete xml types and a bad improvement

Question

I want to save certain classes and since xml-serialization won't do it in my case i'm saving the values manually into an xml-document. Works fine, but FxCop doesn't like it and since FxCop normally gives good advice and reasons why i shouldn't do things in a certain way i try to keep it happy.

This time, i dont understand how this is an improvement.

This is what i had:

public void Save()
{
      XmlDocument doc = new XmlDocument();
      XmlNode XmlNodeJob = doc.CreateElement("Job");
      doc.AppendChild(XmlNodeJob);
      OtherclassSave2(XmlNodeJob);//Node as Parameter
 }

 public void OtherclassSave2(XmlNode node)
 {

 }

And this is what FxCop complained: "Modify member 'OtherclassSave2(XmlNode)' so that it no longer exposes the concrete type 'XmlNode'. Use IXPathNavigable to represent XML data sources."

And now my awesome solution:

    public void Save()
    {
        XmlDocument doc = new XmlDocument();
        XmlNode XmlNodeJob = doc.CreateElement("Job");
        doc.AppendChild(XmlNodeJob);
        OtherclassSave2(XmlNodeJob.CreateNavigator());//Interface from a node's navigator
    }

    public void OtherclassSave2(IXPathNavigable nav)
    {
        XmlNode node = (XmlNode)(nav.CreateNavigator().UnderlyingObject);

    }

This way i get my node in the other method and FxCop is happy, but i really don't see the improvement and i need a node to add things in it, not something to read.

I though about changing void SaveInThisNode(XmlNode) into a XmlNode GetMeTheNode() but to create nodes via CreateElements, i need the XmlDocument-object which i am not allowed to use as a parameter, but i could create new XmlDocuments in every step, fine.

My solution was simple and worked fine for everything i wanted it to do, but FxCop does not seem to allow solutions that are not obviously worse and more complicated.

Answer 1

FxCop is saying that you should use the interface instead of the concrete implementation of the interface. It probably detected that in your OtherclassSave2 method the parameter nav could be used as a IXPathNavigable without specifying the concrete implementation (only members exposed by IXPathNavigable are used).

As XmlNode implements IXPathNavigable, you should be able to write:

public void Save()
{
      XmlDocument doc = new XmlDocument();
      XmlNode XmlNodeJob = doc.CreateElement("Job");
      doc.AppendChild(XmlNodeJob);
      OtherclassSave2(XmlNodeJob);
 }

public void OtherclassSave2(IXPathNavigable node)
{
    // Deal with node using the interface only
}

---

Just to clarify why FxCop is saying that, here is the most common example of the issue FxCop detected:

Say you have:

public int Sum(List<int> parameter)
{
    int tmp = 0;
    foreach (int i in parameter)
    {
        tmp += i;
    }

    return i;
}

List<int> lst = new List<int> {3, 4, 5};
int sum = Sum(lst);

As Sum implementation does not use specific methods of the List<T> type, it's not a good idea to set the type of parameter as List<int> because it will limit the usage of your Sum method. As Sum implementation only use a foreach, it's preferable to write:

public int Sum(IEnumerable<int> parameter)
{
    int tmp = 0;
    foreach (int i in parameter)
    {
        tmp += i;
    }

    return i;
}

so you can call Sum with other types that List<T>: ObservableCollection<T>...etc.