In my previous discussion of patterns I touched on one of the most frequent anti-patterns; overuse of Singletons. Today I'm going to elaborate with an example of what happens when you use multiple anti-patterns together. As always details not relevant to the example have been changed to protect the guilty.
The application in question was an existing code base which I was brought in to assist with. The system suffered badly from Singletonitis. Many of the items configured as Singletons had no state and had no business being Singletons at all. In particular the data access layer was invoked via a Singleton which was also set up as a Facade.
The Gang of Four describe the intent of Facade as to "Provide a unified interface to a set of interfaces in a subsystem. Facade defines a higher-level interface that makes the subsystem easier to use." That was not the case here. This particular Facade exposed every method on every database repository class. As such it possessed hundreds of methods, many of which had names starting with common term like "Get". It therefore actually increased the complexity of the data access interface by exposing a large number of low level data access methods on a single interface with low cohesion.
In addition the Facade class performed a variety of support logic required by the repository classes, including managing transactions and database connections. This was done using cut and paste, resulting in the same code being duplicated a few hundred times within a single class. This of course had slight variances in places as changes were made in specific instances but not propagated to all instances.
The way this class was used and implemented indicates that it's an anti-pattern. It did provide a unified interface to the subsystem, but what it failed to do was make that subsystem easier to use. Additionally the massively duplicate code certainly didn't help when errors in it were discovered.
I can only speculate as to the cause of this design, but I have what I believe to be a reasonable hypothesis as to the cause. This includes:
- An incorrect belief that the data access code must be accessed via a Singleton. Objects without state are very cheap to instantiate in .NET. Even if they weren't better mechanisms (such as Dependency Injection) exist to support this requirement.
- As it was believed that a Singleton was needed, an unnecessary incentive to consolidate all the data access methods onto a single interface. Use of the more fine grained repository interfaces would have been a substantial improvement in this case due to improved cohesion and simpler interfaces.
- The Facade (or pseudo-Facade) implementation contained enormous duplication. I can only presume that the developers didn't consider this a significant problem or know how to fix it if they did.
This hypothesis, if correct, shows a cascading problem where the incorrect usage of Singletons incentivises further poor design decisions. Failing to consider the trade-offs of the patterns under consideration has imposed costs on all code that must relate to them. Combined these problems had a number of negative impacts on the code base:
- Use of Singletons destroyed testability as testing units in isolation was difficult to impossible.
- Working with the large repository Facade was difficult due to the sheer number of data access methods.
- The duplicate behaviour in the Facade required significant effort to maintain when issues with it were discovered.
None of these things are blockers than will prevent the software being developed. Yet they impact on the overall quality and maintainability of the system. Over time these are essential factors in ensuring the success of a software system. Professional developers have a responsibility to be concerned with long term consequences, hopefully for reasons other than concern they'll be stuck with the consequences. They therefore have a responsibility to apply patterns correctly and with a full and proper understanding of the trade-offs involved.