Friday, September 2, 2016

Even Good Code Comments Deteriorate

As I mentioned in a previous blog post, I've been working a bit with Infinispan recently. One of the things that I like about Infinispan's and PostgreSQL's documentation is that each product's documentation makes it very clear which version of the product the documentation applies to and they each make it easy to find other or more current versions of the documentation. For example, the PostgreSQL 9.5 documentation lists "This page in other versions" across the top with links to other versions of the same documentation. An Infinispan example is the Infinispan 8.1 documentation that not only includes the version in the URL, but also states in very obvious fashion, "This is written specifically for Infinispan 8.1." In the case of the Infinispan documentation, the versioned documentation helped me realize when some class-level Javadoc documentation was no longer correct.

The Javadoc-based class level description for class org.infinispan.manager.DefaultCacheManager provides the type of core API class documentation that I like to see. It explains the types and number of instances of the class would commonly be expected and even provides a code-based "Sample usage" of the class. Unfortunately, this description applies more to the old interface CacheManager than the current class DefaultCacheManager that formerly (but no longer) implemented the CacheManager interface. The following screen snapshot shows the 8.2 Javadoc documentation for DefaultCacheManager.

This Javadoc documentation for CacheManager appears to have been on the interface org.infinispan.manager.CacheManager since at least Infinispan 4 (first version of Infinispan because of its JBoss Cache heritage). The class org.infinispan.manager.DefaultCacheManager at that time implemented CacheManager and had almost the same Javadoc documentation as the interface it implemented as shown in the next screen snapshot.

Infinispan 5 introduced another CacheManager as a class (org.infinispan.cdi.CacheManager). Infinispan 6 removed that CacheManager class and deprecated the CacheManager interface with comment, "This interface is only for backward compatibility with Infinispan 4.0.Final and it will be removed in a future version. Use EmbeddedCacheManager or CacheContainer wherever needed." There is no mention of a CacheManager (class or interface) in Infinispan 7.0 Javadoc, but the DefaultCacheManager class still references CacheManager like it's an interface.

This is an example of how even good code comments can deteriorate over time as APIs and other documented constructs change. I have seen this effect in several code bases that I've worked on: even in-code comments that are accurate and helpful when they are first written can become less helpful or even misleading when things change and the in-code documentation doesn't change with them. This is more likely to happen when the documentation is repeated across multiple constructs such that when the code changes, the comments only get updated or removed in one of the places and not the other.

I don't want to imply that I think Javadoc or other comments or in-code documentation should not be written because I definitely think that well-written in-code documentation can be highly useful. However, it's also true that comments are not something to be written once with the expectation that they won't need to be changed at some point. In-code documentation needs to be treated with the same care and attention as the code it describes. Otherwise, the in-code documentation that at one time accurately described a construct might actually end up confusing understanding of the construct it's expected to help describe.

1 comment:

Norbert Kiesel said...

I'm sure they would welcome a patch to fix this!