mirror of
https://github.com/jupeter/clean-code-php.git
synced 2025-09-25 21:49:04 +02:00
correct text and code style
This commit is contained in:
151
README.md
151
README.md
@@ -1,6 +1,7 @@
|
||||
# Clean Code PHP
|
||||
|
||||
## Table of Contents
|
||||
|
||||
1. [Introduction](#introduction)
|
||||
2. [Variables](#variables)
|
||||
3. [Functions](#functions)
|
||||
@@ -27,62 +28,74 @@ years of collective experience by the authors of *Clean Code*.
|
||||
|
||||
Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript)
|
||||
|
||||
## **Variables**
|
||||
## Variables
|
||||
|
||||
### Use meaningful and pronounceable variable names
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
$ymdstr = $moment->format('y-m-d');
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
$currentDate = $moment->format('y-m-d');
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Use the same vocabulary for the same type of variable
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
getUserInfo();
|
||||
getClientData();
|
||||
getCustomerRecord();
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
getUser();
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Use searchable names
|
||||
|
||||
We will read more code than we will ever write. It's important that the code we do write is
|
||||
readable and searchable. By *not* naming variables that end up being meaningful for
|
||||
understanding our program, we hurt our readers.
|
||||
Make your names searchable.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
// What the heck is 86400 for?
|
||||
addExpireAt(86400);
|
||||
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
// Declare them as capitalized `const` globals.
|
||||
interface DateGlobal {
|
||||
interface DateGlobal
|
||||
{
|
||||
const SECONDS_IN_A_DAY = 86400;
|
||||
}
|
||||
|
||||
addExpireAt(DateGlobal::SECONDS_IN_A_DAY);
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
|
||||
### Use explanatory variables
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
$address = 'One Infinite Loop, Cupertino 95014';
|
||||
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/';
|
||||
@@ -91,7 +104,7 @@ preg_match($cityZipCodeRegex, $address, $matches);
|
||||
saveCityZipCode($matches[1], $matches[2]);
|
||||
```
|
||||
|
||||
**Not bad**:
|
||||
**Not bad:**
|
||||
|
||||
It's better, but we are still heavily dependent on regex.
|
||||
|
||||
@@ -104,9 +117,10 @@ list(, $city, $zipCode) = $matches;
|
||||
saveCityZipCode($city, $zipCode);
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
Decrease dependence on regex by naming subpatterns.
|
||||
|
||||
```php
|
||||
$address = 'One Infinite Loop, Cupertino 95014';
|
||||
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(?<city>.+?)\s*(?<zipCode>\d{5})?$/';
|
||||
@@ -114,13 +128,16 @@ preg_match($cityZipCodeRegex, $address, $matches);
|
||||
|
||||
saveCityZipCode($matches['city'], $matches['zipCode']);
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Avoid Mental Mapping
|
||||
|
||||
Don’t force the reader of your code to translate what the variable means.
|
||||
Explicit is better than implicit.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
$l = ['Austin', 'New York', 'San Francisco'];
|
||||
|
||||
@@ -136,7 +153,8 @@ for ($i = 0; $i < count($l); $i++) {
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
$locations = ['Austin', 'New York', 'San Francisco'];
|
||||
|
||||
@@ -147,10 +165,10 @@ foreach ($locations as $location) {
|
||||
// ...
|
||||
// ...
|
||||
dispatch($location);
|
||||
});
|
||||
}
|
||||
```
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Don't add unneeded context
|
||||
|
||||
@@ -170,7 +188,7 @@ class Car
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
class Car
|
||||
@@ -210,7 +228,7 @@ function createMicrobrewery($name = null)
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
If you support only PHP 7+, then you can use [type hinting](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration) and be sure that the `$breweryName` will not be `NULL`.
|
||||
|
||||
@@ -222,8 +240,11 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.')
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
## **Functions**
|
||||
|
||||
## Functions
|
||||
|
||||
### Function arguments (2 or fewer ideally)
|
||||
|
||||
Limiting the amount of function parameters is incredibly important because it makes
|
||||
testing your function easier. Having more than three leads to a combinatorial explosion
|
||||
where you have to test tons of different cases with each separate argument.
|
||||
@@ -234,13 +255,16 @@ arguments then your function is trying to do too much. In cases where it's not,
|
||||
of the time a higher-level object will suffice as an argument.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
function createMenu($title, $body, $buttonText, $cancellable) {
|
||||
function createMenu($title, $body, $buttonText, $cancellable)
|
||||
{
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
class MenuConfig
|
||||
{
|
||||
@@ -256,15 +280,16 @@ $config->body = 'Bar';
|
||||
$config->buttonText = 'Baz';
|
||||
$config->cancellable = true;
|
||||
|
||||
function createMenu(MenuConfig $config) {
|
||||
function createMenu(MenuConfig $config)
|
||||
{
|
||||
// ...
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
|
||||
### Functions should do one thing
|
||||
|
||||
This is by far the most important rule in software engineering. When functions do more
|
||||
than one thing, they are harder to compose, test, and reason about. When you can isolate
|
||||
a function to just one action, they can be refactored easily and your code will read much
|
||||
@@ -273,7 +298,8 @@ of many developers.
|
||||
|
||||
**Bad:**
|
||||
```php
|
||||
function emailClients($clients) {
|
||||
function emailClients($clients)
|
||||
{
|
||||
foreach ($clients as $client) {
|
||||
$clientRecord = $db->find($client);
|
||||
if ($clientRecord->isActive()) {
|
||||
@@ -283,22 +309,28 @@ function emailClients($clients) {
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
function emailClients($clients) {
|
||||
function emailClients($clients)
|
||||
{
|
||||
$activeClients = activeClients($clients);
|
||||
array_walk($activeClients, 'email');
|
||||
}
|
||||
|
||||
function activeClients($clients) {
|
||||
function activeClients($clients)
|
||||
{
|
||||
return array_filter($clients, 'isClientActive');
|
||||
}
|
||||
|
||||
function isClientActive($client) {
|
||||
function isClientActive($client)
|
||||
{
|
||||
$clientRecord = $db->find($client);
|
||||
|
||||
return $clientRecord->isActive();
|
||||
}
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Function names should say what they do
|
||||
@@ -441,9 +473,7 @@ class Tokenizer
|
||||
return $tokens;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```php
|
||||
class Lexer
|
||||
{
|
||||
public function lexify($tokens)
|
||||
@@ -456,9 +486,7 @@ class Lexer
|
||||
return $ast;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```php
|
||||
class BetterJSAlternative
|
||||
{
|
||||
private $tokenizer;
|
||||
@@ -502,7 +530,7 @@ function createFile($name, $temp = false)
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
function createFile($name)
|
||||
@@ -571,6 +599,7 @@ var_dump($newName); // ['Ryan', 'McDermott'];
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Don't write to global functions
|
||||
|
||||
Polluting globals is a bad practice in many languages because you could clash with another
|
||||
library and the user of your API would be none-the-wiser until they get an exception in
|
||||
production. Let's think about an example: what if you wanted to have configuration array.
|
||||
@@ -627,6 +656,7 @@ And now you must use instance of `Configuration` in your application.
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Don't use a Singleton pattern
|
||||
|
||||
Singleton is an [anti-pattern](https://en.wikipedia.org/wiki/Singleton_pattern).
|
||||
|
||||
**Bad:**
|
||||
@@ -690,7 +720,7 @@ if ($article->state === 'published') {
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
if ($article->isPublished()) {
|
||||
@@ -824,7 +854,7 @@ function travelToTexas($vehicle)
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
function travelToTexas(Traveler $vehicle)
|
||||
@@ -860,7 +890,7 @@ function combine($val1, $val2)
|
||||
}
|
||||
```
|
||||
|
||||
**Good**:
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
function combine(int $val1, int $val2)
|
||||
@@ -909,7 +939,8 @@ inventoryTracker('apples', $request, 'www.inventory-awesome.io');
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
|
||||
## **Objects and Data Structures**
|
||||
## Objects and Data Structures
|
||||
|
||||
### Use getters and setters
|
||||
|
||||
In PHP you can set `public`, `protected` and `private` keywords for methods.
|
||||
@@ -1015,7 +1046,8 @@ class Employee
|
||||
$this->name = $name;
|
||||
}
|
||||
|
||||
public function getName() {
|
||||
public function getName()
|
||||
{
|
||||
return $this->name;
|
||||
}
|
||||
}
|
||||
@@ -1026,10 +1058,10 @@ echo 'Employee name: '.$employee->getName(); // Employee name: John Doe
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
|
||||
## **Classes**
|
||||
## Classes
|
||||
|
||||
### Single Responsibility Principle (SRP)
|
||||
|
||||
As stated in Clean Code, "There should never be more than one reason for a class
|
||||
to change". It's tempting to jam-pack a class with a lot of functionality, like
|
||||
when you can only take one suitcase on your flight. The issue with this is
|
||||
@@ -1040,6 +1072,7 @@ it can be difficult to understand how that will affect other dependent modules i
|
||||
your codebase.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
class UserSettings
|
||||
{
|
||||
@@ -1065,6 +1098,7 @@ class UserSettings
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
class UserAuth
|
||||
{
|
||||
@@ -1081,7 +1115,6 @@ class UserAuth
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
class UserSettings
|
||||
{
|
||||
private $user;
|
||||
@@ -1101,6 +1134,7 @@ class UserSettings
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Open/Closed Principle (OCP)
|
||||
@@ -1163,12 +1197,12 @@ class HttpRequester
|
||||
}
|
||||
}
|
||||
|
||||
protected function makeAjaxCall($url)
|
||||
private function makeAjaxCall($url)
|
||||
{
|
||||
// request and return promise
|
||||
}
|
||||
|
||||
protected function makeHttpCall($url)
|
||||
private function makeHttpCall($url)
|
||||
{
|
||||
// request and return promise
|
||||
}
|
||||
@@ -1324,7 +1358,7 @@ class Rectangle extends Shape
|
||||
|
||||
class Square extends Shape
|
||||
{
|
||||
protected $length = 0;
|
||||
private $length = 0;
|
||||
|
||||
public function setLength($length)
|
||||
{
|
||||
@@ -1542,6 +1576,7 @@ class Manager
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Use method chaining
|
||||
|
||||
This pattern is very useful and commonly used in many libraries such
|
||||
as PHPUnit and Doctrine. It allows your code to be expressive, and less verbose.
|
||||
For that reason, use method chaining and take a look at how clean your code
|
||||
@@ -1549,6 +1584,7 @@ will be. In your class functions, simply use `return $this` at the end of every
|
||||
and you can chain further class methods onto it.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
class Car
|
||||
{
|
||||
@@ -1585,6 +1621,7 @@ $car->dump();
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
class Car
|
||||
{
|
||||
@@ -1628,9 +1665,11 @@ $car = (new Car())
|
||||
->setModel('F-150')
|
||||
->dump();
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Prefer composition over inheritance
|
||||
|
||||
As stated famously in [*Design Patterns*](https://en.wikipedia.org/wiki/Design_Patterns) by the Gang of Four,
|
||||
you should prefer composition over inheritance where you can. There are lots of
|
||||
good reasons to use inheritance and lots of good reasons to use composition.
|
||||
@@ -1649,6 +1688,7 @@ relationship (Human->Animal vs. User->UserDetails).
|
||||
(Change the caloric expenditure of all animals when they move).
|
||||
|
||||
**Bad:**
|
||||
|
||||
```php
|
||||
class Employee
|
||||
{
|
||||
@@ -1685,6 +1725,7 @@ class EmployeeTaxData extends Employee
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```php
|
||||
class EmployeeTaxData
|
||||
{
|
||||
@@ -1712,12 +1753,15 @@ class Employee
|
||||
$this->email = $email;
|
||||
}
|
||||
|
||||
public function setTaxData($ssn, $salary) {
|
||||
public function setTaxData($ssn, $salary)
|
||||
{
|
||||
$this->taxData = new EmployeeTaxData($ssn, $salary);
|
||||
}
|
||||
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
## Don’t repeat yourself (DRY)
|
||||
@@ -1750,7 +1794,7 @@ updating multiple places anytime you want to change one thing.
|
||||
```php
|
||||
function showDeveloperList($developers)
|
||||
{
|
||||
foreach ($developers as $developer) {
|
||||
foreach ($developers as $developer) {
|
||||
$expectedSalary = $developer->calculateExpectedSalary();
|
||||
$experience = $developer->getExperience();
|
||||
$githubLink = $developer->getGithubLink();
|
||||
@@ -1766,7 +1810,7 @@ function showDeveloperList($developers)
|
||||
|
||||
function showManagerList($managers)
|
||||
{
|
||||
foreach ($managers as $manager) {
|
||||
foreach ($managers as $manager) {
|
||||
$expectedSalary = $manager->calculateExpectedSalary();
|
||||
$experience = $manager->getExperience();
|
||||
$githubLink = $manager->getGithubLink();
|
||||
@@ -1786,10 +1830,10 @@ function showManagerList($managers)
|
||||
```php
|
||||
function showList($employees)
|
||||
{
|
||||
foreach ($employees as $employee) {
|
||||
$expectedSalary = $employee->calculateExpectedSalary();
|
||||
$experience = $employee->getExperience();
|
||||
$githubLink = $employee->getGithubLink();
|
||||
foreach ($employees as $employee) {
|
||||
$expectedSalary = $employee->calculateExpectedSalary();
|
||||
$experience = $employee->getExperience();
|
||||
$githubLink = $employee->getGithubLink();
|
||||
$data = [
|
||||
$expectedSalary,
|
||||
$experience,
|
||||
@@ -1808,7 +1852,7 @@ It is better to use a compact version of the code.
|
||||
```php
|
||||
function showList($employees)
|
||||
{
|
||||
foreach ($employees as $employee) {
|
||||
foreach ($employees as $employee) {
|
||||
render([
|
||||
$employee->calculateExpectedSalary(),
|
||||
$employee->getExperience(),
|
||||
@@ -1820,13 +1864,12 @@ function showList($employees)
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
|
||||
|
||||
## Translations
|
||||
|
||||
This is also available in other languages:
|
||||
-  **Chinese**:
|
||||
- [yangweijie/clean-code-php](https://github.com/yangweijie/clean-code-php)
|
||||
- [php-cpm/clean-code-php](https://github.com/php-cpm/clean-code-php)
|
||||
|
||||
*  **Chinese:**
|
||||
* [yangweijie/clean-code-php](https://github.com/yangweijie/clean-code-php)
|
||||
* [php-cpm/clean-code-php](https://github.com/php-cpm/clean-code-php)
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
Reference in New Issue
Block a user