Telerik blogs

If you’ve never done Test Driven Development or aren’t even sure what this "crazy TDD stuff” is all about than this is the series for you. Over the next 30 days this series of posts take you from “I can spell TDD” to being able to consider yourself a “functional” TDD developer. Of course TDD is a very deep topic and truly mastering it will take quite a bit of time, but the rewards are well worth it. Along the way I’ll be showing you how tools like JustCode and JustMock can help you in your practice of TDD.

Previous Posts in this Series:  30 Days of TDD – Day 17 – Specifying Order of Execution in Mocks

If you’ve been following this series you are not doubt familiar with the TDD Store example that we’ve been using to demonstrate the concepts of TDD. In this post we’ll take a break from creating new tests and use our current suite of tests to help us refactor our code to keep is readable and maintainable.


Where We Start

Note: Before this post I updated my unit tests to account for changes made to Place Order in the previous post. You can download the current unit tests here.

Over the past few posts we’ve been fleshing out the PlaceOrder method of the OrderService. For review, the current method looks like this:

 1:  public Guid PlaceOrder(Guid customerId, ShoppingCart shoppingCart)
 2:         {
 3:  foreach (var item in shoppingCart.Items)
 4:             {
 5:  if (item.Quantity == 0)
 6:                 {
 7:  throw new InvalidOrderException();
 8:                 }
 9:             }
 10:  
 11:             var customer = _customerService.GetCustomer(customerId);
 12:  
 13:  //Open Session
 14:             var orderFulfillmentSessionId = _orderFulfillmentService.OpenSession(USERNAME, PASSWORD);
 15:  
 16:             var firstItemId = shoppingCart.Items[0].ItemId;
 17:             var firstItemQuantity = shoppingCart.Items[0].Quantity;
 18:  
 19:  //Check Inventory Level
 20:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 21:  
 22:  //Place Order
 23:             var orderForFulfillmentService = new Dictionary<Guid, int>();
 24:             orderForFulfillmentService.Add(firstItemId, firstItemQuantity);
 25:             var orderPlaced = _orderFulfillmentService.PlaceOrder(orderFulfillmentSessionId, 
 26:                 orderForFulfillmentService, 
 27:                 customer.ShippingAddress.ToString());
 28:  
 29:  //Close Session
 30:             _orderFulfillmentService.CloseSession(orderFulfillmentSessionId);
 31:  
 32:             var order = new Order();
 33:  return _orderDataService.Save(order);
 34:         }

(get sample code)

This method is getting a bit long. We’ve also started to creep into an area where we are violating the Single Responsibility Principal (check out Day Five for a review of SRP). Right now I count six separate reasons for this method to have to change

  1. The means of validating the quantity of each item in the shopping cart
  2. The way sessions are opened with the order fulfillment service
  3. How item inventory levels are checked with the order fulfillment service
  4. The way that an order is placed with the order fulfillment service
  5. How sessions are closed with the order fulfillment service
  6. Creating and saving an order

Of all of those listed, “Creating and saving an order” is the one I am least concerned about now. For one thing, there’s not enough code to worry about at the moment. This will definitely change as we continue to develop this feature, but for now it’s not really on my radar. Item One is also a concern, but right now it’s not the most disturbing violation of SRP, so it can wait.

What I want to focus on right now are items two through five. These all have to do with the order fulfillment service and constitute a large percentage or our current PlaceOrder method. I want to tackle these first.

Developers who are new to the concept of SRP or refactoring in general will probably look at the above list and determine that we need four new methods; one each for items two, three, four and five. That’s not incorrect, but it’s only part of the story. Simply drawing those four methods out will make the code more readable and promote reuse of that code, but it still leaves PlaceOrder with six reasons to change. If we pull those methods out but call them from the PlaceOrder method, we’re still prone to changes that alter the parameter list or method names of those methods. Refactoring and SRP is not just about pulling code out to private methods to make methods shorter, it’s about creating logical abstractions in code that protect me (as much as possible) from changes down the road.

