Sat down today trying to get some legacy code under tests. I started out with a fairly
simple test... I taught.
[Test]
publicvoid XmlGetsBiggerWhenCustomerInformationIsProvidedTest()
{
XmlDocument inXmlDoc =new XmlDocument();
inXmlDoc.LoadXml(SetUp_GetInXml());
CustomerFetcher customerFetcher =new CustomerFetcher();
XmlDocument outXmlDoc = customerFetcher.PopulateXml(inXmlDoc);
Assert.IsTrue(outXmlDoc.InnerXml.Length > inXmlDoc.InnerXml.Length);
}
All
I got was the red bar, and I couldn't figure out why. The xml looked OK! After diving
into the source I found the villain:
public XmlDocument
PopulateXml(XmlDocument xmlDoc)
{
/*...
lots of code ...*/
return xmlDoc;
}
So
it returns the same object it takes as in parameter. That doesn't feel right in my
book, why return the same object when they are passed along by reference? Just return
void then and it will become more obvious to the client. So the reason that my test
failed was that I was comparing an object to itself.
So
I went ahead and changed it. Trying to create a new XmlDocument from the in parameter.
Why are there no copy constructors in thees object? I would like to:
XmlDocument
xmlDocToPopulate =new XmlDocument(xmlDoc);
But
that doesn't work. So I tried to IntelliSense my way using the clone-method. But that
didn't work out to well either, it only works with XmlNode objects (OK, I did not
spend a more than a minute trying). So I ended up with this ugly piece of code:
public XmlDocument
PopulateXml(XmlDocument xmlDocIn)
{
XmlDocument xmlDoc =new XmlDocument();
xmlDoc.LoadXml(xmlDocIn.OuterXml);
/*...
populating etc ...*/
return xmlDoc;
}
Not
beautiful, but at least I got the code under test! Next step i to refactor that away
keeping the green bar.
Conclusion?
Make your API's hard to misunderstand. (And put some copy constructors into the object
hierarchies)
