Sat down today trying to get some legacy code under tests. I started out with a fairly simple test… I taught.

[Test]
void  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)