published: Fri, 11-Mar-2005 | updated: Thu, 27-Oct-2005
We're having a minor discussion here in development about a bug I found. The bug is completely benign at the moment (indeed, I only found it through a code review, not through using the product), but code could easily be written to fire it.
Essentially there is a flag in a container class that causes an instance of the class to function in two different ways (let's call it a sort order). The flag is initially set through the constructor of the class. There is a private container of data inside the class that is built according to the flag (so, if you like, it'll be built according to the current sort order). The flag is exposed as a property with a setter, but the setter does not rebuild the internal container when the flag is changed. It obviously should and therein lies the bug.
As it happens, the property is not referenced anywhere in the solution at all; it's never read, it's never written to. Even worse in a way, there is no test case for it (the unit test cases are also part of the solution, although they are in different projects from the main code).
This property should never have been implemented at all. It was presumably added in the laudable endeavor to have the class be helpful about its internal state for some future implementation possibility. I've done this kind of thing before (the "hey, let's add a Count property, I'm bound to want it at some time" situation).
If we call implementing code to satisfy a requirement or to pass a test case "intentional programming," then this is "unintentional programming." It is solely the requirements or the design that should drive what we should or should not write, not some desire to code a complete class encompassing every future use we could possibly envision.
For a simple property like this, especially when it's not even used, who cares, right? But, I'd argue that if we start on this slippery slope of implementing "might need this in the future" type functionality who know where it will end?
At the moment, the number of people involved in this particular solution is small (2 or 3). There are 216 .cs files in 48 projects. It's not too difficult to have some understanding of the entire source code, especially as the current team wrote it. This particular piece of functionality has to be readied for a (very) imminent release and the code is so locked down that only mega- priority bugs are being fixed. My bug is by no means mega-priority, but it will be fixed after the release since it is in the bug reporting system.
However, consider this: we have a policy here at Configuresoft for developers to be rotated onto different projects in order that we don't get islands of expertise and hence so we don't suffer from the "Joe was run over by a bus and he's the only one who knows this stuff" problem. After this release, who knows who will be maintaining and extending this code? I stumbled on one issue like this but who knows what other similar issues may still exist and that might be triggered inadvertently six months down the road?
Resist unintentional programming. Implement only what is required now, and no more.
Oh, and write tests for everything you code <g>.