There is a good article on API Guidelines at the “Do not set yourself on fire” blog. Must reading for anybody that is doing API design work.
Single Responsibility Principal, Refactoring, and API Design:
As Bjorn mentioned there is a good discussion going on about API Design and bloat. Some of this is due to the number of internal API that people can’t access, but in many cases that’s only part of the problem. Pieces of eclipse in general need an overall Refactoring of their code base. There are classes that aren’t following the Single Responsibility Principle (SRP). They have multiple reasons to change, they are doing more than one thing. Part of designing a reusable set of APIs is the SRP. If a class has one clear duty and a method has one clear duty, then it makes it much easier to resuse that functionality elsewhere in the system. It also makes it MUCH easier to maintain the system. There was discussion in the thread that Bjorn linked too, about how API needs to be clear so that it can be used effectively. Following the SRP will greatly help in that effort.
I’ll give an example from XSL Tools where I did the refactoring to take the XSL Content Assistance and refactor it. The original implementation handled all the various types of content assistance.
- Select and Test XPath content assistance.
- Template content assistance for match and name attributes
- Content Assistance for non XSLT attributes
- Content assistance for general XML (through extending and overriding XML Content Assist Processor).
If I wanted to add aditional content assistance I had to start making all sorts of small changes in lots of different methods in this one class.
- Overriding methods.
- Copying code from the Base class that was private but I needed to access it from lower class.
- Touching multiple methods and functions to add additonal functionality, thus making the existing class more fragile and bug prone.
The class was doing way to much and had too many reasons to be changed.
Through several rounds of refactoring, the design changed…the XSLContentAssistProcessor class is just responsible for returning the available proposals. It doesn’t figure out what proposals need to be returned and when. This is now handled by an XLContentAssistRequestFactory. This makes the determination of which polymorphic instance of the IContentAssistRequest interface needs to be returned. So now for any new content assist request, it is maintained in it’s own class. Each class is responsible for one specific purpose of content assist. For SelectTest attribute assistance, that is it’s own class. For Call-Template assistance that is it’s own class. Any new content assistance get it’s own class that handles that particular task.
I’m hoping that the e4 incubator project is taking this to heart, as code bloat is much more than just the public API’s that are available, it is also about the overall design of the system as a whole.
Editors Note: Corrected credit on the original posting, should have been Bjorn not Boris even though he participated in the thread.