On Comments
One way or another, all programmers eventually
learn the importance of commenting their code. The importance of clear
documentation increases with the size of the team and the complexity of the
project. Ideally, the code will not only be documented but unit tested as well.
Here's an example of what happens when, for one reason or another, we let
ourselves be less than thorough...
Let's create a hypothetical method,
GetRecord,
that takes an identifier string as an argument and returns an object of type
MyRecord:
// Return a record given its identifier.
public MyRecord GetRecord( string id )
{
}
Admittedly, the comment is a little obvious here and, for all it says, could be omitted. But what's missing?
While implementation details generally shouldn't be part of the comment if they're subject to change, the example clearly omits information that callers need to know. Suppose the identifier is malformed, or the record for that identifier has been deleted. What does the method return? Null? A default record of some sort? Does an exception get thrown? And what happens if our implementation needs to hit a remote server to complete the request, and a connection can't be established?
Pretend for a moment the implementation throws an exception when a record isn't found. Code gets built upon the method with this assumption. Some time in the future, another programmer makes use of the method and decides it'd be better if it returned null instead, changing the implementation to suit new code. It's a small change, really, and development on the product continues. It's obvious where this is leading.
There's now a body of code that expects an exception to be thrown when the record isn't found, and another that expects a null return value. The code expecting the exception may not be prepared to handle a null return value, and null reference exceptions could result. If, to fix this, a developer goes in and changes GetRecord back to its original implementation, the code expecting a null return value will get exceptions it may not be prepared to deal with. Either way, this "innocent" little change has created the sort of mess you don't want in the final stages of development.
Good documentation can help, if one takes it seriously. And one should: it is no less important than the method's signature; it is the human part of the interface contract. That not only means the comments need to be in place and respected, but it means that comments must be updated as the method's functionality is extended or modified. The impacts of even minor changes must be studied, particularly changes made in lower layers of the code. Like Bradbury's crushed butterfly, tiny changes in lower layers can have a ripple effect on the stability of the product.
Would unit testing have saved the day? Only possibly. Certainly, if a unit test had been written against the possibility of a missing record, the change would've been caught... but in this case the change was intentional, not accidental, and the developer who made the change might just have updated the unit test as well. All of which leaves you with a passing unit test and broken code.
Would comments describing the behavior of GetRecord in the case of missing records really have helped? Only if the developers involved understand those comments to be part of the method's contract and act appropriately.
So: document both the expected and unexpected behavior, and foster a culture where the comments are no less important than the code itself. And if something isn't documented but clearly needs to be, don't assume that changing the implementation is safe.
// Return a record given its identifier.
public MyRecord GetRecord( string id )
{
}
Admittedly, the comment is a little obvious here and, for all it says, could be omitted. But what's missing?
While implementation details generally shouldn't be part of the comment if they're subject to change, the example clearly omits information that callers need to know. Suppose the identifier is malformed, or the record for that identifier has been deleted. What does the method return? Null? A default record of some sort? Does an exception get thrown? And what happens if our implementation needs to hit a remote server to complete the request, and a connection can't be established?
Pretend for a moment the implementation throws an exception when a record isn't found. Code gets built upon the method with this assumption. Some time in the future, another programmer makes use of the method and decides it'd be better if it returned null instead, changing the implementation to suit new code. It's a small change, really, and development on the product continues. It's obvious where this is leading.
There's now a body of code that expects an exception to be thrown when the record isn't found, and another that expects a null return value. The code expecting the exception may not be prepared to handle a null return value, and null reference exceptions could result. If, to fix this, a developer goes in and changes GetRecord back to its original implementation, the code expecting a null return value will get exceptions it may not be prepared to deal with. Either way, this "innocent" little change has created the sort of mess you don't want in the final stages of development.
Good documentation can help, if one takes it seriously. And one should: it is no less important than the method's signature; it is the human part of the interface contract. That not only means the comments need to be in place and respected, but it means that comments must be updated as the method's functionality is extended or modified. The impacts of even minor changes must be studied, particularly changes made in lower layers of the code. Like Bradbury's crushed butterfly, tiny changes in lower layers can have a ripple effect on the stability of the product.
Would unit testing have saved the day? Only possibly. Certainly, if a unit test had been written against the possibility of a missing record, the change would've been caught... but in this case the change was intentional, not accidental, and the developer who made the change might just have updated the unit test as well. All of which leaves you with a passing unit test and broken code.
Would comments describing the behavior of GetRecord in the case of missing records really have helped? Only if the developers involved understand those comments to be part of the method's contract and act appropriately.
So: document both the expected and unexpected behavior, and foster a culture where the comments are no less important than the code itself. And if something isn't documented but clearly needs to be, don't assume that changing the implementation is safe.
Posted: Tue - January 16, 2007 at 05:07 PM
Quick Links
Recommended!
Software I've Written
Other Writings
Sites I Maintain
Archives
