A safe refactoring: extracting a method

Sometimes you have some code that is duplicated in multiple places. One approach to getting rid of this would be to extract it into a method. In this article I’ll show you two different approaches you can take to do this and when I would use each one.

Setting the scene
I’m going to use the example of some tests for the checkout kata. Let’s say we’ve reached the following code

class CheckoutTests < Test::Unit::TestCase

	def test_scanning_one_a
		prices = {'A':50, 'B': 70}
		checkout = Checkout.new(prices)
		checkout.scan('A')
		assert_equal(50, checkout.total)
	end

	def test_scanning_multiple_items
		prices = {'A':50, 'B': 70}
		checkout = Checkout.new(prices)
		checkout.scan('A')
		checkout.scan('B')
		assert_equal(120, checkout.total)
	end
end

We have some duplication in the code where we set up our Checkout.

prices = {'A':50, 'B': 70}
checkout = Checkout.new(prices)

Basic method extraction

So let’s start off by adding a method which returns our Checkout. This is basically Martin Fowler’s Extract Method refactoring. We’ll just do this in one of the tests to start off with to make sure we aren’t making to big a change in one go.

	def new_checkout
		prices = {'A':50, 'B': 70}
		Checkout.new(prices)
	end

	def test_scanning_one_a
		checkout = new_checkout
		checkout.scan('A')
		assert_equal(50, checkout.total)
	end

For me this feels like quite a jump so lets take a step back and see if we can break this down into a smaller step.

We can use (or abuse) the fact that you can call a method in ruby without brackets to our advantage here. If we first assign our checkout to a variable called

new_checkout

.

        def test_scanning_one_a
		prices = {'A':50, 'B':70}
		new_checkout = Checkout.new(prices)
		checkout = new_checkout
		checkout.scan('A')
		assert_equal(50, checkout.total)
	end

Then we can move out the instantiation of our checkout into a method called

new_checkout

.

        def new_checkout
		prices = {'A':50, 'B':70}
		Checkout.new(prices)
	end

	def test_scanning_one_a
		checkout = new_checkout
		checkout.scan('A')
		assert_equal(50, checkout.total)
	end

This is only a slightly smaller step, but as we are running our tests after each change it can give us more confidence that what we are doing is working, and if it’s not then we can press undo (or git stash) and try the refactor again from the last point where the tests passed.

We can now apply the same to our other test method.

class CheckoutTests < Test::Unit::TestCase

	def new_checkout
		prices = {'A':50, 'B':70}
		Checkout.new(prices)
	end

	def test_scanning_one_a
		checkout = new_checkout
		checkout.scan('A')
		assert_equal(50, checkout.total)
	end

	def test_scanning_multiple_items
		checkout = new_checkout
		checkout.scan('A')
		checkout.scan('B')
		assert_equal(120, checkout.total)
	end
end

Extract to method using fields
Let’s go back to our original test before we started refactoring.

class CheckoutTests < Test::Unit::TestCase

	def test_scanning_one_a
		prices = {'A':50, 'B': 70}
		checkout = Checkout.new(prices)
		checkout.scan('A')
		assert_equal(50, checkout.total)
	end

	def test_scanning_multiple_items
		prices = {'A':50, 'B': 70}
		checkout = Checkout.new(prices)
		checkout.scan('A')
		checkout.scan('B')
		assert_equal(120, checkout.total)
	end
end

This time we will try and break this down into steps in a different way. We’ll start by assigning our checkout to a field.

        def test_scanning_one_a
		prices = {'A':50, 'B': 70}
		@checkout = Checkout.new(prices)
		@checkout.scan('A')
		assert_equal(50, @checkout.total)
	end

Now we’ll move the setup into a different method called

setup_checkout

.

        def setup_checkout
		prices = {'A':50, 'B':70}
		@checkout = Checkout.new(prices)
	end

	def test_scanning_one_a
                setup_checkout
		@checkout.scan('A')
		assert_equal(50, @checkout.total)
	end

This has required less changes in our existing code so far. We can now use our

setup_checkout

method in our second test by simply assigning our checkout to a field again and then removing the first two lines. This would leave us with something like:

class CheckoutTests < Test::Unit::TestCase

	def setup_checkout
		prices = {'A':50, 'B': 70}
		@checkout = Checkout.new(prices)
	end

	def test_scanning_one_a
		setup_checkout
		@checkout.scan('A')
		assert_equal(50, @checkout.total)
	end

	def test_scanning_multiple_items
		setup_checkout
		@checkout.scan('A')
		@checkout.scan('B')
		assert_equal(120, @checkout.total)
	end
end

In fact this works quite well as a test setup so we can rename our

setup_checkout

method to

setup

and removing the calls to it so that it is run before each test by the test framework. This leaves us with:


class CheckoutTests < Test::Unit::TestCase

	def setup
		prices = {'A':50, 'B': 70}
		@checkout = Checkout.new(prices)
	end

	def test_scanning_one_a
		@checkout.scan('A')
		assert_equal(50, @checkout.total)
	end

	def test_scanning_multiple_items
		@checkout.scan('A')
		@checkout.scan('B')
		assert_equal(120, @checkout.total)
	end
end

Summary
We’ve looked at two approaches to doing this refactoring. Firstly we extracted a method to return a new checkout. I would probably use this if I was wanting to abstract away something complicated or dirty that would hinder readability.

The second approach where we used assigned our checkout to a field on the object and used that everywhere instead. In the example we used I would probably prefer this approach. We’ve moved the repeated algorithm (setting up the checkout) to state, which in this case I think is simpler and more readable.

While this has been a very simple example, practicing refactorings in this situation means that we have more options in our toolkit when we are working on real code.

Hopefully you can see how taking small steps allows us to evaluate why we have chosen to use particular refactorings and help you think using about different approaches.

This entry was posted in Uncategorized. Bookmark the permalink.

Leave a comment