Monday, February 2, 2009

Joel on SOLID

After listening to Jeff and Joel's Stack Overflow podcast today I felt like Joel had slightly missed the point of the SOLID principles. I can see how it is easy to miss the point of SOLID - an acronym of acronyms! But for me "SOLID" adds to design patterns and anti-patterns because it helps me to describe, more formally, when some code doesn't seem right - when code has a "code smell" to be a bit of a hippy about it.

Joel goes on to say that he doesn't think the principles apply well to real life situations, however, I think they apply perfectly. They are all about taking a pragmatic but disciplined approach. Take the current work I'm doing as an example: I was asked to change our product's export function and make it parallel where possible. To start with it was a single class with a few thousand lines of code. It had a "unit" test to go with it that covers about 70% of the entire code base when run and takes about five minutes to complete. To even think about how it might be made parallel I had to do some serious refactoring.

Anyway here is my interpretation of the principles and how I used them when refactoring our export code:

  • Single responsibility principle: Read "keep it simple stupid". We literally had an Export class that did the whole export. It exported the raw data, PDF-ed if required, TIFF-ed if required, etc, etc. I changed the code so that the export class creates a WorkQueue class that determines the items to process; processors for processing items at each step; a report writer; and much more. Since parts of the system were no longer tightly coupled I could add tests for each part which ran faster and were more maintainable. Here is a simplified before and after:
    Classes before and after.
  • Open/closed principle: extending a module shouldn't break stuff. To be honest I don't consider this principle much during my day to day work, perhaps that is due to the type of product I'm working on.
  • Liskov substitution principle: have a good reason for using a base class. I feel that this strongly tied in with the SRP. For example our Exporter class inherited from BaseExporter (we have multiple types of export) and common "export" code was placed here. Sometimes a better solution is to place common code in a supporting class instead of inheriting. The acid test for inheritance should be am I ever going to use the base class independently of the subclasses? The LSP is about avoiding broken inheritance hierarchies. Uncle Bob gives the example of a set of number types which is excellent. He says inheritance is not "is a" and in the numbering example he points out that while integers are a subset of real numbers they have no commonality in terms of storage: an integer might be stored with a single 32 bit value where a real number has a mantissa and an exponent.
  • Interface segregation principle: again read "keep it simple stupid" but also "you aren't going to need it". This is simply a principle to avoid creating fat interfaces. The point is to define a service in terms of the clients that are going to use it. This is especially important in Java where events an delegates aren't available. For example if you have an ExportListener interface that has itemProcessedNotification and itemCompletedNotification a client will have to implement both listeners even if it doesn't care about both. Fat interfaces reduce signal to noise ratio - it obscures what you are trying to do.
  • Dependency inversion principle: don't tightly couple things. If everything depends on concrete implementation classes, if there are global static singletons, etc, things will be hard to modify and test. This is especially important in C# where methods are sealed by default (in Java you can mock most classes and mock out their non-final methods). In my export code I change things so that the Exporter class only depended on abstractions such as the ExportProcessor interface. This decoupled the components and made testing easier.