30 Days of TDD – Day 18 – Refactoring Revisited Pt. 1

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.

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:

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

The means of validating the quantity of each item in the shopping cart

The way sessions are opened with the order fulfillment service

How item inventory levels are checked with the order fulfillment service

The way that an order is placed with the order fulfillment service

How sessions are closed with the order fulfillment service

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.

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.

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):

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:

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

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.

Progress, Telerik, and certain product names used herein are trademarks or registered trademarks of Progress Software Corporation and/or one of its subsidiaries or affiliates in the U.S. and/or other countries. See Trademarks or appropriate markings.