Sunday, 2 June 2013

Path vs Content in Method Parameter

Time for some irrelevant musing over some design details in 2 well known .Net libraries.

The other day, while reading the documentation of log4net, the signature of one of its methods rather caught my attention. I'm referring to the use of a FileInfo parameter in one of the XmlConfigurator.Configure overloads. I'm quite unaware of having seen before this technique in any other libraries, but at first sight it seemed quite interesting for how clearly it transmits the intent of the method. If you were passing a string, you would need to read the documentation to know whether that string is a path or a block of config values. With a FileInfo parameter it's crystal clear that the method expects a path. On the other side, as somebody says in this good StackOverflow discussion, probably most times you'll find yourself doing something like Configure(new FileInfo(path)), without leveraging any of the additional functionality of FileInfo, which seems like a waste of resources. On second thought, I think I would favor a design like this:
XmlConfigurator.ConfigureFromFile(string path);

This has made me think about how other popular libraries deal with similar issues. The first case that came to mind is .Net's BCL XmlDocument and its Load overloads. We find some Load methods expecting Readers or a Stream (notice that log4net's also had a Configure overload expecting a Stream), which intent is pretty clear, and then we find a Load method receiving a String, that the documentation tells is a path. Afterwards we find a LoadXml method expecting a string, and that the documentation explains as being a xml block.

Well, I would say this naming is confusing, and actually I can remember one case where this led me to write wrong code (by using the autocomplete without reading the doc). As all the other Load methods are receiving the xml data (either in a Stream or Reader), I think the Load(string) method should also receive xml data there (hence replacing LoadXml) and then, there should be a LoadFromFile(string) method.

No comments:

Post a Comment