Refactoring towards a DRY, fluent interface

But I (Jesus) say to you, love your enemies, bless those who curse you, do good to those who hate you, and pray for those who spitefully use you and persecute you — Matthew 5:44

Problem

In my current project, I have an application service that is used to make various modifications to a shopping cart, like adding cart items, applying discounts, changing quantities, etc.  Well as this service class evolved, the methods were starting to look like this…

   1: public void ApplyDiscountToShoppingCart(decimal discount)

   2: {

   3:     IShoppingCart shoppingCart = shoppingCartRepository.GetCurrentShoppingCart();

   4:     shoppingCart.ApplyDiscountOf(discount);

   5:  

   6:     shoppingCartRepository.Save(shoppingCart);

   7: }

   8:  

   9: public void AddToCartUsing(string productId, int quantityToAdd)

  10: {

  11:     IShoppingCart shoppingCart = shoppingCartRepository.GetCurrentShoppingCart();

  12:     shoppingCart.AddItemToCart(cartItemFactory.CreateFrom(productId, quantityToAdd));

  13:  

  14:     shoppingCartRepository.Save(shoppingCart);

  15: }

  16:  

  17: public void ChangeQuantityForCartItem(Guid cartItemId, int newQuantity)

  18: {

  19:     IShoppingCart shoppingCart = shoppingCartRepository.GetCurrentShoppingCart();

  20:     shoppingCart.ChangeQuantityForItem(cartItemId, newQuantity);

  21:  

  22:     shoppingCartRepository.Save(shoppingCart);

  23: }

Notice the amount of duplication going on here?  We always need to retrieve the current shopping cart, perform some action on it and save it.  Smells like it’s time for some refactoring.

(One) Resolution 

Important to note, that I’ve already fleshed out my application service class via TDD, with all unit tests currently passing.  So this refactoring is simply to remove duplication, and therefore improve the design (as opposed to a Test-Driven Refactoring which is often a very good way of making changes and/or adding new features). 

By definition, refactoring is the act of “altering the internal structure without changing the external behavior” through a series of “behavior preserving transformations”.

So let’s get to it.  First off, I pick a method to start with; let’s choose ApplyDiscountToShoppingCart.  Just like when practicing TDD, I’m going to write out the syntax that I would like to be able to use.  Then, implement it.

   1: public void ApplyDiscountToShoppingCart(decimal discount)

   2: {

   3:     Within.ShoppingCartSaveScope(shoppingCartRepository).Do(delegate(IShoppingCart shoppingCart) { shoppingCart.ApplyDiscountOf(discount); });

   4: }

Ok, so that looks ok (although that anonymous delegate may need some more work, more on that in a sec).  Readability is pretty important to me, so basically what I’m saying here is that Within a ShoppingCartSaveScope, Do the action that I tell you to do.

First we’ll need to create this thing called Within.  This will be the entry point to initiate the operation.  So I’ll just make it a nested private class for now.

   1: private class With

   2: {

   3:     public static ShoppingCartSaveScope ShoppingCartSaveScope(IShoppingCartRepository shoppingCartRepository)

   4:     {

   5:         return new ShoppingCartSaveScope(shoppingCartRepository);

   6:     }

   7: }

Ok, pretty simple.  Just one problem.  We don’t have this ShoppingCartSaveScope class yet.  So let’s create it also as a nested private class (using ReSharper in all of this of course…).

   1: private class ShoppingCartSaveScope

   2: {

   3:     private readonly IShoppingCartRepository shoppingCartRepository;

   4:  

   5:     public ShoppingCartSaveScope(IShoppingCartRepository shoppingCartRepository)

   6:     {

   7:         this.shoppingCartRepository = shoppingCartRepository;

   8:     }

   9: }

Looks good.  Notice we’ll have to pass it an instance of our IShoppingCartRepository which is currently being injected into our service class from an IoC container.  So back up to our original method.

   1: public void ApplyDiscountToShoppingCart(decimal discount)

   2: {

   3:     Within.ShoppingCartSaveScope(shoppingCartRepository).Do(delegate(IShoppingCart shoppingCart) { shoppingCart.ApplyDiscountOf(discount); });

   4: }

Now we have to deal with this Do method.  The method name is probably influenced by the time I’ve been spending writing Ruby lately.  So basically this is taking advantage of the wonderful world of “blocks”.  I’m sure most of us know that in C# we can pass blocks of code as method arguments using an anonymous delegate.  But most of the time, folks are only utilizing this powerful feature doing things like List.FindAll() or similar.  So what do we do when we need to write a method that _consumes_ a code “block”?  Well, you have a couple options. 

Predicate is what is used in most of the List methods, like Find, RemoveAll, etc.  The point of a Predicate is to return a boolean value so that the caller can determine what to do based on the result. 

Action on the other hand is simply a “fire and forget it” kinda thing.  It acts the same as the Predicate, with the exception of returning a value. 

For now we’re not going to worry about returning a boolean, so we’ll just go with Action.  So remember our **Within** class returned us back an instance of **ShoppingCartSaveScope**.  That’s where we’ll need to implement our **Do** method.

   1: private class ShoppingCartSaveScope

   2: {

   3:     private readonly IShoppingCartRepository shoppingCartRepository;

   4:  

   5:     public ShoppingCartSaveScope(IShoppingCartRepository shoppingCartRepository)

   6:     {

   7:         this.shoppingCartRepository = shoppingCartRepository;

   8:     }

   9:  

  10:     public void Do(Action<IShoppingCart> actionUsingShoppingCart)

  11:     {

  12:         IShoppingCart shoppingCart = shoppingCartRepository.GetCurrentShoppingCart();

  13:         actionUsingShoppingCart(shoppingCart);

  14:  

  15:         shoppingCartRepository.Save(shoppingCart);

  16:     }

  17: }

Notice our Do method simply takes an Action which is just some code block that does some action on an instance of a shopping cart.  Here’s where the removal of duplication happens.  Previously, we had to both retrieve and save the shopping cart in every single service method.  Now, we can put this in a single place, where it can be re-used by any operations that need to perform some series of actions on a shopping cart and then save.

Don’t Like In-Line Anonymous Delegates?

I’m usually fine with using anonymous delegates in-line, but this is one instance where I felt something like this would be much more readable.

   1: public void ApplyDiscountToShoppingCart(decimal discount)

   2: {

   3:     Within.ShoppingCartSaveScope(shoppingCartRepository).Do(ApplyDiscountToShoppingCartCommand.Using(discount));

   4: }

This could simply be implemented by extracting out the anonymous delegate into its own method.  I’m using another private class for readability’s sake, but you could just use a private method as well.

   1: private class ApplyDiscountToShoppingCartCommand

   2: {

   3:     public static Action<IShoppingCart> Using(decimal discount)

   4:     {

   5:         return delegate(IShoppingCart shoppingCart) { shoppingCart.ApplyDiscountOf(discount); };

   6:     }

   7: }

So now, just rinse and repeat with the rest of the service methods as necessary.

Any Of This Look Familiar?

Ok, so this whole ShoppingCartSaveScope deal is basically a very lightweight implementation of the Unit of Work pattern (without change tracking and all the other stuff you normally would need).  But this is all we need so far.  And it keeps our code DRY, while at the same time providing us with a decent fluent interface that we can use.

Better Solutions?

Again, this is just merely one of many ways we could have done this.  In fact, I’m thinking that it could be done even easier using Aspects (AOP), but I’ll have to save (and learn) that for another time.

Feedback?