info/clean-code-php
1
0
Fork 0
mirror of https://github.com/jupeter/clean-code-php.git synced 2025-09-25 21:49:04 +02:00
Code Issues Releases Wiki Activity

Merge pull request #90 from peter-gribanov/cs

Browse Source 
Correct text and code style
This commit is contained in:
Tomáš Votruba
2017-09-08 21:56:43 +02:00
committed by GitHub
parent 53a944deb3 cd7733d091
commit 8979cdcd8d
1 changed files with 111 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

183
README.md
View File

@@ -1,6 +1,7 @@
# Clean Code PHP # Clean Code PHP
## Table of Contents ## Table of Contents
1. [Introduction](#introduction) 1. [Introduction](#introduction)
2. [Variables](#variables) 2. [Variables](#variables)
3. [Functions](#functions) 3. [Functions](#functions)
@@ -27,23 +28,28 @@ years of collective experience by the authors of *Clean Code*.
Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript) Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript)
## **Variables** ## Variables
### Use meaningful and pronounceable variable names ### Use meaningful and pronounceable variable names
**Bad:** **Bad:**
```php ```php
$ymdstr = $moment->format('y-m-d'); $ymdstr = $moment->format('y-m-d');
``` ```
**Good**: **Good:**
```php ```php
$currentDate = $moment->format('y-m-d'); $currentDate = $moment->format('y-m-d');
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Use the same vocabulary for the same type of variable ### Use the same vocabulary for the same type of variable
**Bad:** **Bad:**
```php ```php
getUserInfo(); getUserInfo();
getUserData(); getUserData();
@@ -51,10 +57,12 @@ getUserRecord();
getUserProfile(); getUserProfile();
``` ```
**Good**: **Good:**
```php ```php
getUser(); getUser();
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Use searchable names (part 1) ### Use searchable names (part 1)
@@ -106,9 +114,10 @@ if ($user->access & User::ACCESS_UPDATE) {
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Use explanatory variables ### Use explanatory variables
**Bad:** **Bad:**
```php ```php
$address = 'One Infinite Loop, Cupertino 95014'; $address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/'; $cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/';
@@ -117,7 +126,7 @@ preg_match($cityZipCodeRegex, $address, $matches);
saveCityZipCode($matches[1], $matches[2]); saveCityZipCode($matches[1], $matches[2]);
``` ```
**Not bad**: **Not bad:**
It's better, but we are still heavily dependent on regex. It's better, but we are still heavily dependent on regex.
@@ -130,9 +139,10 @@ list(, $city, $zipCode) = $matches;
saveCityZipCode($city, $zipCode); saveCityZipCode($city, $zipCode);
``` ```
**Good**: **Good:**
Decrease dependence on regex by naming subpatterns. Decrease dependence on regex by naming subpatterns.
```php ```php
$address = 'One Infinite Loop, Cupertino 95014'; $address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(?<city>.+?)\s*(?<zipCode>\d{5})?$/'; $cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(?<city>.+?)\s*(?<zipCode>\d{5})?$/';
@@ -140,13 +150,16 @@ preg_match($cityZipCodeRegex, $address, $matches);
saveCityZipCode($matches['city'], $matches['zipCode']); saveCityZipCode($matches['city'], $matches['zipCode']);
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Avoid Mental Mapping ### Avoid Mental Mapping
Don’t force the reader of your code to translate what the variable means. Don’t force the reader of your code to translate what the variable means.
Explicit is better than implicit. Explicit is better than implicit.
**Bad:** **Bad:**
```php ```php
$l = ['Austin', 'New York', 'San Francisco']; $l = ['Austin', 'New York', 'San Francisco'];
@@ -162,7 +175,8 @@ for ($i = 0; $i < count($l); $i++) {
} }
``` ```
**Good**: **Good:**
```php ```php
$locations = ['Austin', 'New York', 'San Francisco']; $locations = ['Austin', 'New York', 'San Francisco'];
@@ -173,10 +187,10 @@ foreach ($locations as $location) {
// ... // ...
// ... // ...
dispatch($location); dispatch($location);
}); }
``` ```
**[⬆ back to top](#table-of-contents)**
**[⬆ back to top](#table-of-contents)**
### Don't add unneeded context ### Don't add unneeded context
@@ -196,7 +210,7 @@ class Car
} }
``` ```
**Good**: **Good:**
```php ```php
class Car class Car
@@ -236,7 +250,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`. 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`.
@@ -248,8 +262,11 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.')
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
## **Functions**
## Functions
### Function arguments (2 or fewer ideally) ### Function arguments (2 or fewer ideally)
Limiting the amount of function parameters is incredibly important because it makes 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 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. where you have to test tons of different cases with each separate argument.
@@ -260,13 +277,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. of the time a higher-level object will suffice as an argument.
**Bad:** **Bad:**
```php ```php
function createMenu($title, $body, $buttonText, $cancellable) { function createMenu($title, $body, $buttonText, $cancellable)
{
// ... // ...
} }
``` ```
**Good**: **Good:**
```php ```php
class MenuConfig class MenuConfig
{ {
@@ -282,15 +302,16 @@ $config->body = 'Bar';
$config->buttonText = 'Baz'; $config->buttonText = 'Baz';
$config->cancellable = true; $config->cancellable = true;
function createMenu(MenuConfig $config) { function createMenu(MenuConfig $config)
{
// ... // ...
} }
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Functions should do one thing ### Functions should do one thing
This is by far the most important rule in software engineering. When functions do more 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 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 a function to just one action, they can be refactored easily and your code will read much
@@ -299,7 +320,8 @@ of many developers.
**Bad:** **Bad:**
```php ```php
function emailClients($clients) { function emailClients($clients)
{
foreach ($clients as $client) { foreach ($clients as $client) {
$clientRecord = $db->find($client); $clientRecord = $db->find($client);
if ($clientRecord->isActive()) { if ($clientRecord->isActive()) {
@@ -309,22 +331,28 @@ function emailClients($clients) {
} }
``` ```
**Good**: **Good:**
```php ```php
function emailClients($clients) { function emailClients($clients)
{
$activeClients = activeClients($clients); $activeClients = activeClients($clients);
array_walk($activeClients, 'email'); array_walk($activeClients, 'email');
} }
function activeClients($clients) { function activeClients($clients)
{
return array_filter($clients, 'isClientActive'); return array_filter($clients, 'isClientActive');
} }
function isClientActive($client) { function isClientActive($client)
{
$clientRecord = $db->find($client); $clientRecord = $db->find($client);
return $clientRecord->isActive(); return $clientRecord->isActive();
} }
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Function names should say what they do ### Function names should say what they do
@@ -381,7 +409,7 @@ function parseBetterJSAlternative($code)
$regexes = [ $regexes = [
// ... // ...
]; ];
$statements = explode(' ', $code); $statements = explode(' ', $code);
$tokens = []; $tokens = [];
foreach ($regexes as $regex) { foreach ($regexes as $regex) {
@@ -389,12 +417,12 @@ function parseBetterJSAlternative($code)
// ... // ...
} }
} }
$ast = []; $ast = [];
foreach ($tokens as $token) { foreach ($tokens as $token) {
// lex... // lex...
} }
foreach ($ast as $node) { foreach ($ast as $node) {
// parse... // parse...
} }
@@ -411,7 +439,7 @@ function tokenize($code)
$regexes = [ $regexes = [
// ... // ...
]; ];
$statements = explode(' ', $code); $statements = explode(' ', $code);
$tokens = []; $tokens = [];
foreach ($regexes as $regex) { foreach ($regexes as $regex) {
@@ -419,7 +447,7 @@ function tokenize($code)
$tokens[] = /* ... */; $tokens[] = /* ... */;
} }
} }
return $tokens; return $tokens;
} }
@@ -429,7 +457,7 @@ function lexer($tokens)
foreach ($tokens as $token) { foreach ($tokens as $token) {
$ast[] = /* ... */; $ast[] = /* ... */;
} }
return $ast; return $ast;
} }
@@ -467,9 +495,7 @@ class Tokenizer
return $tokens; return $tokens;
} }
} }
```
```php
class Lexer class Lexer
{ {
public function lexify($tokens) public function lexify($tokens)
@@ -482,9 +508,7 @@ class Lexer
return $ast; return $ast;
} }
} }
```
```php
class BetterJSAlternative class BetterJSAlternative
{ {
private $tokenizer; private $tokenizer;
@@ -528,7 +552,7 @@ function createFile($name, $temp = false)
} }
``` ```
**Good**: **Good:**
```php ```php
function createFile($name) function createFile($name)
@@ -597,6 +621,7 @@ var_dump($newName); // ['Ryan', 'McDermott'];
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Don't write to global functions ### Don't write to global functions
Polluting globals is a bad practice in many languages because you could clash with another 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 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. production. Let's think about an example: what if you wanted to have configuration array.
@@ -646,6 +671,7 @@ And now you must use instance of `Configuration` in your application.
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Don't use a Singleton pattern ### Don't use a Singleton pattern
Singleton is an [anti-pattern](https://en.wikipedia.org/wiki/Singleton_pattern). Singleton is an [anti-pattern](https://en.wikipedia.org/wiki/Singleton_pattern).
**Bad:** **Bad:**
@@ -709,7 +735,7 @@ if ($article->state === 'published') {
} }
``` ```
**Good**: **Good:**
```php ```php
if ($article->isPublished()) { if ($article->isPublished()) {
@@ -845,7 +871,7 @@ function travelToTexas($vehicle)
} }
``` ```
**Good**: **Good:**
```php ```php
function travelToTexas(Traveler $vehicle) function travelToTexas(Traveler $vehicle)
@@ -881,7 +907,7 @@ function combine($val1, $val2)
} }
``` ```
**Good**: **Good:**
```php ```php
function combine(int $val1, int $val2) function combine(int $val1, int $val2)
@@ -930,7 +956,8 @@ inventoryTracker('apples', $request, 'www.inventory-awesome.io');
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
## **Objects and Data Structures** ## Objects and Data Structures
### Use getters and setters ### Use getters and setters
In PHP you can set `public`, `protected` and `private` keywords for methods. In PHP you can set `public`, `protected` and `private` keywords for methods.
@@ -1030,13 +1057,14 @@ echo 'Employee name: '.$employee->name; // Employee name: John Doe
class Employee class Employee
{ {
private $name; private $name;
public function __construct($name) public function __construct($name)
{ {
$this->name = $name; $this->name = $name;
} }
public function getName() { public function getName()
{
return $this->name; return $this->name;
} }
} }
@@ -1047,10 +1075,10 @@ echo 'Employee name: '.$employee->getName(); // Employee name: John Doe
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
## Classes
## **Classes**
### Single Responsibility Principle (SRP) ### Single Responsibility Principle (SRP)
As stated in Clean Code, "There should never be more than one reason for a class 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 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 when you can only take one suitcase on your flight. The issue with this is
@@ -1061,6 +1089,7 @@ it can be difficult to understand how that will affect other dependent modules i
your codebase. your codebase.
**Bad:** **Bad:**
```php ```php
class UserSettings class UserSettings
{ {
@@ -1070,14 +1099,14 @@ class UserSettings
{ {
$this->user = $user; $this->user = $user;
} }
public function changeSettings($settings) public function changeSettings($settings)
{ {
if ($this->verifyCredentials()) { if ($this->verifyCredentials()) {
// ... // ...
} }
} }
private function verifyCredentials() private function verifyCredentials()
{ {
// ... // ...
@@ -1086,6 +1115,7 @@ class UserSettings
``` ```
**Good:** **Good:**
```php ```php
class UserAuth class UserAuth
{ {
@@ -1102,7 +1132,6 @@ class UserAuth
} }
} }
class UserSettings class UserSettings
{ {
private $user; private $user;
@@ -1122,6 +1151,7 @@ class UserSettings
} }
} }
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Open/Closed Principle (OCP) ### Open/Closed Principle (OCP)
@@ -1184,12 +1214,12 @@ class HttpRequester
} }
} }
protected function makeAjaxCall($url) private function makeAjaxCall($url)
{ {
// request and return promise // request and return promise
} }
protected function makeHttpCall($url) private function makeHttpCall($url)
{ {
// request and return promise // request and return promise
} }
@@ -1228,7 +1258,7 @@ class HttpRequester
{ {
$this->adapter = $adapter; $this->adapter = $adapter;
} }
public function fetch($url) public function fetch($url)
{ {
return $this->adapter->request($url); return $this->adapter->request($url);
@@ -1345,7 +1375,7 @@ class Rectangle extends Shape
class Square extends Shape class Square extends Shape
{ {
protected $length = 0; private $length = 0;
public function setLength($length) public function setLength($length)
{ {
@@ -1367,7 +1397,7 @@ function renderLargeRectangles($rectangles)
$rectangle->setWidth(4); $rectangle->setWidth(4);
$rectangle->setHeight(5); $rectangle->setHeight(5);
} }
$area = $rectangle->getArea(); $area = $rectangle->getArea();
$rectangle->render($area); $rectangle->render($area);
} }
@@ -1490,10 +1520,10 @@ it makes your code hard to refactor.
```php ```php
class Employee class Employee
{ {
public function work() public function work()
{ {
// ....working // ....working
} }
} }
class Robot extends Employee class Robot extends Employee
@@ -1563,6 +1593,7 @@ class Manager
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Use method chaining ### Use method chaining
This pattern is very useful and commonly used in many libraries such 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. 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 For that reason, use method chaining and take a look at how clean your code
@@ -1570,6 +1601,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. and you can chain further class methods onto it.
**Bad:** **Bad:**
```php ```php
class Car class Car
{ {
@@ -1606,6 +1638,7 @@ $car->dump();
``` ```
**Good:** **Good:**
```php ```php
class Car class Car
{ {
@@ -1649,9 +1682,11 @@ $car = (new Car())
->setModel('F-150') ->setModel('F-150')
->dump(); ->dump();
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
### Prefer composition over inheritance ### Prefer composition over inheritance
As stated famously in [*Design Patterns*](https://en.wikipedia.org/wiki/Design_Patterns) by the Gang of Four, 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 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. good reasons to use inheritance and lots of good reasons to use composition.
@@ -1670,12 +1705,13 @@ relationship (Human->Animal vs. User->UserDetails).
(Change the caloric expenditure of all animals when they move). (Change the caloric expenditure of all animals when they move).
**Bad:** **Bad:**
```php ```php
class Employee class Employee
{ {
private $name; private $name;
private $email; private $email;
public function __construct($name, $email) public function __construct($name, $email)
{ {
$this->name = $name; $this->name = $name;
@@ -1706,12 +1742,13 @@ class EmployeeTaxData extends Employee
``` ```
**Good:** **Good:**
```php ```php
class EmployeeTaxData class EmployeeTaxData
{ {
private $ssn; private $ssn;
private $salary; private $salary;
public function __construct($ssn, $salary) public function __construct($ssn, $salary)
{ {
$this->ssn = $ssn; $this->ssn = $ssn;
@@ -1733,12 +1770,15 @@ class Employee
$this->email = $email; $this->email = $email;
} }
public function setTaxData($ssn, $salary) { public function setTaxData($ssn, $salary)
{
$this->taxData = new EmployeeTaxData($ssn, $salary); $this->taxData = new EmployeeTaxData($ssn, $salary);
} }
// ... // ...
} }
``` ```
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
## Don’t repeat yourself (DRY) ## Don’t repeat yourself (DRY)
@@ -1771,7 +1811,7 @@ updating multiple places anytime you want to change one thing.
```php ```php
function showDeveloperList($developers) function showDeveloperList($developers)
{ {
   foreach ($developers as $developer) { foreach ($developers as $developer) {
$expectedSalary = $developer->calculateExpectedSalary(); $expectedSalary = $developer->calculateExpectedSalary();
$experience = $developer->getExperience(); $experience = $developer->getExperience();
$githubLink = $developer->getGithubLink(); $githubLink = $developer->getGithubLink();
@@ -1780,14 +1820,14 @@ function showDeveloperList($developers)
$experience, $experience,
$githubLink $githubLink
]; ];
render($data); render($data);
} }
} }
function showManagerList($managers) function showManagerList($managers)
{ {
   foreach ($managers as $manager) { foreach ($managers as $manager) {
$expectedSalary = $manager->calculateExpectedSalary(); $expectedSalary = $manager->calculateExpectedSalary();
$experience = $manager->getExperience(); $experience = $manager->getExperience();
$githubLink = $manager->getGithubLink(); $githubLink = $manager->getGithubLink();
@@ -1796,7 +1836,7 @@ function showManagerList($managers)
$experience, $experience,
$githubLink $githubLink
]; ];
render($data); render($data);
} }
} }
@@ -1807,16 +1847,16 @@ function showManagerList($managers)
```php ```php
function showList($employees) function showList($employees)
{ {
   foreach ($employees as $employee) { foreach ($employees as $employee) {
       $expectedSalary = $employee->calculateExpectedSalary(); $expectedSalary = $employee->calculateExpectedSalary();
       $experience = $employee->getExperience(); $experience = $employee->getExperience();
       $githubLink = $employee->getGithubLink(); $githubLink = $employee->getGithubLink();
$data = [ $data = [
$expectedSalary, $expectedSalary,
$experience, $experience,
$githubLink $githubLink
]; ];
render($data); render($data);
} }
} }
@@ -1829,7 +1869,7 @@ It is better to use a compact version of the code.
```php ```php
function showList($employees) function showList($employees)
{ {
   foreach ($employees as $employee) { foreach ($employees as $employee) {
render([ render([
$employee->calculateExpectedSalary(), $employee->calculateExpectedSalary(),
$employee->getExperience(), $employee->getExperience(),
@@ -1841,13 +1881,12 @@ function showList($employees)
**[⬆ back to top](#table-of-contents)** **[⬆ back to top](#table-of-contents)**
## Translations ## Translations
This is also available in other languages: This is also available in other languages:
- ![cn](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/China.png) **Chinese**:
- [yangweijie/clean-code-php](https://github.com/yangweijie/clean-code-php) * ![cn](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/China.png) **Chinese:**
- [php-cpm/clean-code-php](https://github.com/php-cpm/clean-code-php) * [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)** **[⬆ back to top](#table-of-contents)**