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 19 – Refactoring Revisited Pt. 2

In the previous post we started refactoring our code to make sure we were complying with the SRP. Using our tests we are able to optimize our code for readability and maintainability. We’ll continue in the post by examining how the interactions between the code we want to extract to methods needs to be taken into account when refactoring. 

Almost There!

Over the last two posts we’ve spent a good bit of time refactoring the PlaceOrder method to make it more readable, maintainable and more in line with SRP. We’ve accomplished this by extracting the logic that interacts with the OrderFulfillment service to other methods so that PlaceOrder does not have to know the specific details of the OrderFulfillment process or API. Sometimes this process is referred to as “abstracting away” the functionality from one place to another.

We pulled all the order fulfillment logic out of PlaceOrder and put it a method called PlaceOrderWithFulfillmentService method, which in turn has been refactored to have the individual tasks for working with the OrderFulfillment service pulled out. Our code currently looks like this:

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

(get sample code)

The code that opens and closes the session with the order fulfillment service has been pulled out to specific methods. In the last post we pulled the two steps that are used to actually place an order into a separate method, which was also called PlaceOrderWithFulfillmentService, but takes the orderFulfillmentSessionId as a parameter (line 12). The reasons for this were explained in the previous post. This is pretty good and the code is, for the most part, starting to look better. But I really think the new version of PlaceOrderWithFulfillmentService (line 12) can use a bit more work.

Note: For the remainder of this post, unless otherwise specified, when I refer to the PlaceOrderWithFulfillmentService method I am referring to the version that takes three arguments and starts on line 12.

The PlaceOrderWithFulfillmentService method encapsulates two steps in the order process; verifying that the items in the order are in stock and then actually ordering them. These two steps together are collectively known as “placing an order” but they are separate and distinct steps in that process. In an effort to increase the maintainability of the code and our constant pursuit of SRP, it makes sense to abstract both of these steps out to separate methods.

This is the current state of the PlaceOrderWithFulfillmentService method:

 1:  private void PlaceOrderWithFulfillmentService(Guid orderFulfillmentSessionId, ShoppingCart shoppingCart, Customer customer)
 2:         {
 3:             var firstItemId = shoppingCart.Items[0].ItemId;
 4:             var firstItemQuantity = shoppingCart.Items[0].Quantity;
 5:  
 6:  //Check Inventory Level
 7:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 8:  
 9:  //Place Orders
 10:             var orderForFulfillmentService = new Dictionary<Guid, int>();
 11:             orderForFulfillmentService.Add(firstItemId, firstItemQuantity);
 12:             var orderPlaced = _orderFulfillmentService.PlaceOrder(orderFulfillmentSessionId,
 13:                 orderForFulfillmentService,
 14:                 customer.ShippingAddress.ToString());
 15:         }

(get sample code)

The automated refactorings in JustCode have made pulling code out to new methods easy. Looking at this code one might be inclined to highlight lines three through seven and invoke the “Extract Method” command in JustCode. Doing so would result in the following method being created:

 1:  private void CheckInventoryLevels(ShoppingCart shoppingCart, Guid orderFulfillmentSessionId, out Guid firstItemId, out int firstItemQuantity)
 2:         {
 3:             firstItemId = shoppingCart.Items[0].ItemId;
 4:             firstItemQuantity = shoppingCart.Items[0].Quantity;
 5:  
 6:  //Check Inventory Level
 7:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 8:         }

(get sample code)

Whoa! That’s not what I wanted. I was expecting something that would (roughly) return the results of IsInInventory. Now I have a method with a void return type and two “out” parameters. This method isn’t full of code smells; it is a code smell!

So what went wrong? What happened was we failed to recognize the call to IsInInventory is not the end of this inventory verification process. It’s really the middle of it. The IsInInventory method returns a Boolean that tells is if the item we are looking for is in stock. We don’t have a test that uses that value right now, so we’re pretty much just ignoring it and we’re assuming that the item is in inventory because the tests we have (at the moment) only test for that. In reality the inventory verification process should also include the code on lines ten and 11. The code on these lines are creating and populating the dictionary that gets passed to the order fulfillment service’s PlaceOrder method. The functionality is obfuscated a bit; we’re assuming there is only one element in the shopping cart and we’re not paying attention to the result of the IsInInventory method. We don’t have tests that expose these issues (yet). But the workflow of our code does not support having IsInInventory be the last step of the inventory verification process; the addition of the item to the orderForFulfillmentService is. Our refactoring effort needs to recognize this.

I’ll undo the previous refactor and this time I’ll select the code from line three through 11 and execute the “Extract Method” command. This time the method gets created a little close to what we are looking for:

 1:  private Dictionary<Guid, int> CheckInventoryLevels(ShoppingCart shoppingCart, Guid orderFulfillmentSessionId)
 2:         {
 3:             var firstItemId = shoppingCart.Items[0].ItemId;
 4:             var firstItemQuantity = shoppingCart.Items[0].Quantity;
 5:  
 6:  //Check Inventory Level
 7:             var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity);
 8:  
 9:             var orderForFulfillmentService = new Dictionary<Guid, int>();
 10:             orderForFulfillmentService.Add(firstItemId, firstItemQuantity);
 11:  return orderForFulfillmentService;
 12:         }

(get sample code)

This is much better! I have an actual return type and no “out” parameters. It also does a very good job of encapsulating all the steps needed for verifying the inventory levels in the order fulfillment system. Running the tests (and really, this should be habit by now!) shows the my code changes have not stopped my code from working (figure 1)

image

Figure 1 – Passing tests

Summary

I hope this short workshop on refactoring. Refactoring is a powerful tool that can help make your code easier to read and maintain. By keeping the SRP in mind when refactoring, you can also enhance code reuse.

 

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.