Translation instructions
This article is translated and synchronized by PHP-CPM based on yangweijie version of clean-code-php.
The original text is updated frequently, and my translation method is to directly use the text comparison tool line by line comparison. Make sure the text content is up to date first, then gradually improve the quality of translation.
You are welcome to submit your PR if you encounter any link failures, old content, incorrect terminology and other translation errors.
Although many developers still use PHP5, most of the examples in this article run on PHP 7.1+.
- introduce
- variable
- Use variable names that are visible
- Use the same variable name for the same entity
- Use searchable names (Part 1)
- Use searchable names (Part 2)
- Use self-explanatory variables
- Use self-explanatory variables
- Avoid deep nesting, return early (Part 1)
- Avoid deep nesting, return early (Part 2)
- Use less meaningless variable names
- Don’t add unnecessary context
- Use parameter defaults wisely. There is no need to check default values in methods
- function
- Function arguments (preferably less than 2)
- The function should only do one thing
- The function name should be a meaningful verb (or an indication of what is being done)
- There should be only one layer of abstract abstraction in the function
- Do not use flag as an argument to functions
- Avoid side effects
- Don’t write global functions
- Do not use the singleton pattern
- Encapsulating conditional statement
- Avoid using antisense conditions
- Avoid conditional judgment
- Avoiding Type Checking (Part 1)
- Avoiding Type Checking (Part 2)
- Removing zombie code
- Objects and Data Structures
- Use getters and setters Use object Encapsulation
- Object attributes are mostly private/protected qualified
- class
- Composition over Inheritance
- Avoid coherent interfaces
- Class SOLID principle SOLID
- S: Single Responsibility Principle (SRP)
- O: Open/Closed Principle (OCP)
- L: Liskov Substitution Principle (LSP)
- I: Interface Segregation Principle (ISP)
- D: Dependency Inversion Principle (DIP)
- Don’t write duplicate code (DRY)
- translation
introduce
This article is based on principles for Software Engineers in Robert C. Martin’s Clean Code book and applies to PHP. This is not a style guide. This is a guide to developing readable, reusable, and reconfigurable PHP software.
Not all of these principles are followed, and few are universally accepted. These are just guidelines, but they have been developed over the years by Clean Code’s authors.
This article was inspired by clean-code-javascript
Use variable names that are visible
The bad:
$ymdstr = $moment->format('y-m-d');Copy the code
Good:
$currentDate = $moment->format('y-m-d');Copy the code
Use the same variable name for the same entity
The bad:
getUserInfo();
getUserData();
getUserRecord();
getUserProfile();Copy the code
Good:
getUser();Copy the code
Use searchable names (Part 1)
Code is written to read. So writing readable, searchable code is critical. Naming variables that don’t make sense and are hard to understand does the reader a disservice. Make your code searchable.
The bad:
// What the heck is 448 for?
$result = $serializer->serialize($data.448);Copy the code
Good:
$json = $serializer->serialize($data.JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);Copy the code
The bad:
// What the heck is 4 for?
if ($user->access & 4) {
//.
}Copy the code
Good:
class User
{
const ACCESS_READ = 1;
const ACCESS_CREATE = 2;
const ACCESS_UPDATE = 4;
const ACCESS_DELETE = 8;
}
if ($user->access & User: :ACCESS_UPDATE) {
// do edit ...
}Copy the code
Pass back to top
Use self-explanatory variables
The bad:
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\ \]+ [,\ \ \s]+(.+?).\s*(\d{5})?$/ ';
preg_match($cityZipCodeRegex.$address.$matches);
saveCityZipCode($matches[1].$matches[2]);Copy the code
Good:
Better, but strongly dependent on regular expression familiarity
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\ \]+ [,\ \ \s]+(.+?).\s*(\d{5})?$/ ';
preg_match($cityZipCodeRegex.$address.$matches);
[, $city.$zipCode] = $matches;
saveCityZipCode($city.$zipCode);Copy the code
Good:
Use named subrules that can be read without knowing the regular
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\ \]+ [,\ \ \s]+(? <city>.+?).\s*(? <zipCode>\d{5})?$/ ';
preg_match($cityZipCodeRegex.$address.$matches);
saveCityZipCode($matches['city'].$matches['zipCode']);Copy the code
Avoid deep nesting, return early (Part 1)
Too many if else statements usually make your code hard to read, and plain is better than obscure
Bad:
function isShopOpen($day) :bool
{
if ($day) {
if (is_string($day)) {
$day = strtolower($day);
if ($day = = = 'friday') {
return true;
} elseif ($day = = = 'saturday') {
return true;
} elseif ($day = = = 'sunday') {
return true;
} else {
return false;
}
} else {
return false;
}
} else {
return false;
}
}Copy the code
Ok:
function isShopOpen(string $day) :bool
{
if (empty($day)) {
return false;
}
$openingDays = [
'friday'.'saturday'.'sunday'
];
return in_array(strtolower($day), $openingDays);
}Copy the code
Avoid deep nesting, return early (Part 2)
The bad:
function fibonacci(int $n)
{
if ($n < 50) {
if ($n != = 0) {
if ($n != = 1) {
return fibonacci($n - 1) + fibonacci($n - 2);
} else {
return 1;
}
} else {
return 0;
}
} else {
return 'Not supported';
}
}Copy the code
Ok:
function fibonacci(int $n) :int
{
if ($n = = = 0) {
return 0;
}
if ($n = = = 1) {
return 1;
}
if ($n > 50) {
throw new \Exception('Not supported');
}
return fibonacci($n - 1) + fibonacci($n - 2);
}Copy the code
Use less meaningless variable names
Don’t make people reading your code guess what the variables you write mean. Clear writing is better than vague writing.
The bad:
$l = ['Austin'.'New York'.'San Francisco'];
for ($i = 0; $i < count($l); $i++) {
$li = $l[$i];
doStuff();
doSomeOtherStuff();
//.
//.
//.
//Wait, what does' $li 'stand for?
dispatch($li);
}Copy the code
Good:
$locations = ['Austin'.'New York'.'San Francisco'];
foreach ($locations as $location) {
doStuff();
doSomeOtherStuff();
//.
//.
//.
dispatch($location);
}Copy the code
Don’t add unnecessary context
If you already know something from your class name or object name, don’t repeat it in the variable name.
The bad:
class Car
{
public $carMake;
public $carModel;
public $carColor;
//.
}Copy the code
Good:
class Car
{
public $make;
public $model;
public $color;
//.
}Copy the code
Use parameter defaults wisely. There is no need to check default values in methods
Bad:
This is not good because $breweryName
can be NULL
.
function createMicrobrewery($breweryName = 'Hipster Brew Co.') :void
{
//.
}Copy the code
Ok:
This opinion is more understandable than the previous version, but it better controls the value of the variable.
function createMicrobrewery($name = null) :void
{
$breweryName = $name? :'Hipster Brew Co.';
//.
}Copy the code
Good:
If you support only PHP 7+, then you can use type hinting and be sure that the $breweryName
will not be NULL
.
function createMicrobrewery(string $breweryName = 'Hipster Brew Co. ') :void
{
//.
}Copy the code
Pass back to top
function
Limiting the number of arguments to a function is extremely important so that testing your function is easier. With more than 3 optional parameter parameters resulting in an explosive combination of growth, you will have tons of independent parameter cases to test.
No parameters is the ideal case. One or two is fine. Avoid three. Any more and you’ll need reinforcement. Usually if your function has more than two arguments, it has too many things to deal with. If you must pass in a lot of data, it is recommended to encapsulate a high-level object as a parameter.
The bad:
function createMenu(string $title.string $body.string $buttonText.bool $cancellable) :void
{
//.
}Copy the code
Good:
class MenuConfig
{
public $title;
public $body;
public $buttonText;
public $cancellable = false;
}
$config = new MenuConfig(a);
$config->title = 'Foo';
$config->body = 'Bar';
$config->buttonText = 'Baz';
$config->cancellable = true;
function createMenu(MenuConfig $config) :void
{
//.
}Copy the code
Pass back to top
The function should only do one thing
This is by far the most important rule in software engineering. When functions do more than one thing, they are difficult to implement, test, and understand. When you break a function down to just one function, they’re easier to refactor, and your code reads clearer. If you just follow this rule, you’ll be ahead of most developers.
The bad:
function emailClients(array $clients) :void
{
foreach ($clients as $client) {
$clientRecord = $db->find($client);
if ($clientRecord->isActive()) {
email($client);
}
}
}Copy the code
Good:
function emailClients(array $clients) :void
{
$activeClients = activeClients($clients);
array_walk($activeClients.'email');
}
function activeClients(array $clients) :array
{
return array_filter($clients.'isClientActive');
}
function isClientActive(int $client) :bool
{
$clientRecord = $db->find($client);
return $clientRecord->isActive();
}Copy the code
The function name should be a meaningful verb (or an indication of what is being done)
The bad:
class Email
{
//.
public function handle() :void
{
mail($this->to.$this->subject.$this->body);
}
}
$message = new Email(.);
//What? What does handle do with a message? Write code into a file?
$message->handle();Copy the code
Good:
class Email
{
//.
public function send() :void
{
mail($this->to.$this->subject.$this->body);
}
}
$message = new Email(.);
//Simple and clear
$message->send();Copy the code
There should be only one layer of abstract abstraction in the function
When you have too many levels of abstraction, functions do too many things. Split functionality is needed to improve reusability and ease of use in order to simplify testing. (Too much nesting)
The bad:
function parseBetterJSAlternative(string $code) :void
{
$regexes = [
//.
];
$statements = explode(' '.$code);
$tokens = [];
foreach ($regexes as $regex) {
foreach ($statements as $statement) {
//.
}
}
$ast = [];
foreach ($tokens as $token) {
// lex...
}
foreach ($ast as $node) {
// parse...
}
}Copy the code
The bad:
We pulled some methods out of the loop, but the parseBetterJSAlternative() method was still complex and not very testing.
function tokenize(string $code) :array
{
$regexes = [
//.
];
$statements = explode(' '.$code);
$tokens = [];
foreach ($regexes as $regex) {
foreach ($statements as $statement) {
$tokens[] = / *.* /;
}
}
return $tokens;
}
function lexer(array $tokens) :array
{
$ast = [];
foreach ($tokens as $token) {
$ast[] = / *.* /;
}
return $ast;
}
function parseBetterJSAlternative(string $code) :void
{
$tokens = tokenize($code);
$ast = lexer($tokens);
foreach ($ast as $node) {
// parse...
}
}Copy the code
Good:
The best solution is to remove the dependency of the parseBetterJSAlternative() method.
class Tokenizer
{
public function tokenize(string $code) :array
{
$regexes = [
//.
];
$statements = explode(' '.$code);
$tokens = [];
foreach ($regexes as $regex) {
foreach ($statements as $statement) {
$tokens[] = / *.* /;
}
}
return $tokens;
}
}
class Lexer
{
public function lexify(array $tokens) :array
{
$ast = [];
foreach ($tokens as $token) {
$ast[] = / *.* /;
}
return $ast;
}
}
class BetterJSAlternative
{
private $tokenizer;
private $lexer;
public function __construct(Tokenizer $tokenizer.Lexer $lexer)
{
$this->tokenizer = $tokenizer;
$this->lexer = $lexer;
}
public function parse(string $code) :void
{
$tokens = $this->tokenizer->tokenize($code);
$ast = $this->lexer->lexify($tokens);
foreach ($ast as $node) {
// parse...
}
}
}Copy the code
So that we can do the mock to rely on, and test the BetterJSAlternative: : parse () operation is in line with expectations.
Do not use flag as an argument to functions
Flag is just telling you that there are a lot of things to do in this method. As I said earlier, a function should only do one thing. Split the code of different flags into multiple functions.
The bad:
function createFile(string $name.bool $temp = false) :void
{
if ($temp) {
touch('./temp/'.$name);
} else {
touch($name);
}
}Copy the code
Good:
function createFile(string $name) :void
{
touch($name);
}
function createTempFile(string $name) :void
{
touch('./temp/'.$name);
}Copy the code
Avoid side effects
A function that does more than get a value and then return another value or values can have side effects if. The side effects could be writing a file, modifying some global variable or accidentally giving all your money to a stranger.
Now, you do need to have side effects in a program or situation, as in the previous example, you might need to write a file. What you want to do is centralize the places where you do this. Don’t use several functions and classes to write to a particular file. Do it with a service, one at a time.
The emphasis is on avoiding common pitfalls such as sharing unstructured data between objects, using mutable data types that can be written to any object, and not focusing on where side effects occur. If you do that you’ll be happier than most programmers.
The bad:
// Global variable referenced by following function.
// If we had another function that used this name, now it'd be an array and it could break it.
$name = 'Ryan McDermott';
function splitIntoFirstAndLastName() :void
{
global $name;
$name = explode(' '.$name);
}
splitIntoFirstAndLastName();
var_dump($name); // ['Ryan', 'McDermott'];Copy the code
Good:
function splitIntoFirstAndLastName(string $name) :array
{
return explode(' '.$name);
}
$name = 'Ryan McDermott';
$newName = splitIntoFirstAndLastName($name);
var_dump($name); // 'Ryan McDermott';
var_dump($newName); // ['Ryan', 'McDermott'];Copy the code
Don’t write global functions
Contaminating global variables is a bad practice in most languages, because you may collide with other libraries and the person calling your API won’t know about the pit until they catch the exception. Let’s consider one scenario: if you want to configure an array, you might write a global function config(), but it might conflict with other libraries trying to do the same thing.
The bad:
function config() :array
{
return [
'foo' = > 'bar'.
]
}Copy the code
Good:
class Configuration
{
private $configuration = [];
public function __construct(array $configuration)
{
$this->configuration = $configuration;
}
public function get(string $key): ?string
{
return isset($this->configuration[$key])?$this->configuration[$key] : null;
}
}Copy the code
Load configuration and create instance of Configuration
class
$configuration = new Configuration([
'foo' = > 'bar'.
]);Copy the code
And now you must use instance of Configuration
in your application.
Do not use the singleton pattern
Singletons are an anti-pattern. Paraphrased from Brian Button:
- They are generally used as a global instance, why is that so bad? Because you hide the dependencies of your application in your code, instead of exposing them through the interfaces. Making something global to avoid passing it around is a code smell.
- They violate the single responsibility principle: by virtue of the fact that they control their own creation and lifecycle.
- They inherently cause code to be tightly coupled. This makes faking them out under test rather difficult in many cases.
- They carry state around for the lifetime of the application. Another hit to testing since you can end up with a situation where tests need to be ordered which is a big no for unit tests. Why? Because each unit test should be independent from the other.
There is also very good thoughts by Misko Hevery about the root of problem.
The bad:
class DBConnection
{
private static $instance;
private function __construct(string $dsn)
{
//.
}
public static function getInstance() :DBConnection
{
if (self: :$instance = = = null) {
self: :$instance = new self(a);
}
return self: :$instance;
}
//.
}
$singleton = DBConnection: :getInstance();Copy the code
Good:
class DBConnection
{
public function __construct(string $dsn)
{
//.
}
//.
}Copy the code
Create instance of DBConnection
class and configure it with DSN.
$connection = new DBConnection($dsn);Copy the code
And now you must use instance of DBConnection
in your application.
Pass back to top
Encapsulating conditional statement
The bad:
if ($article->state = = = 'published') {
//.
}Copy the code
Good:
if ($article->isPublished()) {
//.
}Copy the code
Avoid using antisense conditions
The bad:
function isDOMNodeNotPresent(\DOMNode $node) :bool
{
//.
}
if (!isDOMNodeNotPresent($node))
{
//.
}Copy the code
Good:
function isDOMNodePresent(\DOMNode $node) :bool
{
//.
}
if (isDOMNodePresent($node)) {
//.
}Copy the code
Avoid conditional judgment
It seems like an impossible task. That’s what people say when they first hear it. “What else can I do without the if statement?” The answer is that you can use polymorphism to accomplish the same task in multiple scenarios. The second question is very common, “It’s ok, but why am I doing this?” The answer is a Clean Code principle we learned earlier: a function should only do one thing. When you have many classes and functions that contain if statements, your function does more than one thing. Remember, just do one thing.
The bad:
class Airplane
{
//.
public function getCruisingAltitude() :int
{
switch ($this->type) {
case '777':
return $this->getMaxAltitude() - $this->getPassengerCount();
case 'Air Force One':
return $this->getMaxAltitude();
case 'Cessna':
return $this->getMaxAltitude() - $this->getFuelExpenditure();
}
}
}Copy the code
Good:
interface Airplane
{
//.
public function getCruisingAltitude() :int;
}
class Boeing777 implements Airplane
{
//.
public function getCruisingAltitude() :int
{
return $this->getMaxAltitude() - $this->getPassengerCount();
}
}
class AirForceOne implements Airplane
{
//.
public function getCruisingAltitude() :int
{
return $this->getMaxAltitude();
}
}
class Cessna implements Airplane
{
//.
public function getCruisingAltitude() :int
{
return $this->getMaxAltitude() - $this->getFuelExpenditure();
}
}Copy the code
Avoiding Type Checking (Part 1)
PHP is weakly typed, which means your functions can accept arguments of any type. Sometimes you suffer from this freedom and gradually try type checking in your functions. There are ways to avoid this. The first is a unified API.
The bad:
function travelToTexas($vehicle) :void
{
if ($vehicle instanceof Bicycle) {
$vehicle->pedalTo(new Location('texas'));
} elseif ($vehicle instanceof Car) {
$vehicle->driveTo(new Location('texas'));
}
}Copy the code
Good:
function travelToTexas(Traveler $vehicle) :void
{
$vehicle->travelTo(new Location('texas'));
}Copy the code
Avoiding Type Checking (Part 2)
If you are using primitive values such as strings, integers, and arrays, requiring PHP 7+, no polymorphism, and type detection, you should consider type declarations or strict schemas. Provides static typing based on standard PHP syntax. The problem with manually checking types is that doing so requires a lot of nonsense, as if the loss of readability can be compromised for safety. Keep your PHP clean, write tests, and do code reviews. If not, use PHP’s strict type declarations and strict schemas to ensure security.
The bad:
function combine($val1.$val2) :int
{
if (!is_numeric($val1) || !is_numeric($val2)) {
throw new \Exception('Must be of type Number');
}
return $val1 + $val2;
}Copy the code
Good:
function combine(int $val1.int $val2) :int
{
return $val1 + $val2;
}Copy the code
Removing zombie code
Zombie code is just as bad as duplicate code. There’s no reason to keep it in your code base. If it’s never been called, delete it! It’s safe because it’s still in the repository.
The bad:
function oldRequestModule(string $url) :void
{
//.
}
function newRequestModule(string $url) :void
{
//.
}
$request = newRequestModule($requestUrl);
inventoryTracker('apples'.$request.'www.inventory-awesome.io');Copy the code
Good:
function requestModule(string $url) :void
{
//.
}
$request = requestModule($requestUrl);
inventoryTracker('apples'.$request.'www.inventory-awesome.io');Copy the code
Objects and data structures
In PHP you can use public, protected, and private methods to control object property changes.
- When you want to do something other than fetch on an object property, you don’t need to find and modify every access method to that property in your code
- When you have
set
The corresponding attribute method is easy to add parameter verification - Encapsulate the internal representation
- Using the setAnd get, easy to add logging and error control
- When inheriting from the current class, you can override the default method functionality
- Get *, set* are easy to use lazy loading when object properties are retrieved from a remote server
In addition, this approach conforms to the open close principle of OOP development
Bad:
class BankAccount
{
public $balance = 1000;
}
$bankAccount = new BankAccount(a);
// Buy shoes...
$bankAccount->balance -= 100;Copy the code
Good:
class BankAccount
{
private $balance;
public function __construct(int $balance = 1000)
{
$this->balance = $balance;
}
public function withdrawBalance(int $amount) :void
{
if ($amount > $this->balance) {
throw new \Exception('Amount greater than available balance.');
}
$this->balance -= $amount;
}
public function depositBalance(int $amount) :void
{
$this->balance += $amount;
}
public function getBalance() :int
{
return $this->balance;
}
}
$bankAccount = new BankAccount(a);
// Buy shoes...
$bankAccount->withdrawBalance($shoesPrice);
// Get balance
$balance = $bankAccount->getBalance();Copy the code
Pass back to top
Object attributes are mostly private/protected qualified
Bad:
class Employee
{
public $name;
public function __construct(string $name)
{
$this->name = $name;
}
}
$employee = new Employee('John Doe');
echo 'Employee name: '.$employee->name; // Employee name: John DoeCopy the code
Good:
class Employee
{
private $name;
public function __construct(string $name)
{
$this->name = $name;
}
public function getName() :string
{
return $this->name;
}
}
$employee = new Employee('John Doe');
echo 'Employee name: '.$employee->getName(); // Employee name: John DoeCopy the code
class
As the Gang of Four’s design patterns stated earlier, we should try to prioritize composition over inheritance. There are many benefits to using inheritance and composition. The main implication of this rule is that when you instinctively use inheritance, try to think about whether combinations can better model your requirements. In some cases, yes.
Next you might be thinking, “So when should I use inheritance?” The answer depends on your question, but here are some examples of when inheritance is better than composition:
- Your inheritance expresses a “is a” rather than “has a” relationship (human – “animal, user -” User Details)
- You can reuse code from base classes (humans can move like animals)
- You want to make global changes to all derived classes by modifying the base class (modifying the energy cost of animals as they move)
The bad:
class Employee
{
private $name;
private $email;
public function __construct(string $name.string $email)
{
$this->name = $name;
$this->email = $email;
}
//.
}
//No, because Employees "has" TaxData
//EmployeeTaxData is not of type Employee
class EmployeeTaxData extends Employee
{
private $ssn;
private $salary;
public function __construct(string $name.string $email.string $ssn.string $salary)
{
parent: :__construct($name.$email);
$this->ssn = $ssn;
$this->salary = $salary;
}
//.
}Copy the code
Good:
class EmployeeTaxData
{
private $ssn;
private $salary;
public function __construct(string $ssn.string $salary)
{
$this->ssn = $ssn;
$this->salary = $salary;
}
//.
}
class Employee
{
private $name;
private $email;
private $taxData;
public function __construct(string $name.string $email)
{
$this->name = $name;
$this->email = $email;
}
public function setTaxData(string $ssn.string $salary)
{
$this->taxData = new EmployeeTaxData($ssn.$salary);
}
//.
}Copy the code
Pass back to top
Avoid coherent interfaces
Fluent Interface is an API design pattern designed to improve code readability in object-oriented programming. It is based on Method chaining
While there can be some contexts, frequently builder objects, where this pattern reduces the verbosity of the code (for example the PHPUnit Mock Builder or the Doctrine Query Builder), more often it comes at some costs:
- Breaks Encapsulation
- Breaks Decorators
- Is harder to mock in a test suite
- Makes diffs of commits harder to read
For more informations you can read the full blog post on this topic written by Marco Pivetta.
The bad:
class Car
{
private $make = 'Honda';
private $model = 'Accord';
private $color = 'white';
public function setMake(string $make) :self
{
$this->make = $make;
// NOTE: Returning this for chaining
return $this;
}
public function setModel(string $model) :self
{
$this->model = $model;
// NOTE: Returning this for chaining
return $this;
}
public function setColor(string $color) :self
{
$this->color = $color;
// NOTE: Returning this for chaining
return $this;
}
public function dump() :void
{
var_dump($this->make.$this->model.$this->color);
}
}
$car = (new Car())
->setColor('pink')
->setMake('Ford')
->setModel('F-150')
->dump();Copy the code
Good:
class Car
{
private $make = 'Honda';
private $model = 'Accord';
private $color = 'white';
public function setMake(string $make) :void
{
$this->make = $make;
}
public function setModel(string $model) :void
{
$this->model = $model;
}
public function setColor(string $color) :void
{
$this->color = $color;
}
public function dump() :void
{
var_dump($this->make.$this->model.$this->color);
}
}
$car = new Car(a);
$car->setColor('pink');
$car->setMake('Ford');
$car->setModel('F-150');
$car->dump();Copy the code
Pass back to top
SOLID
SOLID, an easy-to-remember acronym recommended by Michael Feathers, represents the five most important object-oriented coding principles named by Robert Martin
- S: Single Responsibility Principle (SRP)
- O: Open close Principle (OCP)
- L: Richter’s Substitution Principle (LSP)
- I: Interface Isolation Principle (ISP)
- D: Dependency Inversion Principle (DIP)
Single Responsibility Principle (SRP)
As stated in Clean Code, “A class should be modified for only one reason.” It’s easy to cram a class with a bunch of methods, just like we can only carry one suitcase on an airplane. The problem with this is that such a class is conceptually not highly cohesive, and that leaves plenty of reason to modify it. It’s important to minimize the number of times you need to modify classes. This is because, when there are many methods in a class, modifying one of them makes it difficult to know which dependent modules in the code base will be affected.
The bad:
class UserSettings
{
private $user;
public function __construct(User $user)
{
$this->user = $user;
}
public function changeSettings(array $settings) :void
{
if ($this->verifyCredentials()) {
//.
}
}
private function verifyCredentials() :bool
{
//.
}
}Copy the code
Good:
class UserAuth
{
private $user;
public function __construct(User $user)
{
$this->user = $user;
}
public function verifyCredentials() :bool
{
//.
}
}
class UserSettings
{
private $user;
private $auth;
public function __construct(User $user)
{
$this->user = $user;
$this->auth = new UserAuth($user);
}
public function changeSettings(array $settings) :void
{
if ($this->auth->verifyCredentials()) {
//.
}
}
}Copy the code
Open/Closed Principle (OCP)
As Bertrand Meyer put it, “Software artifacts (classes, modules, functions, etc.) should be open for extension and closed for modification.” But what does that mean? This principle basically says that you should allow new functionality to be added without changing existing code
The bad:
abstract class Adapter
{
protected $name;
public function getName() :string
{
return $this->name;
}
}
class AjaxAdapter extends Adapter
{
public function __construct(a)
{
parent: :__construct();
$this->name = 'ajaxAdapter';
}
}
class NodeAdapter extends Adapter
{
public function __construct(a)
{
parent: :__construct();
$this->name = 'nodeAdapter';
}
}
class HttpRequester
{
private $adapter;
public function __construct(Adapter $adapter)
{
$this->adapter = $adapter;
}
public function fetch(string $url) :Promise
{
$adapterName = $this->adapter->getName();
if ($adapterName = = = 'ajaxAdapter') {
return $this->makeAjaxCall($url);
} elseif ($adapterName = = = 'httpNodeAdapter') {
return $this->makeHttpCall($url);
}
}
private function makeAjaxCall(string $url) :Promise
{
// request and return promise
}
private function makeHttpCall(string $url) :Promise
{
// request and return promise
}
}Copy the code
Good:
interface Adapter
{
public function request(string $url) :Promise;
}
class AjaxAdapter implements Adapter
{
public function request(string $url) :Promise
{
// request and return promise
}
}
class NodeAdapter implements Adapter
{
public function request(string $url) :Promise
{
// request and return promise
}
}
class HttpRequester
{
private $adapter;
public function __construct(Adapter $adapter)
{
$this->adapter = $adapter;
}
public function fetch(string $url) :Promise
{
return $this->adapter->request($url);
}
}Copy the code
Liskov Substitution Principle (LSP)
It’s a simple principle, but in a difficult terminology. Its formal definition is “if S is a subtype of T, then any object of type T can be replaced by an object of type S (for example, an object of type S can replace an object of type T) without changing the existing properties of the program (checking, performing tasks, etc.).” This definition is harder to understand :-).
The best way to explain this concept is that if you have a parent and a subclass, the parent and subclass can be interchanged without changing the validity of the original result. This still sounds confusing, so let’s look at a classic square-rectangle example. Mathematically, a square is a rectangle, but when your model uses the “IS-A” relationship through inheritance, it’s not right.
The bad:
class Rectangle
{
protected $width = 0;
protected $height = 0;
public function render(int $area) :void
{
//.
}
public function setWidth(int $width) :void
{
$this->width = $width;
}
public function setHeight(int $height) :void
{
$this->height = $height;
}
public function getArea() :int
{
return $this->width * $this->height;
}
}
class Square extends Rectangle
{
public function setWidth(int $width) :void
{
$this->width = $this->height = $width;
}
public function setHeight(int $height) :void
{
$this->width = $this->height = $height;
}
}
function renderLargeRectangles(Rectangle $rectangles) :void
{
foreach ($rectangles as $rectangle) {
$rectangle->setWidth(4);
$rectangle->setHeight(5);
$area = $rectangle->getArea(); // BAD: Will return 25 for Square. Should be 20.
$rectangle->render($area);
}
}
$rectangles = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles($rectangles);Copy the code
Good:
abstract class Shape
{
protected $width = 0;
protected $height = 0;
abstract public function getArea() :int;
public function render(int $area) :void
{
//.
}
}
class Rectangle extends Shape
{
public function setWidth(int $width) :void
{
$this->width = $width;
}
public function setHeight(int $height) :void
{
$this->height = $height;
}
public function getArea() :int
{
return $this->width * $this->height;
}
}
class Square extends Shape
{
private $length = 0;
public function setLength(int $length) :void
{
$this->length = $length;
}
public function getArea() :int
{
return pow($this->length.2);
}
}
function renderLargeRectangles(Shape $rectangles) :void
{
foreach ($rectangles as $rectangle) {
if ($rectangle instanceof Square) {
$rectangle->setLength(5);
} elseif ($rectangle instanceof Rectangle) {
$rectangle->setWidth(4);
$rectangle->setHeight(5);
}
$area = $rectangle->getArea();
$rectangle->render($area);
}
}
$shapes = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles($shapes);Copy the code
Interface Segregation Principle (ISP)
The interface isolation principle states: “Callers should not be forced to rely on interfaces they do not need.”
There is a clear example to illustrate this principle. When a class requires a large number of Settings, callers are not required to set a large number of options for convenience, because they usually do not need all of them. Making Settings optional helps us avoid creating “fat interfaces”
The bad:
interface Employee
{
public function work() :void;
public function eat() :void;
}
class Human implements Employee
{
public function work() :void
{
//. working
}
public function eat() :void
{
//. eating in lunch break
}
}
class Robot implements Employee
{
public function work() :void
{
//. working much more
}
public function eat() :void
{
//. robot can't eat, but it must implement this method
}
}Copy the code
Good:
Not every worker is an employee, but every employee is a worker
interface Workable
{
public function work() :void;
}
interface Feedable
{
public function eat() :void;
}
interface Employee extends Feedable.Workable
{
}
class Human implements Employee
{
public function work() :void
{
//. working
}
public function eat() :void
{
//. eating in lunch break
}
}
// robot can only work
class Robot implements Workable
{
public function work() :void
{
//. working
}
}Copy the code
Dependency Inversion Principle (DIP)
This principle makes two basic points:
- Higher-order modules should not depend on lower-order modules, they should all depend on abstractions
- Abstraction should not depend on implementation; implementation should depend on abstraction
This one may seem a bit arcane at first, but if you’ve ever used a PHP framework such as Symfony, you’ve probably seen dependency injection (DI) implemented this concept. Although they are not completely interlinked concepts, the dependency inversion principle separates implementation details and creation of higher-order modules from lower-order modules. This can be done using dependency injection (DI). The further benefit is that it decouples modules from each other. Coupling makes it hard to refactor, and it’s a very bad development pattern
The bad:
class Employee
{
public function work() :void
{
//. working
}
}
class Robot extends Employee
{
public function work() :void
{
//. working much more
}
}
class Manager
{
private $employee;
public function __construct(Employee $employee)
{
$this->employee = $employee;
}
public function manage() :void
{
$this->employee->work();
}
}Copy the code
Good:
interface Employee
{
public function work() :void;
}
class Human implements Employee
{
public function work() :void
{
//. working
}
}
class Robot implements Employee
{
public function work() :void
{
//. working much more
}
}
class Manager
{
private $employee;
public function __construct(Employee $employee)
{
$this->employee = $employee;
}
public function manage() :void
{
$this->employee->work();
}
}Copy the code
Don’t write duplicate code (DRY)
Try to follow the DRY principle.
Do your best to avoid copying code, it is a very bad practice, copying code usually means that when you need to change some logic, you need to change more than one.
Imagine if you run a restaurant and you keep track of the sales and imports of your warehouse: all the potatoes, Onions, garlic, peppers, etc. If you have multiple lists to manage sales records, you will need to update all lists when you use some of these potatoes for cooking. If you only have one list there’s only one place to update it.
Usually you copy code with two or more slightly different logics, most of which are the same, but because of their differences you must have two or more separate but mostly identical methods, Removing duplicate code means using a function/module/class to create an abstraction that handles differences.
Getting abstractions right is critical, which is why you must learn to follow the SOLID principles outlined in the Classes section. Bad abstractions are worse than copying code, so be careful! Having said that, if you can design a reasonable abstraction, implement it! Don’t duplicate it, or you’ll find that anytime you want to change one logic you have to change multiple places.
The bad:
function showDeveloperList(array $developers) :void
{
foreach ($developers as $developer) {
$expectedSalary = $developer->calculateExpectedSalary();
$experience = $developer->getExperience();
$githubLink = $developer->getGithubLink();
$data = [
$expectedSalary.
$experience.
$githubLink
];
render($data);
}
}
function showManagerList(array $managers) :void
{
foreach ($managers as $manager) {
$expectedSalary = $manager->calculateExpectedSalary();
$experience = $manager->getExperience();
$githubLink = $manager->getGithubLink();
$data = [
$expectedSalary.
$experience.
$githubLink
];
render($data);
}
}Copy the code
Good:
function showList(array $employees) :void
{
foreach ($employees as $employee) {
$expectedSalary = $employee->calculateExpectedSalary();
$experience = $employee->getExperience();
$githubLink = $employee->getGithubLink();
$data = [
$expectedSalary.
$experience.
$githubLink
];
render($data);
}
}Copy the code
Very good:
It is better to use a compact version of the code.
function showList(array $employees) :void
{
foreach ($employees as $employee) {
render([
$employee->calculateExpectedSalary(),
$employee->getExperience(),
$employee->getGithubLink()
]);
}
}Copy the code
translation
Translation of other Languages:
- Chinese:
- yangweijie/clean-code-php
- php-cpm/clean-code-php
- gbcr/clean-code-php
- Russian:
- peter-gribanov/clean-code-php
- Portuguese:
- fabioars/clean-code-php
- jeanjar/clean-code-php
- Thai:
- panuwizzle/clean-code-php
Pass back to top