When designing our applications, often we limit ourselves to creating the most important classes and then we create complex conditional logic in their methods to change their behavior, based on their configuration. This approach is typical of those who used procedural programming for a long time and are accustomed to a cascading code execution, rather than designing objects that communicate with each other.
In procedural programming styles, the only way to change the behavior of code flow is with conditional logic. In object-oriented programming, in many cases, the conditional logic can be implemented in different ways more consistent with the object's behavior.
The logic often tries to change the behavior of a certain object. If this is the intent, we must use polymorphism. But if the condition has sense and gives greater prominence to the details of the condition rather than the conditions themselves, decouple the details of the condition from the condition itself. In other cases, our conditions are used only to check limit cases. We must isolate these cases by adding guard clauses. Sometimes we also have nested conditional logic that uses only one exit point. In this case we can simplify by introducing multiple exit points.
In this chapter we'll see a collection of some refactoring methods created by Martin Fowler [FOW01]. For each method we'll see the motivation and situation for its use, the mechanism that will explain how we can apply the method to our existing code, and some examples showing how the method works in a real-world case.
Problem: “You have a complicated conditional (if/else) statement.”
Solution: “Extract methods from the condition, then extract the if part and then the else part.”
Having complex logical conditional inside our methods is one of the most typical things that happen when our code increases and our design isn't good enough. Such conditional logic can make a method very long and difficult to read. A readable code is easier to maintain. Conditional logics often explain what they do, but it is not easy to understand why they do it, information that is, in most cases, much more useful.
To simplify these pieces of code we need to clarify the intentions of conditional logic. We must be capable of understanding easily because we are entering a conditional branch. To do this we need to highlight the condition of conditional logic, extrapolating this piece of code in an external method, and then extrapolate to other private methods outside of the pieces of code for each individual branch.
We want to decompose conditional for the method getAmount() of the following class
.
class Sale
{
public $expired_at;
public $amount;
public function getAmount()
{
if(!is_null($this->expired_at) && $this->expired_at < time())
{
$interest = 10;
$this->amount = $this->amount + ($this->amount / 100 * $interest);
}
else
{
$discount = 10;
$this->amount = $this->amount - ($this->amount / 100 * $discount);
}
return $this->amount;
}
}
First of all, we write a unit test if we didn't before. We write it to confirm that the behavior doesn't change after refactoring. So, our test will be green at once.
class SaleTest extends PHPUnit_Framework_TestCase
{
public function testAmount()
{
$sale = new Sale();
$sale->amount = 10;
$sale->expired_at = strtotime('-10 days'),
$this->assertEquals(10 + (10/100*10), $sale->getAmount());
$sale = new Sale();
$sale->amount = 10;
$sale->expired_at = strtotime('+10 days'),
$this->assertEquals(10 - (10/100*10), $sale->getAmount());
}
}
Run the test—it should be green. Now we can start refactoring. The first step of refactoring is to extract conditional code in a private method. We name the method isExpired()
because our conditional chunk of code checks if the sale is expired. We create the private method isExpired()
and, with the technique of extract method, we move chunks of code into the new method.
class Sale
{
public $expired_at;
public $amount;
public function getAmount()
{
if($this->isExpired())
{
$interest = 10;
$this->amount = $this->amount + ($this->amount / 100 * $interest);
}
else
{
$discount = 10;
$this->amount = $this->amount - ($this->amount / 100 * $discount);
}
return $this->amount;
}
private function isExpired()
{
return !is_null($this->expired_at) && $this->expired_at < time();
}
}
We run the unit test again and it should still be green.
PHPUnit 3.4.1 by Sebastian Bergmann.
.
Time: 0 seconds
OK (1 test, 2 assertions)
The next step is to move each branch of the condition in a private method. We do the same as we did before for each branch. So we create the private method getAmountWithInterest()
for the first branch and the method getAmountWithDiscount()
for the second branch.
class Sale
{
public $expired_at;
public $amount;
public function getAmount()
{
if($this->isExpired())
{
return $this->getAmountWithInterest();
}
else
{
return $this->getAmountWithDiscount();
}
}
private function isExpired()
{
return !is_null($this->expired_at) && $this->expired_at < time();
}
private function getAmountWithInterest()
{
$interest = 10;
return $this->amount + ($this->amount / 100 * $interest);
}
private function getAmountWithDiscount()
{
$discount = 10;
return $this->amount - ($this->amount / 100 * $discount);
}
}
We run the unit test for the last time and it should still be green. Our refactoring works fine, and we have decomposed our conditional, simplifying the legibility of conditional code.
Problem: “You have a sequence of conditional tests with the same result.”
Solution: “Combine them into a single conditional expression and extract it.”
Sometimes we see a different set of conditional expressions that return all the same value. These expressions in most cases can be combined through logical operators AND/OR. There are two main advantages in unifying these conditions. The first is to express a single condition in a set of conditional interdependent logics. The second is to combine conditional expressions in a single condition using the “extract method” refactoring technique. These advantages improve the readability of the condition itself.
Sometimes the conditional expressions cannot be linked to each other, since they have strong identities. In this case it is not advisable to consolidate the expressions. It's better to leave them separated, despite returning the same result.
In the example we saw earlier, we have a sequence of conditions that return the same result.
class Sale
{
public $never_expired;
public $expired_at;
public $amount;
public function expiredAmount()
{
if ($this->never_expired) return 0;
if (is_null($this->expired_at)) return 0;
if ($this->expired_at > time()) return 0;
return $this->amount/100*10;
}
}
In this case, any condition hides side effects and we can start with consolidation. First of all, we write a unit test to ensure that the behavior remains the same after the refactoring.
class SaleTest extends PHPUnit_Framework_TestCase
{
public function testExpiredAmount()
{
$sale = new Sale();
$sale->amount = 10;
$this->assertEquals(0, $sale->expiredAmount());
$sale->never_expired = true;
$this->assertEquals(0, $sale->expiredAmount());
$sale->never_expired = false;
$sale->expired_at = strtotime('+10 days'),
$this->assertEquals(0, $sale->expiredAmount());
$sale->expired_at = strtotime('-10 days'),
$this->assertEquals(1, $sale->expiredAmount());
}
}
We run a test and verify that it is green. If everything is ok, we can start our refactoring. The first step of consolidation is to combine the sequence into a single condition through the conditional logical operators. In our case we can combine the conditions by the logical operator OR.
class Sale
{
public $never_expired;
public $expired_at;
public $amount;
public function expiredAmount()
{
if ($this->never_expired || is_null($this->expired_at) || $this->expired_at > time())
return 0;
return $this->amount/100*10;
}
}
We have connected conditions linked together in a single logical expression, creating a single point of return instead of three different ones.
To improve readability we can perform the last step of refactoring, extracting the conditional expression into a new private method:
class Sale
{
public $never_expired;
public $expired_at;
public $amount;
public function expiredAmount()
{
if ($this->isNotExpired())
return 0;
return $this->amount/100*10;
}
private function isNotExpired()
{
return $this->never_expired || is_null($this->expired_at) || $this->expired_at > time();
}
}
Run the unit tests again—everything should still work properly.
As we have seen, not-nested sequences can be linked by OR logic; nested sequences can be combined through the logical operator AND.
if (!is_null($this->expired_at))
if ($this->expired_at < time())
return 10;
else
return 0;
else
return 0;
This conditional block can be combined as follows:
if (!is_null($this->expired_at) AND $this->expired_at < time())
return 10;
else
return 0;
Where we have nested and not-nested conditional logic, we can try to use both AND and OR operators to consolidate the logic.
Problem: “The same fragment of code is in all branches of a conditional expression.”
Solution: “Move it outside of the expression.”
When we have repeated code lines within two conditional branches, we should extract them out and put them in the bottom of the conditional block.
In the following class we have a conditional block, and in each branch there are two equal lines duplicated.
class Sale
{
public $expired_at;
public $amount;
...
public function getRealAmount()
{
if ($this->isExpired())
{
$amount = $this->getAmount() + 10;
$this->setAmount($amount);
return $this->getAmount();
}
else
{
$amount = $this->getAmount() - 5;
$this->setAmount($amount);
return $this->getAmount();
}
}
...
}
We write a unit test to confirm that the behavior of the Sale
class remains the same after the refactoring.
class SaleTest extends PHPUnit_Framework_TestCase
{
public function testSaleExpiredAmount()
{
$sale = new Sale();
$sale->setExpiredAt(strtotime('yesterday'));
$this->assertEquals(10, $sale->getRealAmount());
$sale->setAmount(12);
$this->assertEquals(22, $sale->getRealAmount());
}
public function testSaleNotExpiredAmount()
{
$sale = new Sale();
$sale->setExpiredAt(strtotime('tomorrow'));
$this->assertEquals(-5, $sale->getRealAmount());
$sale->setAmount(10);
$this->assertEquals(5, $sale->getRealAmount());
}
}
First of all we need to identify the chunk of code in branches we want to extract. The method setAmount()
is executed in both branch code and in the end of conditional code. So we can move this chunk of code after the conditional block.
class Sale
{
public $expired_at;
public $amount;
...
public function getRealAmount()
{
if ($this->isExpired())
{
$amount = $this->getAmount() + 10;
}
else
{
$amount = $this->getAmount() - 5;
}
$this->setAmount($amount);
return $this->getAmount();
}
...
}
Run the test—it should still be green.
Problem: “You have a variable that is acting as a control flag for a series of boolean expressions.”
Solution: “Use a break or return instead.”
When we write a control structure loop as while
or for
, frequently, we use conditions with control flag variables to understand when to exit from it.
The use of control flags comes from the rules of structured programming languages, allowing a single access point and one exit point. Fortunately, PHP allows you a single access point, but the ability to define multiple exit points. For the same reason, PHP provides control structures such as continue and breaks that allow you to exit a loop without using flag control.
It's truly incredible the value that you can give back to your code by removing control flags, which make our code very messy and difficult to read.
The easiest way to remove the control flags in PHP is to use control structures such as continue and breaks that allow you to stop the execution of a cycle when you need to.
There is also another way, perhaps even more elegant, to remove the control flag:
In the following example there is a CreditCardRepository
class that contains all transactions made with a credit card in the current month. If a transaction exceeds the maximum acceptable limit, then the card is blocked for a month.
class CreditCardRepository
{
public $transactions = array();
public $max_amount = 10000;
public $card_blocked = false;
public function checkAmount()
{
$found = false;
foreach($this->transactions as $transaction)
{
if(!$found)
{
if ($transaction > $this->max_amount)
{
$this->blockCreditCardForOneMonth(time());
$found = true;
}
}
}
}
public function blockCreditCardForOneMonth($time)
{
$this->card_blocked = $time;
}
}
First of all, we write a unit test to confirm that the class behavior doesn't change after removing the control flag.
class CreditCardRepositoryTest extends PHPUnit_Framework_TestCase
{
public function testCheckAmount()
{
$card_repository = new CreditCardRepository();
$this->assertFalse((boolean)$card_repository->card_blocked);
$card_repository->transactions = array('100', '11000'),
$card_repository->checkAmount();
$this->assertTrue((boolean)$card_repository->card_blocked);
$card_repository = new CreditCardRepository();
$card_repository->transactions = array('100', '200'),
$card_repository->checkAmount();
$this->assertFalse($card_repository->card_blocked);
}
}
In the unit test there are two assertions; the first checks that the card is blocked if there is one transaction that exceeds the maximum limit. The second test checks that the card is not blocked if the transactions are all below the maximum limit. We run the unit test and make sure the line is green. Yeah! It's time to refactor.
The first step is to identify the control flag. In our case it is the variable $found through which we verify that a transaction limit has been reached. Once we find the control flag, we can replace its assignment with the control structure break and remove the control flag conditions and its references.
class CreditCardRepository
{
...
public function checkAmount()
{
foreach($this->transactions as $transaction)
{
if ($transaction > $this->max_amount)
{
$this->blockCreditCardForOneMonth(time());
break;
}
}
}
...
}
Let's run tests to verify that everything works as before. With this refactoring, we also improved the performance of our routine. When it is interrupted, it just finds the first max transaction, rather than loop over all transactions.
Consider the same example shown previously, with the difference that this time the variable $found is assigned the maximum value of the transaction, and then passed to a method that is executed outside the loop.
class CreditCardRepository
{
...
public function checkAmount()
{
$found = false;
foreach($this->transactions as $transaction)
{
if(!$found)
{
if ($transaction > $this->max_amount)
{
$this->blockCreditCardForOneMonth(time());
$found = $transaction;
}
}
}
$this->setMaxTransaction($transction);
}
...
}
First, write a unit test if you didn't before. In this case we want to use an exit point to stop our cycle instead of the control structure break. First we extract the logic into a new private method and return the value found.
class CreditCardRepository
{
...
public function checkAmount()
{
$transaction = $this->maxTransaction();
$this->setMaxTransaction($transaction);
}
private function maxTransaction()
{
$found = false;
foreach($this->transactions as $transaction)
{
if(!$found)
{
if ($transaction > $this->max_amount)
{
$this->blockCreditCardForOneMonth(time());
$found = $transaction;
}
}
}
return $found;
}
...
}
Then we can replace the assignment of the control flags directly with a return, delete the final return, the conditional control flag, and all its references.
class CreditCardRepository
{
...
private function maxTransaction()
{
foreach($this->transactions as $transaction)
{
if ($transaction > $this->max_amount)
{
$this->blockCreditCardForOneMonth(time());
return $transaction;
}
}
}
...
}
Let's run the unit tests and make sure the line is still green.
Problem: “A method has conditional behavior that does not make clear the normal path of execution.”
Solution: “Use guard clauses for all the special cases.”
Conditional forms occur, in most cases, in two forms. The first is one in which both branches of if and else check normal behavior. The second is where only one branch checks the normal case while the other checks some extreme cases that do not belong to the normal flow case studies. This happens especially with nested conditional control structures.
These two types of conditions are very different. In the first case it is necessary to express the condition through normal if and else, because both branches have the same weight and value. In the second case, extreme cases must be closed off from the normal condition. It must simply verify that the limit condition is true and return its result. These types of controls are called guard clauses [BACK].
The purpose of this refactoring is to highlight the borderline cases and isolate them from the normal flow applications. It must be clear that we have fallen into a borderline case and we must immediately return value and exit the method.
The use of nested conditional logic to express extreme cases is typical of those programmers who think that the exit point of a method or function can be only one. Fortunately it isn't true, and we can create many exit points, as many as there are guard clauses.
In the following method we have two conditional branches representing two extreme cases. The first checks if today is the first day of the month, and the second checks if it is the last day of the month, assigning different discounts if the check passes. The last is the normal case where the standard discount is returned.
public function getOfferDiscount()
{
// Limit conditions
if ($this->isFirstDayOfMonth())
{
$offer_discount = $this->getFirstDayDiscount();
}
else
{
if ($this->isLastDayOfMonth())
{
$offer_discount = $this->getLastDayDiscount();
}
else
{
$offer_discount = $this->getDiscount();
}
}
return $offer_discount;
}
The nested conditional block, in this method, checks both extreme conditions and normal conditions, making it seem all of the same value. But the condition with greater value is the one that returns the standard discounts. Our task is to insert guard clauses to isolate the borderline cases and return the right value and visibility to normal conditions.
Like other techniques of refactoring, this technique may sound simple, but let's not forget to create a unit test to confirm that the behavior of the method does not change after refactoring. Run the unit test, and if the line is green you can start with the refactoring.
To simplify this conditional logic we can add guard clauses for the extreme conditions. Starting from the first condition, at the beginning of the branch, remove the variable $offer_discount and create an exit point that returns the value of the getFirstDayDiscount()
method.
public function getOfferDiscount()
{
// Limit conditions
if ($this->isFirstDayOfMonth())
{
return $this->getFirstDayDiscount();
}
else
{
if ($this->isLastDayOfMonth())
{
$offer_discount = $this->getLastDayDiscount();
}
else
{
$offer_discount = $this->getDiscount();
}
}
return $offer_discount;
}
We can do the same type of operation for the other two conditions and remove the final return.
public function getOfferDiscount()
{
// Limit conditions
if ($this->isFirstDayOfMonth())
{
return $this->getFirstDayDiscount();
}
else
{
if ($this->isLastDayOfMonth())
{
return $this->getLastDayDiscount();
}
else
{
return $this->getDiscount();
}
}
}
Now we can move the normal case outside the conditional block and remove the else branch that becomes useless, because it is empty.
public function getOfferDiscount()
{
// Limit conditions
if ($this->isFirstDayOfMonth())
{
return $this->getFirstDayDiscount();
}
else
{
if ($this->isLastDayOfMonth())
{
return $this->getLastDayDiscount();
}
}
return $this->getDiscount();
}
Now we can move down the conditional block, the second extreme case, and remove the else, because it is empty.
public function getOfferDiscount()
{
// Limit conditions
if ($this->isFirstDayOfMonth())
{
return $this->getFirstDayDiscount();
}
if ($this->isLastDayOfMonth())
{
return $this->getLastDayDiscount();
}
return $this->getDiscount();
}
Removing the braces from the first clause of the guard, we can make it even easier to read.
public function getOfferDiscount()
{
// Limit conditions
if ($this->isFirstDayOfMonth()) return $this->getFirstDayDiscount();
if ($this->isLastDayOfMonth()) return $this->getLastDayDiscount();
return $this->getDiscount();
}
After each small step we run tests to make sure everything is working the way it worked before.
Problem: “You have a conditional that chooses different behavior depending on the type of an object.”
Solution: “Move each leg of the conditional to an overriding method in a subclass. Make the original method abstract.”
Polymorphism is one of the most charming features of object-oriented programming because it avoids the use of logical conditional on object type when we have an object that changes its behavior according to its type. When we have switch or nested if-else statements that control an object type to change its behavior, in reality we are not using the object-oriented programming paradigm properly.
An object must always have the same behavior, and it should not change depending on its configuration or parameter value. If that happens, we need to replace this code with a sub-class or through a state/strategy pattern.
The disadvantage of switch and if-else statements is that if we use a lot of them in the application, when we add a new type, we should change them all. Instead, through polymorphism we can simply add a new subclass and implement the methods required. All of that will remain hidden from customers who use this class. They will simply invoke the method and not know about the subclasses. This will make the system easier to extend and maintain, having small objects with a strong identity.
Before replacing conditional logic with polymorphism, we must have an inheritance structure implemented. Where we haven't, we can use the “Replace type code with subclasses” or “Replace type code with state/strategy pattern” refactoring methods. If the object has not yet been extended and its type does not change after it is created, we can use the subclasses. Instead, if the object has already been extended and/or its type can be changed after it is created, we have to use the state/strategy pattern.
Once you create the right structure, you can begin to change the conditional logic.
We use an example common to most web services that use “freemium” policies. In these services we usually have three types of users: the administrator user, the premium user, and the base user. Their credentials for access to various services are different depending on their type. In our application we have a unique User
class that assigns the credentials depending on user type and does it with an if-else statement. We want to remove it.
class User
{
...
public function initCredentials()
{
if ($this->type == 'admin')
{
$this->addCredential('admin'),
}
elseif($this->type == 'premium')
{
$this->addCredential('premium'),
}
elseif($this->type == 'base')
{
$this->addCredential('base'),
}
else
{
throw new Exception('Error: type is not valid'),
}
}
...
private function addCredential($credential)
{
$this->credentials[] = $credential;
}
...
}
The User
class has not yet been extended, and the user type does not change after creating the object. So we can use subclasses to make our refactoring. Lacking the structure of inheritance, we must first create it using the “Replace type code with subclasses” method.
At the end of this first refactoring we have the super class User
and three subclasses for each type: Admin
, Premium
, and Basic
.
We first write a unit test that confirms the functioning of our method with the new subclasses.
class UserTest extends PHPUnit_Framework_TestCase
{
public function testAdminInitCredentials()
{
$admin = new Admin();
$admin->initCredentials();
$credentials = $admin->getCredentials();
$this->assertEquals('base', $credentials[0]);
$this->assertEquals('credential', $credentials[1]);
$this->assertEquals('admin', $credentials[2]);
}
public function testPremiumInitCredentials()
{
$admin = new Premium();
$admin->initCredentials();
$credentials = $admin->getCredentials();
$this->assertEquals('base', $credentials[0]);
$this->assertEquals('credential', $credentials[1]);
$this->assertEquals('premium', $credentials[2]);
}
public function testBaseInitCredentials()
{
$admin = new Base();
$admin->initCredentials();
$credentials = $admin->getCredentials();
$this->assertEquals('base', $credentials[0]);
$this->assertEquals('credential', $credentials[1]);
}
Run the tests and make sure that everything is ok. The first step would be to move the method in the super class if it is present in the subclass. In our case the method is already in the super class, so we shouldn't move it.
We start from the Admin
class and continue with the next refactoring operations. Create the method setCredential()
, which overrides the same method of the super class, and add onto the method body the leg of conditional logic related to admin type behavior.
class Admin extends User
{
...
public function setCredential()
{
$this->addCredential('admin'),
}
...
}
Remove from the super class method the if block related to admin type. We also need to change the visibility of the method addCredential()
in the super class, from private to protected, otherwise the subclass cannot use it.
class User
{
...
public function setCredential()
{
if($this->type == 'premium')
{
$this->credentials = array_merge($this->getBaseCredential(), 'premium'),
}
elseif($this->type == 'base')
{
$this->credentials = $this->getBaseCredential();
}
throw new Exception('Error on getting credential: type is not valid'),
}
...
private function addCredential($credential)
{
$this->credentials[] = $credential;
}
...
}
Run unit tests and make sure that everything still works properly. At this point we repeat the same activity, first for the Premium
class and then for the Base
class.
class Premium extends User
{
...
public function setCredential()
{
$this->addCredential('premium'),
}
...
}
class Base extends User
{
...
public function setCredential()
{
$this->credentials = $this->getBaseCredential();
}
...
}
After deleting its conditional blocks one at a time, we make the super class and its methods abstract.
abstract class User
{
protected $credentials;
...
abstract public function setCredential();
...
}
Once the variable $type in our User
class will not be used anymore in the conditional logic of our application (since it is replaced by polymorphism), we could remove the variable $type in subclasses.
Finally, we run the test again and make sure everything is working correctly.
In this chapter we learned how to simplify the conditional logic of our code, decomposing it, consolidating it, or removing its control flags. We learned that sometimes we introduce conditional because we code in a procedural way rather than with the object paradigm. To remove these procedural defects, we learned how to replace conditional logic with polymorphism. In the next chapter, we'll discuss how to simplify method calls.
18.188.98.148