Refactoring 2: Extract Method
The ‘Extract Method’ is one of the most common refactorings you will ever do. It is also one you will frequently see implemented on the Refactor tool menu on various IDE’s.
What this method basically does is to take a group of related code and convert it to a function with a appropriate name that easily explains the purpose of the code. You can also use ‘Extract Method’ if one of your methods is too long or requires copious amount of comments to explain its purpose.
Take for example the following code from a recent project. The code converts a hotel room price to points depending on the currency. The code by itself doesn’t do much to explain what it does.
Note: In view of the recent constructive comments given by readers I’ve changed the code to more correctly reflect the concept of the Extract method. Its people like you that always keep me alert. Thanks guys.
<?php function getRoomPoints($roomPrice) { /* $forex is an array containing exchange rates */ $forex = array("gbp"=>1.2, "eur"=>.75, "usd"=>1); $_factor = 0; $currency_code = "GBP"; switch($currency_code) { case "GBP" : $_factor = 1/$forex['gbp']; break; case "USD" : $_factor = 1; break; case "EUR" : $_factor = 1/$forex['eur']; break; default : $_factor = 1; break; } return $roomPrice * $_factor; } $room_points = getRoomPoints($_service_hotel['AvailableRoom']['Price']); ?> |
The block of code that does the actual conversion can be extracted and converted into a function with an appropriate name as given below which now makes the intention of the code clear.
<?php function getRoomPoints($roomPrice) { $currency = $_service_hotel['Currency']['code']; $_factor = getConversionRateFactor($currency); return $roomPrice * $_factor; } function getConversionRateFactor($currency_code) { $_factor = 0; $forex = array("gbp"=>1.2, "eur"=>.75, "usd"=>1); switch($currency_code) { case "GBP" : $_factor = 1/$forex['gbp']; break; case "USD" : $_factor = 1; break; case "EUR" : $_factor = 1/$forex['eur']; break; default : $_factor = 1; break; } return $_factor; } $room_points = getRoomPoints($_service_hotel['AvailableRoom']['Price']); ?> |
This site is a digital habitat of Sameer Borate, a freelance web developer working in PHP, MySQL and WordPress. I also provide web scraping services, website design and development and integration of various Open Source API's. Contact me at metapix[at]gmail.com for any new project requirements and price quotes.
Get the latest posts
10 Responses
1
BradGman
January 12th, 2009 at 10:16 pm
Not to be confused with the “true” extract method:
extract($_POST);
Which is the utmost evil thing to leave behind for fellow developers, be warned.
2
Chris
January 13th, 2009 at 1:43 am
In an effort to reduce the code size while being more extensible and – as I think – more readable: http://paste.mootools.net/f8819e01
Just wrapping existing code in a function is not refactoring as this changes the external behaviour (it provides a new function to “the outside” and you have to replace all occurences of currency calculation) – I know it is only an example, but this is not refactoring and more of reworking or so.
Refactoring, or the “Extract Method” you describe would be when you have a method getPrice that retrieves the conversion factor AND calculates the price based on the currency and you want to move the retrieval of the conversion factor to a new function getConversionRateFactor and call it within getPrice. This way you won’t have to change the surrounding code as getPrice still behaves the same. I know your example aims to be basic, but I think it is more clear to what it is about
3
Mark
January 13th, 2009 at 10:25 am
Or you could just do this:
function getConversionRateFactor($currency_code)
{
$forex = array(“gbp”=>1.2, “eur”=>.75, “usd”=>1);
switch($currency_code)
{
case “GBP” : return 1/$forex['gbp'];
case “USD” : return 1;
case “EUR” : return 1/$forex['eur'];
default : return 1;
}
}
4
Les
March 16th, 2009 at 10:55 am
When I came across the title of this post, I truly was expecting a post about Refactoring but this isn’t refactory (see M Fowler book), and certainly this can’t be seen as the Extract Method either.
With all the hype going about best practices, etc you’ve jumped on the bandwagon (ie keyword Refactoring) to get a higher page index which isn’t really the case, as advertised is it?
Disappointed
5
Dave Doolin
June 3rd, 2009 at 12:04 pm
Thanks for writing.
I did learn something from Chris’s comment: that’s not really refactoring.
But it is restructuring, which can really help improve code readability.
Please continue this series, and don’t be afraid to make mistakes. If you’re lucky, people will actually read your articles and catch your mistakes.
Given you came up on the first page on Google, you’re doing the php community a valuable service as few others see fit to write about refactoring. Just make sure you keep it simple enough for non-experts to understand, and as accurate as you know how. Personally, I almost have to write to learn, and I make mistakes along the way. Some people harsh me for mistakes, others point them out so I can learn better.
6
Dave Doolin
June 3rd, 2009 at 12:06 pm
I went to look for more articles and this was the last one!
Please write some more!
sameer
June 3rd, 2009 at 10:49 pm
Thanks for the encouraging thought.
8
Sameer Borate’s Blog: Refactoring 3: Replace Temp with Query | Webs Developer
June 8th, 2009 at 10:09 am
[...] temp variable expressions with methods. This method is often also required before you use the Extract Method [...]
9
Arne
June 8th, 2009 at 1:01 pm
I don’t really understand why you use a switch in this example, I wouldn’t use it in this case.
In my opinion this is more dynamic:
if (isset($forex[strtolower($currency_code)])) {
$_factor = 1 / $forex[strtolower($currency_code)];
}
else {
$_factor = 1;
}
return $_factor;
Or just in 1 line:
return isset($forex[strtolower($currency_code)]) ? 1 / $forex[strtolower($currency_code)] : 1;
You can even drop the strtolower() if you change the keys in $forex to capitals.
sameer
June 8th, 2009 at 8:57 pm
We can make the code as terse as we like, but as I had created the code for one of my clients (they are developers) I choose clear expression over brevity.