C H A P T E R 2
Finding “Bad Smells” in Code
Kent Back's grandmother once said, “When it stinks, change it. ” Obviously she was talking not about code, but about Back's child. Well, we do not believe it, but the same sentence should be totally applied to the development of software.
Martin Fowler talks about this strange history in his book Refactoring: Improving the Design of Existing Code, and Kent Back, the father of Agile Method, helped him write this book. We will take a lot of Martin Fowler and Kent Back's refactoring concepts and try to move them into a PHP context.
Why Code Can Smell
Often, we must work with code that stinks, or rather, to be polite, that emanates bad smells. These odors may result from a bad use of software design practices, from a many-hands writing of the same code, from a code that was written in different periods by different people not using best practices, and much more. Some alarm bells for recognizing that a code smells are
- Difficulty understanding or following the logic of the code
- Many inline comments within the code
- Inability to add new features for fear of introducing bugs
- A lot of files with thousands of lines of code
- Procedural code
If our software is strategic for our company and has at least one of these previous characteristics, then it smells bad. We absolutely have to use some kind of deodorant to make our code smell good again. When faced with a bad code, many software houses decide to rewrite the entire product from scratch. As we have seen, this decision can be lethal for a company. Rewriting a software that is currently a money machine for a company could be really dangerous, because we may not ever release it, fearful of not matching the old product features, or meeting customer needs and users' expectations.
When our code smells good, everything smells around him, we can make happy our leaders doing to grow rapidly the software, according to the needs of business logic, we can have more time to spend with what we value, like family, returning home quietly in the evening, without overtime, we can have better health, preventing sickness liver and hair loss. This is possible because with a clean code, we can embrace change with courage and serenity. Martin Fowler has taught us that, using a variety of code refactoring practices, odors can be easily recognized and eliminated.
Now we will consider the bad smells frequently encountered in a PHP development career. We'll not report all the bad smells listed by Martin Fowler, but only the most representative for the PHP world according to our point of view. For every bad smell, we will learn how to recognize it and what practices we can use to eliminate it. All recommended practices are the official practices taught by Martin Fowler in his book Refactoring: Improving the Design of Existing Code [FOW01]. In this chapter we will not go into details of the practice, as each case will be detailed in following chapters.
Duplicated Code
The most hostile enemy of our code is code duplication. It can cause many problems with software maintenance and performance. For example, if we have duplicate code in the same method of two sibling classes, or if we have the same pieces of code in different methods of the same class, or if we use different algorithms to do the same thing in different methods, and we make a change only at one point and not at all points where the code is duplicated, we can change all the behaviors in a consistent manner, reducing the risk of bugs. We should never forget that software is written in the right way when it is enough to change something in one place in order for the entire system to accept and follow the change uniformly.
To refactor our code we can use various strategies. For example, when we have two equal code blocks within the same class, we can simply extract the method and invoke the call in both places. If instead we have two identical methods in different classes that extend the same class, we can move the method in the parent class and delete it from the other two subclasses. If the code is similar but not exactly the same, we must first extract the method with the equal parts and then move it to the parent class. If we have two methods that do the same thing but with two different algorithms, we can choose which algorithm is the best and use only it and substitute the algorithm. If you have duplicate code in classes that are not related, we can think of creating a parent class and move the method, or if the objects cannot be children of the same class, we may decide to keep the method in a single class and invoke it from a related class, or to create a third class and invoke the method of this third class in the other two. The strategies can be varied—it's up to us to understand what is best and choose it.
In the following example we have two classes that extend the same class. They implement the same getAge method. If the method changes we have to change both methods, and as seen before this is wrong, because if we change only one method while forgetting the other, we can introduce a bug.
class Customer extends Person
{
...
public function getAge()
{
return date('Y') - date('Y', $this->getBirthday());
}
...
}
class Vendor extends Person
{
...
public function getAge()
{
return date('Y') - date('Y', $this->getBirthday());
}
...
}To delete this duplication we can move the getAge() method in the parent class.
class Person
{
...
public function getAge()
{
return date('Y') - date('Y', $this->getBirthday());
}
...
}And we remove the method on subclasses.
Refactoring Strategies
Long Method
Many developers now recognize that the longer a routine is, the more difficult it is to read and maintain. There is a lot of outdated PHP software presenting thousands of code lines in every script. Modifying this code after few months could be an impossible mission. With object-oriented programming, we have learned that the smaller a method is, with the responsibility of the individual classes separated, the simpler the software is to maintain, even after a long time. When we have a lot of methods, it becomes very important to give them meaningful names, so that one can easily remember and understand what each method does, without having to read the code inside.
The most common strategy to refactor this type of software is the extract method. We find pieces of code that go great together and put them in an external method. One heuristic method that we can use to correctly separate the code is to follow the comments. Since reading long code is very difficult, diligent programmers often comment on blocks of code that explain what they do. Here, every time we encounter a comment, we can
- Write a new method in the same class (the name should represent what the code does)
- Move the block of code under the comment from the routine to the new method
- Add a comment that explains not what method does but how it does it
We can also create new methods that contain only a single line of code, if the name of the new method explains better what the code does. The key to this strategy is not so much the length of the method, but the semantic distance between what the method does and how it does it. Extracting methods in many ways, the problem that we will face will be to have long lists of parameters to be passed to methods. Later we will see how to shorten this list of parameters. Even flow conditionals and loops may be a sign that we need to extract the code. We must learn to limit the flow conditionals and loops that make reading code very difficult. A strategy for simplifying these blocks is the decomposition conditional. Another strategy might be to redesign part of our code using some patterns. But we will get back to that later.
In the following example we have the Order class with a method that is too long.
class Order
{
...
public function calculate()
{
$details = $this->getOrderDetails();
foreach($details as $detail)
{
if (!$detail->hasVat())
{
$vat = $this->getCustomer()->getVat();
}
else
{
$vat = $detail->getVat();
}
$price = $detail->getAmount() * $detail->getPrice();
$total += $price + ($price/100 * $vat->getValue());
}
if ($this->hasDiscount())
{
$total = $total - ($total/100 * $this->getDiscount());
}
elseif($this->getCustomer()->hasDiscountForMaxAmount()
&& $total >= $this->getCustomer()->getMaxAmountForDiscount())
{
$total = $total - ($total/100 * $this->getCustomer()->getDiscountForMaxAmount())
}
return $total;
}
...
}
First, we can simplify this method extracting two methods—one to calculate detail total price and another to calculate the right order discount.
class Order
{
...
private function calculateDetailsPrice()
{
foreach($this->getOrderDetails() as $detail)
{
if (!$detail->hasVat())
{
$vat = $this->getCustomer()->getVat();
}
else
{
$vat = $detail->getVat();
}
$price = $detail->getAmount() * $detail->getPrice();
$total += $price + ($price/100 * $vat->getValue());
}
return $total;
}
private function applyDiscount($total)
{
if ($this->hasDiscount())
{
$total = $total - ($total/100 * $this->getDiscount());
}
elseif($this->getCustomer()->hasDiscountForMaxAmount() &&
$total >= $this->getCustomer()->getMaxAmountForDiscount())
{
$total = $total - ($total/100 * $this->getCustomer()->getDiscountForMaxAmount())
}
return $total;
}
public function calculate()
{
return $this->applyDiscount($this->calculateDetailsPrice());
}
...
}
If we look carefully at the method calculateDetailsPrice(), we will realize that all code inside the foreach cycle is about order detail object and not order object. So we can extract this code and move it into the OrderDetail class. We can extract two methods—one to retrieve the right vat object and another to calculate detail price.
class OrderDetail
{
...
public function getVat()
{
if (!$this->hasVat())
{
return $this->getOrder()->getCustomer()->getVat();
}
return $this->getVat();
}
public function calculate()
{
$price = $this->getAmount() * $this->getPrice();
return $price + ($price/100 * $this->getVat()->getValue());
}
...
}
class Order
{
...
private function calculateDetailsPrice()
{
foreach($this->getOrderDetails() as $detail)
{
$total += $detail->calculate();
}
return $total;
}
...
}
Refactoring Strategies
Large Class
A class that tries to do too many things can be easily distinguished by the number of attributes it possesses. When a class has too many attributes, it is very easy to create duplicate code, making it difficult to maintain and read.
The two best strategies are to combine the attributes that go well together, such as attributes with the same prefixes or suffixes, e.g., home_address and office_address, or fax_number and phone_number, and extract new classes that include these similar attributes. If the attributes belong strongly to class and can't be merged, but they are used differently in different instances of the same class, we can extract subclasses and divide these attributes in these subclasses. Or, if we just have to extract attributes and non-logical behavior, we can use interfaces rather than classes.
In the following example we have a long class Order that has a lot of properties.
Class Order
{
protected $customer_firstname;
protected $customer_lastname;
protected $customer_company_name;
protected $customer_address;
protected $customer_city;
protected $customer_country;
protected $customer_phonenumber;
protected $customer_faxnumber;
protected $customer_email;
protected $customer_$vat;
protected $order_id;
protected $order_number;
protected $order_discount;
protected $order_total_price;
protected $order_date;
protected $order_shipping_date;
protected $payment_transaction_id;
protected $payment_type;
protected $payment_date;
protected $payment_method;
...
}
We also have accessor methods to access to each property. In cases like this, we are giving too much liability to one class only. To reduce the liability and the code length, we can extract new classes and move each property and accessor method to the right new class.
class Customer
{
protected $firstname;
protected $lastname;
protected $company_name;
protected $address;
protected $city;
protected $country;
protected $phonenumber;
protected $faxnumber;
protected $email;
protected $vat;
protected $payment_method;
...
}
class Payment
{
protected $transaction_id;
protected $type;
protected $date;
protected $method;
...
}
class Order
{
protected $id;
protected $number;
protected $discount;
protected $total_price;
protected $date;
protected $shipping_date;
...
}
Refactoring Strategies
Long Parameter List
With procedural programming we often use lots of long lists of parameters passed to functions. Unfortunately, this method used to be mandatory, since the only alternative was to use global variables, which, we know, are evil. With object-oriented programming, luckily, we no longer need long lists of parameters, for two main reasons:
- There are objects.
- A class method knows all private, protected, and public parameters of the class where it's implemented and all protected and public parameters of the inherit class.
Moreover, when we have parameters that can be grouped into a single object we can introduce a parameter object and pass it directly to the method rather than individual parameters. Instead, if we are passing the individual attributes of the same object, we can preserve the whole object and pass the object directly. If we invoke a method and then pass the result directly to another method, we can replace the parameter with the method and call it directly into the later-invoked method.
The only exception for which these strategies can't be applied is when we don't want to create dependency between object and class. In this case we can pass all the attributes separately, but we must be aware of what this means. Before making this decision we have to review the design of our application to understand whether we can improve it to avoid a bad smell.
In the following example we have a User class and a template CSV class that has a method to render a CSV user row.
class User
{
public $first_name:
public $last_name;
public $type;
public $email;
public $address;
public $city;
public $country;
public $gender;
...
}
class UserCsvTemplate
{
public function render($first_name, $last_name, $type, $email, $address, $city, $country, $gender)
{
echo $first_name, ';', $last_name, ';',
$type, ';', $email, ';',
$address, ';', $city, ';',
$country, ';', $gender, PHP_EOL;
}
}
If we want to render a user CSV row we need to run the following code:
$user = new User();
...
$csv_template = new UserCsvTemplate();
$csv_template->render($user->first_name, $user->last_name, $user->type, $user->email,
$user->address, $user->city, $user->country, $user->gender);
The render() method has a very long list of parameters to be passed. Since all the parameters belong to the same object we can pass the entire object directly, preserving it.
class UserCsvTemplate
{
public function render(User $user)
{
echo $user->first_name, ';', $user->last_name, ';',
$user->type, ';', $user->email, ';',
$user->address, ';', $user->city, ';',
$user->country, ';', $user->gender, PHP_EOL,
}
}
$user = new User();
...
$csv_template = new UserCsvTemplate();
$csv_template->render($user);
Refactoring Strategies
Divergent Change
When we change a certain feature in software that is truly moldable, we should be able to make the change in a single clear point of our system. When this doesn't happen, and, instead, we must change multiple methods of the same class to make different changes, then we have done something wrong.
For example, if we have to modify two methods of the same class to change the database connection, and we have to change four methods to add a new graphical interface to a particular component in the same class, it is probably better to split this class into two classes, so as to isolate the change in a single point. The strategy applied is the extract class. We create a new class; we extract the common methods in the new class and let it communicate in some way.
In the following example we have two methods that retrieve their records from the same MySql database.
class OrderRepository
{
public static function retrieveAll()
{
$connection = mysql_connect('localhost', 'user', 'user')
or throw new Exception('Could not connect: ' . mysql_error());
mysql_select_db('ecommerce')
or throw new Exception('Could not select database'),
$result = mysql_query('SELECT * FROM order')
or throw new Exception('Query failed: ' . mysql_error());
...
}
public static function retrieveOneById($id)
{
$connection = mysql_connect('localhost', 'user', 'user')
or throw new Exception('Could not connect: ' . mysql_error());
mysql_select_db('ecommerce')
or throw new Exception('Could not select database'),
$result = mysql_query('SELECT * FROM orde where id = '.$id)
or throw new Exception('Query failed: ' . mysql_error());
...
}
}
If we want to change the database name, or the database user configuration, we need to change the same line of code in two different methods. If we forget to change one of these, we can introduce a bug. In this case we can extract a new class and instance it inside the method.
class Connection
{
private $connection;
public function __construct()
{
$this->connection = mysql_connect('localhost', 'user', 'user')
or throw new Exception('Could not connect: ' . mysql_error());
mysql_select_db('ecommerce')
or throw new Exception('Could not select database'),
}
...
}
class CustomerRepository
{
public static function retrieveAll()
{
$connection = new Connection();
$result = mysql_query('SELECT * FROM customer')
or throw new Exception('Query failed: ' . mysql_error());
...
}
public static function retrieveAll()
{
$connection = new Connection();
$result = mysql_query('SELECT * FROM order')
or throw new Exception('Query failed: ' . mysql_error());
...
}
}
Refactoring Strategies
Shotgun Surgery
This bad smell is the perfect opposite of the previous divergent change. In practice, when we have to modify many classes to make a single change, this means that we have a design overhead.
When we have to change two different methods of two different classes, we can move the method in the most representative class. If we instead have to change attributes of different classes, we can move only attributes in the most representative class. Sometimes we might remove all classes and move all attributes within the class that uses it. This strategy takes the name of inline class.
Refactoring Strategies
Feature Envy
When we write our software, we must never forget that an object is a set of data and processes that compute these data. This thought should help us to identify all the methods that do not process the data in our class, but data of the other classes, invoking only external methods. As you can imagine, these methods are in the wrong position. To resolve this situation we can simply move the method from one class to another.
If only a part of the code must be moved, first extract it in a new method and then move it. If our code actually invokes methods of different classes, it can sometimes be complex to figure out where to move the method. When we are in trouble, the strategy is always the same—extract more methods and move everyone in your class.
class Order
{
public function calculate()
{
foreach($this->getOrderDetails() as $detail)
{
$this->total += $this->calcultateOrderDetail($detail);
}
}
public function calcultateOrderDetail($detail)
{
return $detail->getAmount() * $detail->getPrice();
}
}
The calculateOrderDetail is a method that doesn't belong to Order class. This method calls only methods of OrderDetail class. We need to move this method to OrderDetail class.
class OrderDetail
{
public function calcultateOrderDetail()
{
return $this->getAmount() * $this->getPrice();
}
}
class Order
{
public function calculate()
{
foreach($this->getOrderDetails() as $detail)
{
$this->total += $detail->calcultateOrderDetail();
}
}
}
Refactoring Strategies
Data Clamps
When our classes start to proliferate groups of attributes that must be changed all at once when one inserts a new feature or change, we have to stop and rewrite that attribute group differently.
The first strategy is to extract these attributes and put them together into a new class, because surely all of these attributes can be represented by another entity. Then declare a new class, extract the attributes in the new class, and add a single attribute in the class that is an instance of the new class. Then we can concentrate on simplifying the parameter list of those methods that use these attributes, preserving the whole object.
Refactoring Strategies
Primitive Obsession
Many programming languages, including PHP, provide two basic data types—the primitive type and structured type, which we represent with associative arrays in PHP. The structured data is used when none of the system primitives provided can properly represent certain data. A very common example is a record in a database, or a matrix. These data types are often duplicated within our code and, as we have seen before, are not good, because the duplicated code is the first bad smell. In this case the objects are the solution to our problem. The object itself is just the right compromise between primitive data and structured data. In this way we can replace our data with objects. For example, PHP supports integers, strings, and primitive booleans, but not the date data. So to represent our data, we can create a Date custom class, or use the Date class that PHP already makes available to us. There are many other cases where it can be very helpful to use objects instead of structured data, such as when we have to represent currency, or area code. We often resist creating this type of object. But if we strive instead to use them, our code can become much easier to read and less duplicated.
To implement this type of class, the strategy that we can use is to replace data value with an object, but if we find type code that is an alias for a structured data, we can replace it with a class or subclass it represents. If we have a condition that depends on the type of a given data, we can replace type code with a subclass or with a state/strategy pattern.
In the following example we retrieve a record from a database, fetching it with array hydration mode.
$id = 0;
$db = new PDO('mysql:dbname=mytest', 'root'),
$stmt = $db->prepare('SELECT * FROM user WHERe id = :id'),
$stmt->bindParam(':id', $id, PDO::PARAM_INT);
$stmt->execute();
$user = $stmt->fetch(PDO::FETCH_ASSOC);
echo $user['firstname'], ' ', $user['lastname'], PHP_EOL;
Instead we can define a User class and use it to hydrate query result.
class User
{
protected $firstname;
protected $lastname;
public function __toString()
{
return $this->firstname. ' '. $this->lastname;
}
}
...
$stmt->execute();
$user = $stmt->fetchObject('User'),
echo $user, PHP_EOL;
Refactoring Strategies
Switch Statements
Another widespread bad smell is the use of repeated and duplicate switches. The switch control structure usually seeks to change the behavior or the state of an object based on one or more parameters. Using switches in this way, we forget that in object-oriented programming this type of behavior has the name “polymorphism” and is a property of objects themselves.
Through the polymorphism property we can remove conditional logic, since the change occurs according to the type of the object itself, not because someone knows how to choose based on the value of a given parameter. The strategy that we implement to remove the switches is as follows:
- Extract the switch in a method.
- Move the newly extracted method in a class more accountable to the knowledge of polymorphism.
- Decide whether to replace the type code executed by the switch with a subclass, or a state/strategy pattern.
- After setting the structure of inheritance, we can use the replace conditional with the polymorphism strategy.
Sometimes the polymorphism may not be the appropriate solution. In this case we can simply replace the condition code with the methods, and in cases where the condition must return null, we can think of introducing a NullObject.
class User
{
...
public function initCredentials()
{
switch($this->type)
{
case 'admin':
$this->addCredential('admin'),
break;
case 'premium':
$this->addCredential('premium'),
break;
case 'base':
$this->addCredential('base'),
break;
default:
throw new Exception('Error: type is not valid'),
break;
}
}
...
}
For example, the following switch statement should be removed using the polymorphism strategy.
Refactoring Strategies
Lazy Class
All classes or subclasses that we make must have a reason to exist. If you think a class is not doing enough and is not very useful, it must be removed, because it costs a lot to maintain. If a subclass is not used in a class, we can collapse the inheritance hierarchy. If an attribute is an instance of a class of which we use only a single attribute or a single method, let's make it a class inline into the referred class.
Refactoring Strategies
Speculative Generality
Developers often commit the sin of pride, thinking that they can predict the future. How many times have we decided to continue with the amount of code produced because we believed that someday it would be useful, for example, to have one abstract class, or a certain method, or that particular interface, only to find that such a day never arrives? Unfortunately, we must accept that we cannot predict the future. To have a good-smelling code, we must remove all the speculative code, such as code that nobody uses except for a test case. We must think only about what we need today, not tomorrow.
For this bad smell, the strategies are similar to bad smells we have already discussed. In fact, we can use the strategy of collapse hierarchy for those subclasses that are not used, or the inline class to remove delegations from classes that are not really useful. We can also remove parameters from methods that never use them, and rename all the methods that are not sufficiently expressive.
As previously suggested, a nice way to find this kind of speculation is when we realize that the only users of these methods are the test cases. So, remove the method and the associated test cases.
Refactoring Strategies
Temporary Field
In object-oriented programming, all the attributes of a certain class should characterize that object in any context. Often when we implement our code, it happens that some of the attributes are used only in one context and the others in another context, perhaps chosen by a given parameter. This type of behavior is wrong because it introduces a lot of unnecessary conditional logic within our code. Instead we can safely extract a subclass that better characterizes the object in that kind of context.
Therefore, the strategy is to extract a subclass and add the attributes that in a certain context do not represent the object. If the condition allows no object if the variable is null, we can introduce a NullObject to handle this case.
class Customer
{
protected $company_name;
protected $first_name;
protected $last_name;
protected $is_company;
public function getFullName()
{
if ($this->is_company)
{
return $this->getCompanyName();
}
return $this->getFirstName() . ' ' . $this->getLastName();
}
}
In the previous class we have a $first_name and $last_name that are temporary properties of our class. They are used only when Customer is not a company, so they are context-dependent. We can remove this double behavior, extracting two subclasses and making Customer an abstract class.
abstract class Customer
{
abstract public function getFullName();
}
class Person extends Customer
{
protected $first_name;
protected $last_name;
public function getFullName()
{
return $this->getFirstName(), ' ', $this->getLastName();
}
}
class Company extends Customer
{
protected $company_name;
public function getFullName()
{
return $this->getCompanyName();
}
}
Refactoring Strategies
Data Class
The data classes are all those classes that only have to set and get a set of their attributes, without having some business logic. They are also called setter/getter classes. Usually these classes, when born, publicly expose all their attributes, and this behavior may be too brash. When the class is small and we have it under control, the problem will be minimal, but when these classes grow, if we are not careful, we risk exposing too much. These classes are like children—while they are growing, they must assume some responsibility.
The first suggestion is to set all the attributes of those classes to private or protected, and create a series of getter and setter that may provide public access to these attributes. This strategy is known as encapsulation field. When an attribute is a collection of objects or structured data, we must do the same thing because the collection can't be changed all at once—only one item at a time. This strategy is called encapsulate the collection. Moreover, if we have attributes that should never be modified outside, we should not remove setting methods that modify them. If there is an object that uses the setter on the attributes that should not be changed, extract the external method into our data class and related external object to it, so that the change is internal.
Refactoring Strategies
Comments
When we say that comments are a source of bad smells, hordes of programmers may become angry. In our opinion, as that of Martin Fowler, the point is that a comment is not a bad smell, but when the comment is inline to code and it is used to explain what the code does, this is a bad smell. All method or class comments are important and should be used to explain how something works, but not what it does. To remove this bad smell, you can implement the following strategy whenever you want to write a comment to a block of code:
- Instead of the comment, write a method where the method name expresses exactly the same concept of the comment.
- Extract the block of code that we wanted to comment in the method.
- Invoke the method where we wanted to put the comment.
But if we want to write a comment about what a certain call to method does because it isn't clear, rename the method with a more expressive name and reduce the list of parameters passed, introducing a parameter object or preserving the whole object via parameters or creating objects of any object. If you need to remember some rules required by the system in a certain context, instead of writing the comment, introduce a method that tests the assertion rule.
When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous.
—Martin Fowler
Comments may make code even more malodorous. But they can be helpful by letting others know that you are not sure of what has been done or that you do not know what to do in that particular context or piece of code.
class Order
{
...
public function calculate()
{
foreach($this->getOrderDetails() as $detail)
{
// Retrieve vat
if ($detail->hasVat())
{
$detail_vat = $detail->getVat();
}
// Calculate details price
$detail_price = $detail->getAmount() * $detail->getPrice();
// Calculate details price adding vat value
$order_total += $detail_price + ($detail_price/100 * $detail_vat->getValue());
}
// Retrieve order discount
if ($this->hasDiscount())
{
$order_total = $order_total - ($order_total/100 * $this->getDiscount());
}
return $total;
}
...
}
In the previous method we can remove the inline comment, extracting some new private methods.
class Order
{
...
private function retrieveVat($detail)
{
return $detail->hasVat() ? $detail->getVat() : 0;
}
private function calculateDetailPrice($detail)
{
return $detail->getAmount() * $detail->getPrice();
}
private function calculateDeatilPriceAddingVatValue($price, $vat)
{
return $price + ($price/100 * $vat->getValue());
}
private function calculatePriceOrderWithDiscount($total)
{
if ($this->hasDiscount())
{
return $total - ($total/100 * $this->getDiscount());
}
return $total;
}
public function calculate()
{
foreach($this->getOrderDetails() as $detail)
{
$detail_vat = $this->retrieveVat($detail);
$detail_price = $this->calculateDetailPrice($detail);
$order_price += $this->calculateDeatilPriceAddingVatValue($price, $vat);
}
return $this->calculateOrderPriceWithDiscount();
}
...
}
Refactoring Strategies
Procedural Code
PHP is an easy and affordable language for unskilled developers to learn. Unfortunately, this advantage can sometimes become a disadvantage. Writing good code is not enough to know a language. As we have seen, we must have much deeper knowledge about design, patterns, and especially object-oriented programming. Today, PHP is a mature and powerful language that lets you use all the best practices of object-oriented programming. Well, we should use them. The procedural or functional code is surely an easy and affordable way to write code, but when that code involves thousands of duplicate lines, which are difficult to read and maintain, our code loses the ability to grow with our needs and we lose the true value of our software.
There is a strategy called big refactoring, which we will see at the end of this book, which turns your application from procedural code to object-oriented code. This is a complex activity but it is worth the effort.
Refactoring Strategies