We had a problem in RemoteX applications, not a big one, but an old structure that had to be refactored. Our open method only accepted URIs as arguments. This forced most of our commands that create new entities, to first save an empty new version of the entity to make it accessible by using a URI for it.
This in turn caused unnecessary round trips to the data access layer, but also caused problems with potentially empty data.
So this Wednesday this code died. In the old implementation; the general OpenView command accepted a URI. Now it can both take a URI but also an actual object.
To allow this I had to change the targeted end-point urls to activate a class instead of a delegate, as it was before. This has two great benefits.
- By using classes I open up for taking advantage of Castle for setting up the different code for opening. Previously all the setup was made in one big wiring class, resulting in excessive class coupling CA warnings on that class.
- Each class that opens an entity view conforms to SRP. It has one purpose, to open a view for a specific entity.
As I implemented the ViewOpener I noticed that all the delegates for opening entities had one thing in common. The first thing they did was to open the entity from the data storage.
By breaking out this behaviour into an abstract base class I could reduce the amount of code, simply by having the base class fetch the entity and then let the more specific view opener setup the UI.
This resulted in the structure illustrated below.
So what are the insights of this refactoring?
Having delegates for a specified behaviour is a code smell. By having simple short classes with a single behaviour instead, we make code easier to refactor. Its easier to simply add another simple class to the Castle wire up, than it is to implement the same behaviour in a large complex class that handles all UI setup.