FxCop complaint: specific xml types detected and poor improvement

Question

I want to save specific classes, and since xml serialization will not do this in my case, I save the values ​​manually in an XML document. It works great, but he doesn’t like FxCop, and since FxCop usually gives good advice and reasons why I shouldn’t do something in a certain way, I try to keep him happy.

This time I do not understand how this 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: “Change the member of“ OtherclassSave2 (XmlNode) ”so that it no longer shows the specific type“ XmlNode. ”Use IXPathNavigable to represent XML data sources.”

And now my wonderful solution:

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

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

Although I’m talking about changing void SaveInThisNode (XmlNode) to XmlNode GetMeTheNode (), but to create nodes via CreateElements I need an XmlDocument object that I am not allowed to use as a parameter, but I could create new XmlDocuments at each level, fine.

My solution was simple and worked perfectly in everything I wanted, but FxCop does not seem to allow solutions that are clearly no worse and more complex.

Answer 1

FxCop says you should use an interface instead of a specific implementation of the interface . It was probably discovered that in your OtherclassSave2 method OtherclassSave2 the nav parameter can be used as IXPathNavigable without specifying a specific implementation (only members affected by IXPathNavigable are IXPathNavigable ).

Since 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 says that here is the most common example of an FxCop problem found:

Say what 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 like List<T> , it is not recommended to set the parameter type as List<int> , because this will limit the use of your Sum method. Since the Sum implementation uses only foreach , it is preferable to write:

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

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

Answer 2

This simply assumes that you are not communicating with a particular implementation of the XmlNode in the method signature. This allows you to change the internal implementation without affecting anything using the class.

It is suggested that you can ignore the warning if you need specific functionality from a particular class. If this is a public API, you should try to split as much as you can, which will give you the freedom to change the implementation with less likelihood of changing the method signatures and, thus, forcing the consumers of the API to change their implementation.

CA1059: Members must not expose certain specific types

Answer 3

I found LinqToXml using XElements , which was exactly the kind I was looking for in a simpler and more powerful way with fewer FxCop and interface errors.