Monday, January 30, 2012

Repositories and Single Responsibility from the Trenches

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.

Tuesday, December 20, 2011

Slides from ANUG talk on Nancy

I did a talk on Nancy a week ago in the Aarhus -NET User Group. And in my own humble and unbiased opinion it went quite well :-) So I thought I'd share the slides with you - although they probably only make semi-sense without the talk to go along. Anyway here they are; enjoy.


Monday, December 5, 2011

Announcing: XmlUnit.Xunit


I wanted to announce a small library I have on my GitHub: XmlUnit.Xunit, which a library for testing against XML. It's a port of XmlUnit over to use Xunit.NET assertions instead of NUnit assertions, plus an addition of a fluent assertion style. The following is the projects README, and gives you a feel for the purpose of the project. Enjoy.

Intro

XmlUnit.Xunit provides Xunit.NET based assertions tailored to testing XML. The project contains assertions for comparing XML, applying XPath and XSLT expression to XML under test. All assertions come in two flavors: A traditional set of static assertion methods, and a fluent "should" style assertions.


Usage
You grab the source from here, download a build from here or install the NuGet package.

Beware that both the binary in the download tab and the NuGet package may give you assembly conflicts on Xunit.NET. To get around that you need an assmebly rebind of Xunit.NET.

Traditional Assertions
The traditional assertions in XmlUnit.Xunit er all static methods on the class XmlAssertion, and are used like this:

  404 [Fact]
  405 public void AssertStringEqualAndIdenticalToSelf()
  406 {
  407     string control = "<assert>true</assert>";
  408     string test = "<assert>true</assert>";
  409     XmlAssertion.AssertXmlIdentical(control, test);
  410     XmlAssertion.AssertXmlEquals(control, test);
  411 }
  412 
  413 private static readonly string MY_SOLAR_SYSTEM =
  414     "<solar-system><planet name='Earth' position='3' supportsLife='yes'/><planet name='Venus' position='4'/></solar-system>";
  415 
  416 [Fact]
  417 public void AssertXPathExistsWorksForExistentXPath()
  418 {
  419     XmlAssertion.AssertXPathExists("//planet[@name='Earth']",
  420                                    MY_SOLAR_SYSTEM);
  421 }
  422 
  423 [Fact]
  424 public void AssertXPathEvaluatesToWorksForMatchingExpression()
  425 {
  426     XmlAssertion.AssertXPathEvaluatesTo("//planet[@position='3']/@supportsLife",
  427                                         MY_SOLAR_SYSTEM,
  428                                         "yes");
  429 }
  430 
  431 [Fact]
  432 public void AssertXslTransformResultsWorksWithStrings()
  433 {
  434     string xslt = XsltTests.IDENTITY_TRANSFORM;
  435     string someXml = "<a><b>c</b><b/></a>";
  436     XmlAssertion.AssertXslTransformResults(xslt, someXml, someXml);
  437 }
  438 


Fluent Assertions
The fluent assertions are all extension methods with names starting with Should, and are used like this:

  404 [Fact]
  405 public void AssertStringEqualAndIdenticalToSelf()
  406 {
  407     string control = "<assert>true</assert>";
  408     string test = "<assert>true</assert>";
  409     test.ShouldBeXmlIdenticalTo(control);
  410     test.ShouldBeXmlEqualTo(control);
  411 }
  412 
  413 private static readonly string MY_SOLAR_SYSTEM =
  414     "<solar-system><planet name='Earth' position='3' supportsLife='yes'/><planet name='Venus' position='4'/></solar-system>";
  415 
  416 [Fact]
  417 public void AssertXPathExestsWorksForXmlInput()
  418 {
  419     new XmlInput(MY_SOLAR_SYSTEM)
  420         .XPath("//planet[@name='Earth']")
  421         .ShouldExist();
  422 }
  423 
  424 [Fact]
  425 public void AssertXPathEvaluatesToWorksForMatchingExpression()
  426 {
  427     MY_SOLAR_SYSTEM
  428         .XPath("//planet[@position='3']/@supportsLife")
  429         .ShouldEvaluateTo("yes");
  430 }
  431 
  432 [Fact]
  433 public void AssertXPathExistsWorksWithXpathFirstWithXmlInput()
  434 {
  435     var sut = new XmlInput(MY_SOLAR_SYSTEM);
  436 
  437     "//planet[@name='Earth']".AppliedTo(sut).ShouldExist();
  438 }
  439 
  440 [Fact]
  441 public void AssertXPathEvaluatesToWorksWithXPathFirst()
  442 {
  443     "//planet[@position='3']/@supportsLife"
  444         .AppliedTo(MY_SOLAR_SYSTEM)
  445         .ShouldEvaluateTo("yes");
  446 }
  447 
  448 [Fact]
  449 public void AssertXslTransformResultsWorksWithStrings()
  450 {
  451     string xslt = XsltTests.IDENTITY_TRANSFORM;
  452     string someXml = "<a><b>c</b><b/></a>";
  453 
  454     someXml.XsltTransformation(xslt).ShouldResultIn(someXml);
  455 }

Further Information
Is probably best gleened off the tests in this project, especially the tests for XmlAssertions and the tests for Should assertions.

Contribute
Please do! Fork, code, send pull request. :-)