Interface Segregation Principle: A Practical Example

I’ve had this partially completed post in my drafts folder for a while, and, thanks to a sort of half-hearted New Years resolution to either finish or discard really old drafts, I’m going to finish this one. I changed the title and re-did the focus a bit, since this one was a year and a half old, and the code in question here is hazier and hazier in my mind. But it’s a tale of Singletons, needless coupling, poor cohesion, and woe.

I’ve been interviewing some candidates of late and one of the things I typically ask about is familiarity with the SOLID principles. An encouraging amount of people say things like, “oh sure, classes should have only one purpose” or “oh, yeah, we use IoC containers!” It seems as though S, O, and D get the most love, while L and I (Liskov Substitution Principle and Interface Segregation Principle, respectively) are rarely mentioned. Today, I’ll talk about I with an example that may hit home with you.

I worked on a project once with a fairly unique ‘architecture.’ It was a GUI-heavy desktop application with a reasonably straightforward set of domain objects that were read from persistence and stored in memory. This was accomplished using what really kind of amounted to an old school, C-style memory map of structs. There was a root object, and everything was hierarchically possessed by this root object. So, let’s say that you were modeling a parking lot full of cars — you’d navigate to the car parts via calls like Root.Cars[12].Engine.Battery or Root.Cars[5].Body.Exterior.Paint. And naturally, Root was implemented as a Singleton so that you could access this memory map from anywhere at any time to read or write. Yep. That was a long project.

This Singleton probably started out modestly enough, but by the time I had worked on the project, its sizable gravitational pull had roped smaller satellites, code-moons and free-falling bits of flotsam into it, creating a runaway juggernaut of anti-pattern. This thing was, if memory serves, pushing 10K lines of code and perhaps even spreading out into some partial classes. I think by the time I moved onto another project, it was showing the first signs of having an event horizon.

LargePlanet

There were several different skins on this desktop app, and for some reason, each of them had their own mini-versions of this Singleton (a much more modest 1K to 2K LOC, each), I guess to contain the skin-specific sprawl that would have created a circular reference situation with the behemoth itself. It was one of these mini-versions that inspired this post, along with a story a friend of mine told me after I’d left the project.

A few of us had, while working in this code base, endeavored to extract at least tiny pockets of code from the massive gravitational field of the Singletons so that we could write some unit tests and avoid the bug whack-a-mole game that ensued whenever you changed something in the global state. I was probably the most successful in fighting this battle and, my friend, having less success after I’d left, complained to me one day over lunch. He said that he’d had a perfectly testable class, but in a code review someone in a position of relative authority had pointed out that something he was doing was already done in one of the satellite Singletons and he should use that code. His request to hide the Singleton behind a wrapper or even an interface implementation was denied. A large portion of his class became thoroughly untestable because naturally, the first call to his stuff triggered the first lazy call to the satellite, which triggered the first lazy call to the black hole Singleton, which fired up every threading model, logger, aspect, and bit of file I/O in the history of creation, crushing the test runner like an insignificant gnat.

And this, to me, is perhaps the best argument for the Interface Segregation Principle that I’ve ever heard. As a quick recap, the ISP says that “clients should not be forced to depend upon interfaces that they don’t use.” In my friend’s case, depending on the utility method that he was being forced to use, in turn, made him depend on an entire, 1K+ LOC Singleton being initialized and, indirectly, a whole host of other application functionality. This was about the most egregious violation that I’d ever encountered.

What’s the alternative? Well, years later, it’s hard to list specifics, but how about pulling that utility out somewhere? If it relied on no global state, this is a no-brainer — just put it into its own class somewhere. Static, instance — it doesn’t matter — just not in that Singleton. If it relied on global state, then create a small interface with one method, have the satellite Singleton implement it, and hand that interface to the class in question. In either case, my friend’s class depends on a single method for that functionality and not the entire application domain and the code responsible for loading it (as well as loggers and other ancillary nonsense).

The Interface Segregation Principle asks you to keep your dependencies to a minimum. This will help you unit test, but it will also help your sanity at maintenance time. After all, would you rather debug a class that you knew depended on an external method or two, or one whose behavior could be explained by any one of tens of thousands of lines of code? These things will matter to you or whoever maintains the application. A lot. So think of this example and think of the ISP as you’re writing your code.

  • Neil

    To me this just shows the problem with static classes more than singletons. As a developer I want to work off an interface, not the actual implementation that the singleton design pattern, or static class always gives me. Given the rise of IOC frameworks, the need for a singleton design pattern is pointless. Just set the scope of the binding as a singleton, and all will be good. Just make sure the binding is an interface to a concrete class. A singleton (design pattern, or IOC scope) should always be implemented by an interface. This way, the dependency is on the interface. The dependency has no knowledge that the underlying object is a singleton.

    One great example I always run into with static classes is encryption. Say I have some sort of encryption/decryption algorithm, I continually see fellow developers move that logic into a static class. Testing that static class implementation is easy enough, but when attempting to write tests, especially when doing behavior specific tests following TDD in some authorization domain class that has direct dependency to the encryption class becomes very tricky. Not only do I have to test the logic to ensure that a user is authorized given some kind of key/token the method was passed, but now I need to pass in encrypted strings that will decrypt to what my unit tests need to test the behavior. It completely couples the code that should strictly handle the authorizing to a specific encryption algorithm.

    Going further, not only is it hard and annoying for the unit tests, but say someone finds a way to exploit TripleDES. Now we need to use a new encryption algorithm in our application. Not only does the production code need to change, but all my unit tests for the authorization domain class fails.

    The important thing to know is that even though the encryption algorithm will never change, the dependency to use it might. The dependency being my authorization domain class.

    My rule of thumb is that singletons should be used sparingly and static classes should never be used 99.99% of the time.

    Singletons to me should be used when there is no state involved, and the dependencies on the singleton are not disposable.

    As for static classes, I think it best to rarely, but I tend towards never use them. Some developers would say they’re utility classes, but nearly all the utility classes I see are prone to changes, or just wrap a call to another static method, which is the worse kind of static method implementation (in my opinion). Also, in my opinion, all the utility methods have already been developed in the framework; like string’s IsNullOrWhiteSpace, but to me, that doesn’t need to be a static method, but a method exposed to the instance of a string object. There’s obvious need to something like LINQ in C# via extension methods; howerver LINQ uses interfaces, e.g. IEnumerable, and LINQ’s behavior is not subject to change.

    Probably rambling now, but using singletons without an interface abstraction is an awful idea like you said, and static classes to me can be one of the worst things in a codebase given the duration it exists.

  • http://www.daedtech.com/blog Erik Dietrich

    For what it’s worth, I am in agreement with you about the nature of statics and static state in general. When it comes to singleton, I like to distinguish between lower case singleton (something of which there is only one) and upper case Singleton (the design pattern — there can BE only ONE!!!). I have no problem with singleton, but I really, really hate dealing with Singleton — an awkward construct that invites abuse. And the main reason I hate dealing with Singleton is because it almost invariably is just a dump for global variables (i.e. the static state problem).

  • Pingback: The Baeldung Weekly Review 7()