class: center, middle
-
What is refactoring? Why do we refactor? --
-
Common refactoring techniques
--
-
Software left alone will decay --
-
A change made to the internal structure of software to make it:
--
1. Easier to understand
--
2. Cheaper to make future changes
--
-
This does not change the observable behavior of the software --
-
Improves the design of the code --
-
Tests help ensure that we didn't change behavior
--
-
Change names to communicate intent --
-
No magic numbers / variables --
-
One responsibility per function / class --
-
Do not be obsessed with primitives
class: center, middle
--
$a = 'Michael';
$b = 'Cheng';
printf('Hello, my name is %s %s', $a, $b);
--
*$first_name = 'Michael';
*$last_name = 'Cheng';
printf('Hello, my name is %s %s', $first_name, $last_name);
--
function foo($first_name, $last_name) {
return sprintf('Hello, my name is %s %s', $first_name, $last_name);
}
echo foo('Luke', 'Skywalker');
--
*function greetings_from($first_name, $last_name) {
return sprintf('Hello, my name is %s %s', $first_name, $last_name);
}
echo greetings_from('Luke', 'Skywalker');
class: center, middle
--
function amount_in_usd($amount) {
return $amount * 1.4;
}
echo amount_in_usd(100);
--
Bring out the meaning with a variable or a constant.
*define('USD_EXCHANGE_RATE', 1.4);
function amount_in_usd($amount) {
return $amount * USD_EXCHANGE_RATE;
}
echo amount_in_usd(100);
class Currency {
* const USD_EXCHANGE_RATE = 1.4;
public static function in_usd($amount) {
return $amount * self::USD_EXCHANGE_RATE;
}
}
echo Currency::in_usd(100);
class: center, middle
name: technique3_header1
template: technique3_header1
You are a software guy at a country club and you maintain a register of club members in CSV
format:
--
Michael,Cheng,[email protected]
Luke,Skywalker,[email protected]
template: technique3_header1
"I want to add a new member to the end of the file. If the email is already in the list, ignore the new entry."
You quickly whip together a proof-of-concept PHP function to meet the above requirement:
function add_member($new_member){
$fp = fopen('members.csv', 'r+');
while (($data = fgetcsv($fp)) !== FALSE) {
if ($data[2] == $new_member[2]) {
fclose($fp);
return false;
}
}
fputcsv($fp, $new_member);
fclose($fp);
}
$new_member = ['Michael', 'Cheng', '[email protected]'];
var_dump(add_member($new_member));
class: center, middle
--
- How many things is
add_member
doing?
--
1. Opens CSV file
3. Ignores if its already there
-
Is it too much? Ideally, each function should only have 1 responsibility. --
-
And this code isn't very readable - you might come back in 2 months and forget what the code means.
--
-
Abstract away implementation details into separate functions. --
-
Ask your self: "What if..." questions.
--
-
Opens CSV fileGet List of members -- -
Checks
for duplicate emailif member is already on the list -- -
Ignores if its already there(this is result of 2) -- -
Adds
it in if its notnew member to the list
name: refactor_separate_functions
template: refactor_separate_functions
function get_members() {
$fp = fopen('members.csv', 'r');
$members = [];
while (($values = fgetcsv($fp)) !== FALSE) {
array_push($members, $values);
}
fclose($fp);
return $members;
}
template: refactor_separate_functions
function is_member_in_list($members, $new_member){
foreach($members as $member) {
if ($member[2] == $new_member[2]) // Email field
return true;
}
return false;
}
template: refactor_separate_functions
function add_member_to_file($user) {
$fp = fopen('members.csv', 'a');
fputcsv($fp, $user);
fclose($fp);
}
function add_member($new_member){
$current_members = get_members();
if (is_member_in_list($current_members, $new_member))
return false;
add_member_to_file($new_member);
return true;
}
$new_member = ['Michael', 'Cheng', '[email protected]']
var_dump(add_member($new_member));
--
.center.emoji[ πππ ]
class: center, middle
--
"I want this to also check that the name of the user is not in the list and ignore that too."
.center.emoji[ π£π£π£ ]
--
People with same name but different email addresses are also being added to the list. You have to help him prevent this in the future.
--
- get_members()
- is_member_in_list($members, $new_member)
- add_member_to_file($user)
- add_member($new_member)
--
- get_members()
is_member_in_list($members, $new_member)
- add_member_to_file($user)
- add_member($new_member)
function add_member($new_member){
$current_members = get_members();
* if (is_member_in_list($current_members, $new_member))
return false;
add_member_to_file($new_member);
return true;
}
The original is_member_in_list
function
function is_member_in_list($members, $new_member){
foreach($members as $member) {
if ($member[2] == $new_member[2]) // Email field
return true;
}
return false;
}
Updated is_member_in_list
function that checks for full name.
function is_member_in_list($members, $new_member){
foreach($members as $member) {
if ($member[2] == $new_member[2]) // Email field
return true;
* if (
* $member[0] == $new_member[0] && // First name
* $member[1] == $new_member[1] // Last name
* ) {
* return true;
* }
}
return false;
}
--
-
Readable code FTW! --
-
How to break up a long function into separate functions - each with 1 responsibility --
-
Future code changes can be easily made by changing one of the functions without affecting the main function's API
class: center, middle
class: center, middle
class: center, middle
class: center, middle