In a project I'm involved with we've done what I suspect lots and lots of projects do. We've used the repository pattern to encapsulate database adds, queries and deletes. We have one such repository per entity type. That worked well for me in earlier projects. But in this case several of the repositories have become somewhat unfocused. None of them are big hairy monsters, but the different public methods really don't have much in common except the fact that they mainly act on the same entity type. In other words cohesion is low. Let me exemplify (anonymized just as the rest of the code in this post):
9 public interface IDeviceRepository
10 {
11 TDeviceType Get<TDeviceType>(long id) where TDeviceType : Device;
12 TDeviceType FindByControllerIdentifier<TDeviceType>(string controllerIdentifer)
13 where TDeviceType : Device;
14
15 IList<TDeviceType> GetAll<TDeviceType>(int offset, int max,
16 DateTimeOffset? createdAfter = null, DateTimeOffset? updatedAfter = null,
17 DateTimeOffset? disconnectedAfter = null)
18 where TDeviceType : Device;
19
20 int Count<TDeviceType>(DateTimeOffset? createdAfter = null,
21 DateTimeOffset? updatedAfter = null, DateTimeOffset? disconnectedAfter = null)
22 where TDeviceType: Device;
23
24 void Add<TDeviceType>(TDeviceType device) where TDeviceType : Device;
25 IList<TDeviceType> GetAll<TDeviceType>() where TDeviceType : Device;
26 IList<TDeviceType> GetAll<TDeviceType>(long[] deviceIds) where TDeviceType : Device;
27 IList<TDeviceType> FindByControllerIdentifiers<TDeviceType>(string[] controllerIdentifiers)
28 where TDeviceType : Device;
29 IList<NodeDevice> FindNodesDevicesForRootDevice(string controllerIdentifier,
30 bool active = false);
31 ICollection<NodeDevice> FindNodeDevicesForRootDevice(long rootDeviceId, bool active = false);
32
33 IEnumerable<DeviceLink> GetTopology<T>(long id, bool active) where T : Device;
34 DeviceLink GetDeviceLink(string controllerIdentifierA, string controllerIdentifierB);
35 DeviceLink GetDeviceLink(long id);
36
37 void AddDeviceLink(DeviceLink link);
38
39 IList<Route> FindRoutesByDeviceLink(DeviceLink deviceLink);
40 void Delete(Device device);
41 }
This doesn't look like the interface of a class that has only one reason to change - i.e. it violates the single responsibility principle (SRP). This is in part because it in fact does not only act on one entity type but on a hierarchy of entity types; device is a base class and the are a handful of concrete types of devices. The obvious solution to that, is to move away from a repository for the base class to a number of repositories for the concrete entities - but that's where we came from. That's a route that led to duplicated code, which led to abstracting into a common device repository, so we did not want to go back down that route again.
Taking Single Responsibility Seriously
The solution we've come up with is - at it's core - simply to take SRP seriously: We want classes that do not do a bunch of database related operations around a type of enity. We want classes that does just one such operation. Take the Delete method on the interface above. That seems innocent enough, but it turns out that the implemetation is a bit complicated: There are a number of relations that need to be untied, and there is some cascading to take care of before the device can be deleted from the database. As a colleague commented: "It looks like a bad stored proc, written in C#". We decided to view that delete operation as "one thing", and decided it justified a class of its own:
11 public class DeleteDeviceRequest : NHibernateWriteRequest
12 {
13 private readonly Device device;
14
15 public DeleteDeviceRequest(Device device)
16 {
17 this.device = device;
18 }
19
20 protected override void DoExecute()
21 {
22 var links =
23 Session.QueryOver<DeviceLinkReference>().Where(x => x.ControllerIdentifier == device.ControllerIdentifier).
24 List();
25
26 links.ForEach(x => x.Device = null);
27
28 DeleteStatistics();
29 new DeleteRoutesWriteRequest(device.Routes).ExecuteWith(DataStore);
30 DeleteFromGroup();
31 DeleteFromOrder();
32 DeleteProfiles();
33 DataStore.Delete(device);
34 }
35
36 // more private methods...
37
38
The only thing public here is the constructor. The contructor takes all arguments for the delete, so once the object is constructed it completely incapsulates the delete. And - importantly - it is incapable of doing anything else than that particular delete.
How is the delete executed? -Through an execute method on the base class NHibernateWriteRequest.
This swaps the repository pattern for the command pattern plus the template method pattern. But more importantly this swaps a small collection of ever growing repositories for a large collection of small never growing query classes (which, btw, also plays better into the open/closed principle). We've only just started down this path, but I think we'll be happy about it.
There are a few twists I've left out, particularly around making these request objects easy to get at for client code, while maintaining encapsulation and testability. I might revisit those in a future post.