What I see is a large “dealing with the order fulfillment service” process that I want to excise from the PlaceOrder method. Once that’s done I will have one method to call that starts the entire process. This protects me from changes to the individual steps involved in working with the order fulfillment service. So the first step is to extract ALL of the code that deals with the order fulfillment service to a separate private method. Since I’m using JustCode this is easy; I can highlight the hole block of code, and either select “Extract Method” from the JustCode Visual Aid menu (CTRL + `) (figure one) or use the keyboard shortcut (CTRL + R, CTRL + M) and JustCode will extract the code to a new private method.

image

Figure 1 – The Extract Method command in the JustCode Visual Aid menu

JustCode will extract the code for me, including figuring out what parameters I need) and prompt me for a name, which in this case will be PlaceOrdeWithFulfillmentService. It will also replace the original code in the PlaceOrder method with a call to the new method it just created.

 1:  public Guid PlaceOrder(Guid customerId, ShoppingCart shoppingCart)
 2:         {
 3:  foreach (var item in shoppingCart.Items)
 4:             {
 5:  if (item.Quantity == 0)
 6:                 {
 7:  throw new InvalidOrderException();
 8:                 }
 9:             }
 10:  
 11:             var customer = _customerService.GetCustomer(customerId);
 12:  
 13:             PlaceOrderWithFulfillmentService(shoppingCart, customer);
 14:  
 15:             var order = new Order();
 16:  return _orderDataService.Save(order);
 17:         }
 18:  
 19:  private void PlaceOrderWithFulfillmentService(ShoppingCart shoppingCart, Customer customer)
 20:         {
 21:  //Open Session
 22:             var orderFulfillmentSessionId = _orderFulfillmentService.OpenSession(USERNAME, PASSWORD);
 23:  
 24:             var firstItemId = shoppingCart.Items[0].ItemId;
 25:             var firstItemQuantity = shoppingCart.Items[0].Quantity;
 26:  
 27:  //Check Inventory Level
 28:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 29:  
 30:  //Place Orders
 31:             var orderForFulfillmentService = new Dictionary<Guid, int>();
 32:             orderForFulfillmentService.Add(firstItemId, firstItemQuantity);
 33:             var orderPlaced = _orderFulfillmentService.PlaceOrder(orderFulfillmentSessionId,
 34:                 orderForFulfillmentService,
 35:                 customer.ShippingAddress.ToString());
 36:  
 37:  //Close Session
 38:             _orderFulfillmentService.CloseSession(orderFulfillmentSessionId);
 39:         }

(get sample code)

The effect on the PlaceCode method is dramatic; it already much easier to read. I notice that JustCode has declared my new method with a return type of void. That’s because there are not values being created in my extracted code that are being used in other parts of the method. More on that in the next post. Normally I find the void return type to be a bit of a red flag, but if I don’t need a return type at the moment then it’s fine. To verify this, and to verify that my method extraction has not broken my code, I need to run my unit tests (figure 2):

image

Figure 2 – Passing tests

This means my first refactor has worked and, for now, I can leave the return type of PlaceOrderWithFulfillmentService as void. Now let’s turn our refactoring attention to that method.

PlaceOrderWithFulfillmentService has helped make our code quite a bit less brittle, but it has some issues of it’s own. For one we’ve simply moved the four reasons to change out of PlaceOrder and into this method. We need to do a little more refactoring and it is at this point that I would want to pull the individual steps out to their own methods. I’ve gone ahead and done the first one; OpenOrderFuilfillmentService:

 1:  private void PlaceOrderWithFulfillmentService(ShoppingCart shoppingCart, Customer customer)
 2:         {
 3:  //Open Session
 4:             var orderFulfillmentSessionId = OpenOrderFulfillmentSession();
 5:  
 6:             var firstItemId = shoppingCart.Items[0].ItemId;
 7:             var firstItemQuantity = shoppingCart.Items[0].Quantity;
 8:  
 9:  //Check Inventory Level
 10:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 11:  
 12:  //Place Orders
 13:             var orderForFulfillmentService = new Dictionary<Guid, int>();
 14:             orderForFulfillmentService.Add(firstItemId, firstItemQuantity);
 15:             var orderPlaced = _orderFulfillmentService.PlaceOrder(orderFulfillmentSessionId,
 16:                 orderForFulfillmentService,
 17:                 customer.ShippingAddress.ToString());
 18:  
 19:  //Close Session
 20:             _orderFulfillmentService.CloseSession(orderFulfillmentSessionId);
 21:         }
 22:  
 23:  private Guid OpenOrderFulfillmentSession()
 24:         {
 25:             var orderFulfillmentSessionId = _orderFulfillmentService.OpenSession(USERNAME, PASSWORD);
 26:  return orderFulfillmentSessionId;
 27:         }

(get sample code)

Running the tests shows that this refactoring is successful. While I’m at it I’ll also pull the code to close the session out to a separate method. I’ll call this method CloseOrderFulfillmentService:

 1:  private void PlaceOrderWithFulfillmentService(ShoppingCart shoppingCart, Customer customer)
 2:         {
 3:  //Open Session
 4:             var orderFulfillmentSessionId = OpenOrderFulfillmentSession();
 5:  
 6:             var firstItemId = shoppingCart.Items[0].ItemId;
 7:             var firstItemQuantity = shoppingCart.Items[0].Quantity;
 8:  
 9:  //Check Inventory Level
 10:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 11:  
 12:  //Place Orders
 13:             var orderForFulfillmentService = new Dictionary<Guid, int>();
 14:             orderForFulfillmentService.Add(firstItemId, firstItemQuantity);
 15:             var orderPlaced = _orderFulfillmentService.PlaceOrder(orderFulfillmentSessionId,
 16:                 orderForFulfillmentService,
 17:                 customer.ShippingAddress.ToString());
 18:  
 19:  //Close Session
 20:             CloseOrderFulfillmentService(orderFulfillmentSessionId);
 21:         }
 22:  
 23:  private void CloseOrderFulfillmentService(Guid orderFulfillmentSessionId)
 24:         {
 25:  //Close Session
 26:             _orderFulfillmentService.CloseSession(orderFulfillmentSessionId);
 27:         }

 (get sample code)

Running the tests again show’s we’re still dealing with working code (figure 3):

image

Figure 3 – Our tests still pass

The remaining two steps are a little trickier, so we’ll deal with them in the next post.

Summary

It’s important to continually refactor our code. Refactoring not only insures that our code is kept readable and maintainable, it also keeps code from becoming stagnant and can help us find design issues. In the next post we’ll take our refactoring a new level by dealing with methods that return values.

 

Continue the TDD journey:

JustCode download banner image


About the Author

James Bender

is a Developer and has been involved in software development and architecture for almost 20 years. He has built everything from small, single-user applications to Enterprise-scale, multi-user systems. His specialties are .NET development and architecture, TDD, Web Development, cloud computing, and agile development methodologies. James is a Microsoft MVP and the author of two books; "Professional Test Driven Development with C#" which was released in May of 2011 and "Windows 8 Apps with HTML5 and JavaScript" which will be available soon. James has a blog at JamesCBender.com and his Twitter ID is @JamesBender. Google Profile

Comments

Comments are disabled in preview mode.