Trade-Offs

While reading the excellent xUnit Test Patterns book I was introduced to the idea that making something less private on a class in order to get a test for it in place may break encapsulation for good. While this pattern ensures that you just have a single instance of a configuration or memory consuming object in your production system, you will have a problem with unit testing it. The xUnit book provides a pattern to make this easy. However, the Substituted Singleton pattern relies on the fact that you don’t define the singleton instance on your original class as private, but rather as protected, so you can override the instance by a subclass where you have influencing methods so you can influence your test behaviour directly. This breaks the encapsulation of the class you wrote for production use, but for good, since you can now test all your users of that singleton class rather than needing to do some special treatments of your environment in order to achieve the same. A similar reasoning can be applied for not using the final modifier on classes. These ones you will not be able to test by then, too. When deciding to do just this, you’re trading off the ability to unit test your code and show that you produced a good quality while doing so and communicating your intents for the next programmer having to deal with your class by breaking encapsulation in your class hierarchy. The first overweighs the second here.

Currently I am faced with a similar situation at work in a different context. We use Perforce for version control at work. This tool is able to indirectly track renamed files not directly – you have to do an integrate to produce a copy with a new name and delete the old file afterwards for that. The current eclipse plugin for Perforce does not cope with this. It just deletes the old file after having created the new file and opened that one for add. By submitting this change you loose the version history and cannot see anymore that the previous name of a class was Foo and is now Bar or that initially you had put up the class in the package foo.bar and now is placed in foo.baz. One of my colleagues realized this and now mandates that we should do a semi-manual refactoring for these kind of operations (rename class, move class). Personally for me the same trade-off as for encapsulation of fields in singletons exists here. Having tool support for doing these refactorings completely automatically is a big win and I can live with the fact that I will not obviously see where the file was coming from in the version history. The class was renamed to hopefully some good reason – i.e. to communicate intent in a better way or to have it placed under a package, where it is reasonable. On a side note all the history I would need should be placed in the unit tests for the classes. This trade-off is fine for me and I don’t see good reasons to get into the semi-automated process of manually integrating a file in the version control system, just to see the complete history of the file. It’s fine if I just see, “Oh I added that file and deleted that one in the first change to that file, seems like a renaming operation.” and follow-up on that file if needed (I don’t read the revision history often – most of the time I don’t have to.)