C H A P T E R 3
Introduction to Refactoring
Due to the strong practical nature of refactoring and the essentially empirical nature of human knowledge, we want to provide you with an example of what all this refactoring is about. Refactoring is a practice addressed to some of the main object-oriented development principles, so we think it best for the reader is to see a real example before diving deep into the refactoring techniques list.
In this chapter we'll try to give you an idea of what refactoring is and what could it mean to developers like us.
The Concept: What Refactoring Is
Everyone can understand the following expression:
(a + b)(a − b) = c
It means that depending on a and b values as input we get another value injected in c as output. It's a defined process, as well as an algorithm, and it has a well-known and expected outcome. Knowing that's true, we can also write
a2 − b2 = c
and say it's true, too. We can say that because, in general, we know
(a + b)(a − b) = a2 − b2
since it's a well-known factoring rule we all study in the early years of school.
The inner meaning we can read between these lines is that given an input, there are many ways to get the same output, or many routes to lead us to our desired result. Among these, we can prefer one more than another not by results—the same by definition—but by other factors such as readability, suppleness, or even just elegance.
That's the main thought behind refactoring: changing the inner structure of software with no change in its external behavior.
The Reason: What's the Goal of Refactoring?
Code has a cost. Always. It has a cost to be created, it has a cost to be maintained, it even has a cost to be executed, though several orders of magnitude less than the first two. That's the reason developers cost much more than CPUs. We have no way (yet?) to code software without use of the human brain.
Considering how we cannot avoid the intrinsic nature of our software costs, we have a strong need to find a way to hugely reduce those costs. That means removing typical pitfalls the human brain falls into. Those pitfalls lie beneath each of the bad smells we met in the previous chapter: long lists, big and complicated structures, interleaved dependencies, and similar things; they all deviate a human brain from control towards panic, resulting in an increment of development and maintenance costs.
Architecture and Structure, They Fade Away
One of the most powerful concept in physics is the second law of thermodynamics. It states that the potential for disorder of a system tends to increase. A glass, along a sufficient elapse of time, tends to break; biological tissue tends to decompose; ice tends to melt. It never happens by chance that a bunch of broken glasses recompose into a bottle fit to contain water; a dead cell never steps back to life; no ice is created spontaneously from water at 25°C. It means that systems need an external contribution to keep themselves in a given state or to step into a more ordered state: broken glasses must be melted to create bottles; cells need lots of energy coming from food to survive; ice is created by means of an A/C supply and a fridge. Everything, if left on its own, tends to decay.
What does that mean for us sitting in front of our nasty and unmaintainable code? It unfortunately means that it will get even nastier and less maintainable if we don't recover its structure in some way. Even the best-designed architecture is exposed to entropy's restless effect: its structure decays if left unattended. We start a project from scratch with the best intentions, and we design the most beautiful architecture ever. Then the code goes into the wild and, even before going into production, we are left with not enough time to redesign our software as long as new features or bugs are discovered. Then disorder creeps in, exposing that beloved good architecture of ours to a progressive decay, leading us to cope with a bad, nasty, uncomfortable, and unmaintainable code base.
We must distinguish between complexity and complicatedness. The first is something we might even want, since in our software we are likely encoding complex business behavior that is the core sense of it. The latter is just an obstacle. We don't want things to get complicated while we try to make our software reach the desired level of complexity.
Reworking Chaos into Well-Designed Code
Refactoring is the best known way to keep code away from disordered states. While the tendency of code is to slowly decay towards chaos, refactoring is exactly the opposite: we actively change the inner structure of our software, bringing it from chaos to a more ordered design through a series of simple stable steps.
No word in the last statement has a secondary role. Apart from the most obvious ones, all of the words have strong consequences in the global meaning. Actively means that we are acting on our code, thus providing that external contribution the second law of thermodynamics refers to. Changing means we have to operate to get a differently-written code at the end of our refactoring steps. Inner means we are acting on the inside of our code, without changing its interface and its external behavior. More means that order is a relative concept, and thus that we could refactor our code indefinitely, while it's our main aim to keep an eye on pragmatic choices. Series refers to the potential concatenation between different refactoring techniques we can exploit to achieve a better global result. Simple implies that every step must be “simple, even simplistic” (quoting Martin Fowler), thus multiplying chances that we won't introduce chaos (errors) while refactoring. Stable states our need to go from a stable condition to another stable one with the smallest leap ahead, to keep reversibility and correctness always granted.
Refactoring, then, is a way to oppose the natural tendency of code to become unmaintainable, while using the right techniques to improve the design of our code, improving it after having written it.
An Example, at Last
We waited long enough, then! Let's have a look at the next example. Ladies and gentlemen, please welcome the Order class:
class Order
{
public $gold_customer = false;
public $silver_customer = false;
public $items = array();
protected $first_name;
protected $last_name;
protected $customer_address;
protected $customer_city;
protected $customer_country;
protected $shipping_address;
public function setItem($code, $price, $description, $quantity)
{
$this->items[] = array('code' => $code,
'price' => $price,
'description' => $description,
'quantity' => $quantity
);
}
public function setItems($items)
{
$this->items = $items;
}
public function listItems()
{
return $this->items;
}
public function setCustomer($customer)
{
list($this->first_name, $this->last_name) = explode(' ', $customer);
}
public function getCustomer()
{
return $this->first_name.' '.$this->last_name;
}
public function setShippingAddress($address)
{
$this->shipping_address = $address;
}
public function getShippingAddress()
{
return $this->shipping_address;
}
public function isGoldCustomer()
{
return $this->gold_customer;
}
public function getTotal()
{
$total = 0;
foreach ($this->items as $item)
{
$currency = '';
// we check for the item to be valid
if (isset($item['price']) && isset($item['quantity']))
{
// we detect currency if indicated
$price = explode(' ', $item['price']);
if (isset($price[1]))
{
$currency = $price[1];
}
$price = $price[0];
$total += $price * $item['quantity'];
}
}
// If the customer is gold we apply 40% discount and...
if ($this->gold_customer)
{
$total = $total * 0.6;
// ...if amount is over 500 we apply further 20% discount
if ($total > 500)
{
$total = $total * 0.8;
}
}
// If the customer is silver we apply 20% discount and...
elseif ($this->silver_customer)
{
$total = $total * 0.8;
// ...if amount is over 500 we apply further 10% discount
if ($total > 500)
{
$total = $total * 0.9;
}
}
else
{
// if customer subscribed no fidelity program we apply 10% over 500
if ($total > 500)
{
$total = $total * 0.9;
}
}
if ($currency)
{
return round($total, 2).' '.$currency;
}
else return round($total, 2);
}
}
This is a simple class meant to describe and manage an order for a simple e-commerce web site. It features a collection of some items bought by a customer whose data are also stored in proper fields. It provides an interface to manage the items list and customer data and to compute the total amount due for the order, including a discount policy based on the total price for the items and the type of affiliation program joined by the customer.
The class is correct, as the next PHPUnit test proves:
class OrderTest extends PHPUnit_Framework_TestCase
{
public function setUp()
{
$this->order = new Order();
}
public function testGetTotal()
{
$items = array(
'34tr45' => array(
'price' => 10,
'description' => 'A very good CD by Jane Doe.',
'quantity' => 2
),
'34tr89' => array(
'price' => 70,
'description' => 'Super compilation.',
'quantity' => 1
)
);
$this->order->setItems($items);
$this->assertEquals((20 + 70), $this->order->getTotal());
}
public function testGetTotalAfterRemovingItem()
{
$items = array(
'34tr45' => array(
'price' => '9.99 EUR',
'description' => 'A very good CD by Jane Doe.',
'quantity' => 2
),
't667t4' => array(
'price' => '69.99 EUR',
'description' => 'Super compilation.',
'quantity' => 1
),
'jhk987' => array(
'price' => '49.99 EUR',
'description' => 'Foo singers. Rare edition.',
'quantity' => 3
),
);
$this->order->setItems($items);
unset($this->order->items['jhk987']);
$this->assertEquals((9.99 * 2 + 69.99).' EUR', $this->order->getTotal());
}
public function testListItems()
{
$this->assertEquals(array(), $this->order->listItems());
$items = array(
'34tr45' => array(
'price' => 10,
'description' => 'A very good CD by Jane Doe.',
'quantity' => 2
),
'34tr89' => array(
'price' => 70,
'description' => 'Super compilation.',
'quantity' => 1
),
);
$this->order->setItems($items);
$this->assertEquals($items, $this->order->listItems());
}
public function testGetCustomer()
{
$this->order->setCustomer('Jean Pistel'),
$this->assertEquals('Jean Pistel', $this->order->getCustomer());
}
public function testShippingAddress()
{
$this->order->setShippingAddress('84 Doe Street, London'),
$this->assertEquals('84 Doe Street, London', $this->order->getShippingAddress());
}
public function testDiscountForGoldSilverCustomer()
{
$this->assertFalse($this->order->isGoldCustomer());
$items = array(
'34tr45' => array(
'price' => 9.99,
'description' => 'A very good CD by Jane Doe.',
'quantity' => 2
),
'34tr89' => array(
'price' => 69.99,
'description' => 'Super compilation.',
'quantity' => 1
),
);
$this->order->setItems($items);
$this->assertEquals((19.98 + 69.99), $this->order->getTotal());
$this->order->silver_customer = true;
$this->assertEquals(71.98, $this->order->getTotal());
$this->order->gold_customer = true;
$this->assertEquals(53.98, $this->order->getTotal());
}
public function testDiscountOverOrderTotal()
{
$items = array(
'34tr45' => array(
'price' => 300,
'description' => 'A very good CD by Jane Doe.',
'quantity' => 1
),
'34tr89' => array(
'price' => 270,
'description' => 'Super compilation.',
'quantity' => 1
),
);
$this->order->setItems($items);
$this->assertEquals(570 * 0.9, $this->order->getTotal());
}
We will see PHPUnit in better detail later; if you don't get all the details right now, don't worry. We will be using variables and method names suitable to be understood in their ultimate essence.
What you may notice here is that this class shows a lack of compliance with good design principles, violating common good practices like single-responsibility—it cares about the order details, customer details, and discounting policies—and featuring quite long methods, even hard to be extended, mostly due to the wide use of conditional expressions. We will now try to detect and address each of these design defects with a step-by-step incremental strategy.
Look Ma'! No Comments!
We are quite used to reading and hearing suggestions about better and more widespread use of comments to make our code clearer to other developers and to ourselves, after even a small period away from a given portion of source code. We strongly agree with this rule, but we would like to point out that way too often this rule is misinterpreted to justify the use of comments as a substitute for clear and self-explanatory code.
While we advocate the use of code in classes and methods headers1 we would like to see the smallest number possible of inline comments among lines of code, writing a code that shows the intention of the coder by itself, in a way almost as readable as natural language. It can be a difficult pursuit, but it pays, both in the short and the long run: it provides an easy way to communicate design among developers, as well as from developers to non-technical stakeholders, including the customer, and to ease the acquisition of new domain knowledge.
In our example we have the whole discount calculation made clear by means of comments, and we would like to get a clearer code on its own. We can overcome this defect by extracting commented code into a separated method. So we extract the code commented with
// ...if amount is over 500 we apply further 20% discount
into a private method with a meaningful name:
private function ifAmountIsOver500WeApplyFurther20Discount($total)
{
if ($total > 500)
{
$total = $total * 0.8;
}
return $total;
}
And we do the same with the code commented with
// ...if amount is over 500 we apply further 10% discount
extracting it into
private function ifAmountIsOver500WeApplyFurther10Discount($total)
{
if ($total > 500)
{
$total = $total * 0.9;
}
return $total;
}
After these two simple extractions we get a slightly different getTotal() method.
public function getTotal()
{
$total = 0;
foreach ($this->items as $item)
{
$currency = '';
// we check for the item to be valid
if (isset($item['price']) && isset($item['quantity']))
{
// we detect currency if indicated
$price = explode(' ', $item['price']);
if (isset($price[1]))
{
$currency = $price[1];
}
$price = $price[0];
$total += $price * $item['quantity'];
}
}
// If the customer is gold we apply 40% discount and...
if ($this->gold_customer)
{
$total = $total * 0.6;
$total = $this->ifAmountIsOver500WeApplyFurther20Discount($total);
}
// If the customer is gold we apply 20% discount and...
elseif ($this->silver_customer)
{
$total = $total * 0.8;
$total = $this->ifAmountIsOver500WeApplyFurther10Discount($total);
}
else
{
$total = $this->ifAmountIsOver500WeApplyFurther10Discount($total);
}
if ($currency)
{
return round($total, 2).' '.$currency;
}
else return round($total, 2);
}
________________________
1 You may already know PHPDocumentor, a PHP documentation–generator available at http://www.phpdoc.org
We run our test and we still find the class doing what we expect it to do. Though we think we still have a bad design, we have taken a little step towards quality. Let's move on, and we will see our design improve at each step.
Once Is Better than Twice
Now that we got rid of useless comments, the code started speaking for itself. Good. If only the Order class showed less duplicated code it would be a lot nicer!
Code duplication is one of the nastiest things a developer will ever have to cope with. It leads to bugs, hard maintenance, and badly-structured design of our software. Luckily enough we can count on many techniques to remove duplications in the code, and entire sections of this book will be about how to find a way through the complexity of your code to encapsulate every single datum or behavioral aspect of your code in one single and well-defined spot.
In our example we extracted two very similar methods: ifAmountIsOver500WeApplyFurther20Discount() and ifAmountIsOver500WeApplyFurther10Discount(). They encapsulate the very same logic applied to two different discount values. It calls for a very rewarding change. We rename one of those methods to keep its name as close as possible to its meaning and we add a $discount parameter:
private function applyDiscountOverThreshold($total, $discount = 1)
{
$threshold = 500;
if ($total > $threshold)
{
$total = $total * $discount;
}
return $total;
}
Then we replace all the references to the two original methods with proper calls to this new method.
$total = $this->applyDiscountOverThreshold($total, 0.8);
and
$total = $this->applyDiscountOverThreshold($total, 0.9);
Now we can remove the second original method we didn't edit and make the new code more expressive by bringing the common threshold into a class constant, obtaining
class Order
{
const DISCOUNT_THRESHOLD = 500;
...
private function applyDiscountOverThreshold($total, $discount = 1)
{
if ($total > self::DISCOUNT_THRESHOLD)
{
$total = $total * $discount;
}
return $total;
}
...
}
With a similar reasoning we can further reduce code duplication by identifying a frequently-used code pattern like
$total = $total * 0.6;
and extracting a method to generalize its use across our class:
private function applyDiscount($total, $discount)
{
return $total * $discount;
}
Let us rewrite our applyDiscountOverThreshold() and getTotal() methods:
private function applyDiscountOverThreshold($total, $discount = 1)
{
if ($total > self::DISCOUNT_THRESHOLD)
{
$total = $this->applyDiscount($total, $discount);
}
return $total;
}
public function getTotal()
{
$total = 0;
foreach ($this->items as $item)
{
$currency = '';
// we check for the item to be valid
if (isset($item['price']) && isset($item['quantity']))
{
// we detect currency if indicated
$price = explode(' ', $item['price']);
if (isset($price[1]))
{
$currency = $price[1];
}
$price = $price[0];
$total += $price * $item['quantity'];
}
}
// If the customer is gold we apply 40% discount and...
if ($this->gold_customer)
{
$threshold_discount = 0.8;
$total = $this->applyDiscount($total, 0.6);
}
// If the customer is gold we apply 20% discount and...
elseif ($this->silver_customer)
{
$threshold_discount = 0.9;
$total = $this->applyDiscount($total, 0.8);
}
else
{
$threshold_discount = 0.9;
}
$total = $this->applyDiscountOverThreshold($total, $threshold_discount);
if ($currency)
{
return round($total, 2).' '.$currency;
}
else return round($total, 2);
}
Goliath Died in the End
We split our code into better self-explanatory single-responsibility methods, but we are still violating the same encapsulation principle at the level of classes. The Order class is still managing the purchase, the customer data, and the discounting policy, and the bad entanglement of these aspects is well represented by the not-so-short if-else block we find in the getTotal() method of our Order class.
We want to attack this bad structure by first extracting customer data management into another new Customer class and then using it as a powerful device to remove stiff conditional logic.
First, we extract the Customer class along with all its related attributes:
class Customer
{
protected $is_gold = false;
protected $is_silver = false;
protected $first_name;
protected $last_name;
protected $customer_address;
protected $customer_city;
protected $customer_country;
public function isGold()
{
return $this->is_gold;
}
public function makeGold()
{
$this->is_gold = true;
}
public function isSilver()
{
return $this->is_silver;
}
public function makeSilver()
{
$this->is_silver = true;
}
public function setName($customer)
{
list($this->first_name, $this->last_name) = explode(' ', $customer);
}
public function __toString()
{
return $this->first_name.' '.$this->last_name;
}
}
Then we add references to this new class in the Order class, changing the constructor and all the methods, using the customer data to empower the needed interaction:
class Order
{
const DISCOUNT_THRESHOLD = 500;
public $items = array();
protected $customer;
protected $shipping_address;
public function __construct()
{
$this->customer = new Customer();
}
public function setItem($code, $price, $description, $quantity)
{
$items[] = array(
'code' => $code,
'price' => $price,
'description' => $description,
'quantity' => $quantity
);
}
public function setItems($items)
{
$this->items = $items;
}
public function listItems()
{
return $this->items;
}
public function getCustomer()
{
return $this->customer;
}
public function setShippingAddress($address)
{
$this->shipping_address = $address;
}
public function getShippingAddress()
{
return $this->shipping_address;
}
private function applyDiscountOverThreshold($total, $discount = 1)
{
if ($total > self::DISCOUNT_THRESHOLD)
{
$total = $this->applyDiscount($total, $discount);
}
return $total;
}
private function applyDiscount($total, $discount)
{
return $total * $discount;
}
public function getTotal()
{
$total = 0;
foreach ($this->items as $item)
{
$currency = '';
// we check for the item to be valid
if (isset($item['price']) && isset($item['quantity']))
{
// we detect currency if indicated
$price = explode(' ', $item['price']);
if (isset($price[1]))
{
$currency = $price[1];
}
$price = $price[0];
$total += $price * $item['quantity'];
}
}
// If the customer is gold we apply 40% discount and...
if ($this->customer->isGold())
{
$threshold_discount = 0.8;
$total = $this->applyDiscount($total, 0.6);
}
// If the customer is gold we apply 20% discount and...
elseif ($this->customer->isSilver())
{
$threshold_discount = 0.9;
$total = $this->applyDiscount($total, 0.8);
}
else
{
$threshold_discount = 0.9;
}
$total = $this->applyDiscountOverThreshold($total, $threshold_discount);
if ($currency)
{
return round($total, 2).' '.$currency;
}
else return round($total, 2);
}
}
Notice how we removed the isGoldCustomer() method since we plan to use $this->customer->isGold() as a predicate to determine whether the customer belongs to the Gold affiliation program.
This time we have to edit our unit test as well, to match our newly-set interaction between the Order and Customer classes. Unit tests are such if they isolate a single unit of code—usually a class—and mock any related external behavior, usually belonging to other classes. Though this is the best way to test a well-defined portion of code in our systems, for clarity's sake here we will switch to so-called integration tests, testing the two classes together. Though it is absolutely not our favorite practice, it will be a lot clearer for those not used to unit tests: all in all, we are here to understand and learn step-by-step, not to exhibit brute force. With that disclaimer given, let's have a look at our test changes:
class OrderTest extends PHPUnit_Framework_TestCase
{
...
public function testGetCustomer()
{
$this->order->getCustomer()->setName('Jean Pistel'),
$this->assertEquals('Jean Pistel', (string)$this->order->getCustomer());
}
...
public function testDiscountForGoldSilverCustomer()
{
$this->assertFalse($this->order->getCustomer()->isGold());
$items = array(
'34tr45' => array(
'price' => 9.99,
'description' => 'A very good CD by Jane Doe.',
'quantity' => 2
),
'34tr89' => array(
'price' => 69.99,
'description' => 'Super compilation.',
'quantity' => 1
),
);
$this->order->setItems($items);
$this->assertEquals((19.98 + 69.99), $this->order->getTotal());
$this->order->getCustomer()->makeSilver();
$this->assertEquals(71.98, $this->order->getTotal());
$this->order->getCustomer()->makeGold();
$this->assertEquals(53.98, $this->order->getTotal());
}
public function testDiscountOverOrderTotal()
{
$items = array(
'34tr45' => array(
'price' => 300,
'description' => 'A very good CD by Jane Doe.',
'quantity' => 2
),
'34tr89' => array(
'price' => 270,
'description' => 'Super compilation.',
'quantity' => 1
),
);
$this->order->setItems($items);
$this->assertEquals(870 * 0.9, $this->order->getTotal());
$this->order->getCustomer()->makeSilver();
$this->assertEquals(870 * 0.8 * 0.9, $this->order->getTotal());
$this->order->getCustomer()->makeGold();
$this->assertEquals(870 * 0.6 * 0.8, $this->order->getTotal());
}
}
Now that we decoupled Customer and Order, we can launch our ultimate assault on the if-else cascading block, where most of the discounting policy is mainly implemented.
Big conditional blocks are a very bad pattern to be used in our software. They are hard to understand if they go over a minimal size, they are error-prone, they incentivize code duplication, and, above all, they are not robust against behavioral extensions: whenever I want to add another case I have to arrange at least a new conditional branch, not to mention complex nested conditional structures.
In our Order class, if we were to add a new customer affiliation program, we would need to add another else-if clause after those meant to manage Gold and Silver affiliation programs. Furthermore, the way we structured the getTotal() method badly manages the default case, forcing us to maintain a whole else branch just to set the $threshold_discount. Last but not least, “Gold” and “Silver” conditional fragments are very similar, feeding our desire to remove that duplication.
A powerful and often suitable solution is to exploit polymorphism. We can encapsulate the concept of affiliation in a stand-alone class and then extend it to represent all kinds of affiliation programs. Once encapsulated, that concept becomes a lot easier to use.
We create an abstract Affiliation class and a few children:
abstract class Affiliation
{
abstract public function getType();
}
class GoldAffiliation extends Affiliation
{
public function getType()
{
return Customer::GOLD;
}
}
class SilverAffiliation extends Affiliation
{
public function getType()
{
return Customer::SILVER;
}
}
class NullAffiliation extends Affiliation
{
public function getType()
{
return null;
}
}
We add needed constants, fields, and methods to the Customer class:
class Customer
{
const GOLD = 'gold';
const SILVER = 'silver';
protected $type;
protected $affiliation;
...
public function __construct()
{
$this->affiliation = new NullAffiliation;
}
public function isGold()
{
return $this->affiliation->getType() == self::GOLD;
}
public function makeGold()
{
$this->affiliation = new GoldAffiliation();
}
public function isSilver()
{
return $this->affiliation->getType() == self::SILVER;
}
public function makeSilver()
{
$this->affiliation = new SilverAffiliation();
}
...
}
This is a middle step. We can run our test to check that everything is syntactically right, but still our Order class doesn't rely on polymorphic behavior. We have to equip our Affiliation hierarchy with methods to calculate the proper discount and then make the getTotal method in the Order class reference them.
We start by adding a protected attribute, $threshold_discount, and an abstract calculateDiscount() method to our abstract Affiliation class. By extending this method in each affiliation program, we get the right behavior case-by-case, moving into the subclass method the code coming from the related source conditional fragment:
class GoldAffiliation extends Affiliation
{
...
public function calculateDiscount($order, $total)
{
$this->threshold_discount = 0.8;
$total = $order->applyDiscount($total, 0.6);
return $order->applyDiscountOverThreshold($total, $this->threshold_discount);
}
}
class SilverAffiliation extends Affiliation
{
...
public function calculateDiscount($order, $total)
{
$this->threshold_discount = 0.9;
$total = $order->applyDiscount($total, 0.8);
return $order->applyDiscountOverThreshold($total, $this->threshold_discount);
}
}
class NullAffiliation extends Affiliation
{
...
public function calculateDiscount($order, $total)
{
$this->threshold_discount = 0.9;
return $order->applyDiscountOverThreshold($total, $this->threshold_discount);
}
}
This way we can get rid of the whole if-else block used in the getTotal() method to compute discounting policies just by calling
$total = $this->customer->getAffiliation()->calculateDiscount($this, $total);
Here you get all Customer, Affiliate, and Order classes presented together for you to have a look at them before we come to the conclusion of this chapter:
class Customer
{
protected $type;
protected $affiliation;
protected $first_name;
protected $last_name;
protected $customer_address;
protected $customer_city;
protected $customer_country;
public function __construct()
{
$this->affiliation = new NullAffiliation;
}
public function isGold()
{
return $this->affiliation->isGold();
}
public function makeGold()
{
$this->affiliation = new GoldAffiliation();
}
public function isSilver()
{
return $this->affiliation->isSilver();
}
public function makeSilver()
{
$this->affiliation = new SilverAffiliation();
}
public function setName($customer)
{
list($this->first_name, $this->last_name) = explode(' ', $customer);
}
public function __toString()
{
return $this->first_name.' '.$this->last_name;
}
public function getAffiliation()
{
return $this->affiliation;
}
}
class GoldAffiliation extends Affiliation
{
public function getType()
{
return Affiliation::GOLD;
}
public function calculateDiscount($order, $total)
{
$this->threshold_discount = 0.8;
$total = $order->applyDiscount($total, 0.6);
return $order->applyDiscountOverThreshold($total, $this->threshold_discount);
}
}
class SilverAffiliation extends Affiliation
{
public function getType()
{
return Affiliation::SILVER;
}
public function calculateDiscount($order, $total)
{
$this->threshold_discount = 0.9;
$total = $order->applyDiscount($total, 0.8);
return $order->applyDiscountOverThreshold($total, $this->threshold_discount);
}
}
class NullAffiliation extends Affiliation
{
public function getType()
{
return null;
}
public function calculateDiscount($order, $total)
{
$this->threshold_discount = 0.9;
return $order->applyDiscountOverThreshold($total, $this->threshold_discount);
}
}
class Order
{
const DISCOUNT_THRESHOLD = 500;
public $items = array();
protected $customer;
protected $shipping_address;
public function __construct()
{
$this->customer = new Customer();
}
public function setItem($code, $price, $description, $quantity)
{
$items[] = array(
'code' => $code,
'price' => $price,
'description' => $description,
'quantity' => $quantity
);
}
public function setItems($items)
{
$this->items = $items;
}
public function listItems()
{
return $this->items;
}
public function getCustomer()
{
return $this->customer;
}
public function setShippingAddress($address)
{
$this->shipping_address = $address;
}
public function getShippingAddress()
{
return $this->shipping_address;
}
public function applyDiscountOverThreshold($total, $discount = 1)
{
if ($total > self::DISCOUNT_THRESHOLD)
{
$total = $this->applyDiscount($total, $discount);
}
return $total;
}
public function applyDiscount($total, $discount)
{
return $total * $discount;
}
public function getTotal()
{
$total = 0;
foreach ($this->items as $item)
{
$currency = '';
// we check for the item to be valid
if (isset($item['price']) && isset($item['quantity']))
{
// we detect currency if indicated
$price = explode(' ', $item['price']);
if (isset($price[1]))
{
$currency = $price[1];
}
$price = $price[0];
$total += $price * $item['quantity'];
}
}
$total = $this->customer->getAffiliation()->calculateDiscount($this, $total);
if ($currency)
{
return round($total, 2).' '.$currency;
}
else return round($total, 2);
}
}
After we performed this simple series of changes, the Order class became
- Easier to understand. We can even ignore what happens in the
AffiliationandCustomerclasses to understand that a discount depending on customer type will be applied to our order's total amount. - Easier to extend. Its behavior is now depending on a plug-in-like device. You add a new
Affiliationsubclass and you can get a completely different discount policy without even needing theOrderclass to know it. - Easier to read. We don't rely on comments anymore to figure out how the discount policy is computed. Every method communicates its role and responsibility in a clear and effective way. Every class is focused on a single concept, creating a smaller context to cope with when reading, understanding, designing, and writing our code.
Some readers could argue that we still keep lots of code duplication in Affiliation subclasses and we still manage the currency in a bad procedural style. We agree. We decided to stop our example here to move on to the next few chapters, but our improvement of the code could go a lot further.
This is not a real-life example and we had to find a trade-off between complexity and clarity. Anyway, in real-life projects, as well, we come to a point when we have to decide whether to stop refactoring a given portion of code, and whether to invest time in improving existing code or developing new features. The right skills to decide this are to be derived from the context with the help of experience. By now what we care about most is that you met your first refactoring.