diff --git a/README.md b/README.md index 1162a20..6aecefb 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # Clean Code PHP ## Table of Contents + 1. [Introduction](#introduction) 2. [Variables](#variables) 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) -## **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(); getUserData(); @@ -51,10 +57,12 @@ getUserRecord(); getUserProfile(); ``` -**Good**: +**Good:** + ```php getUser(); ``` + **[⬆ back to top](#table-of-contents)** ### Use searchable names (part 1) @@ -106,9 +114,10 @@ if ($user->access & User::ACCESS_UPDATE) { **[⬆ back to top](#table-of-contents)** - ### Use explanatory variables + **Bad:** + ```php $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/'; @@ -117,7 +126,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. @@ -130,9 +139,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]+(?.+?)\s*(?\d{5})?$/'; @@ -140,13 +150,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']; @@ -162,7 +175,8 @@ for ($i = 0; $i < count($l); $i++) { } ``` -**Good**: +**Good:** + ```php $locations = ['Austin', 'New York', 'San Francisco']; @@ -173,10 +187,10 @@ foreach ($locations as $location) { // ... // ... dispatch($location); -}); +} ``` -**[⬆ back to top](#table-of-contents)** +**[⬆ back to top](#table-of-contents)** ### Don't add unneeded context @@ -196,7 +210,7 @@ class Car } ``` -**Good**: +**Good:** ```php 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`. @@ -248,8 +262,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. @@ -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. **Bad:** + ```php -function createMenu($title, $body, $buttonText, $cancellable) { +function createMenu($title, $body, $buttonText, $cancellable) +{ // ... } ``` -**Good**: +**Good:** + ```php class MenuConfig { @@ -282,15 +302,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 @@ -299,7 +320,8 @@ of many developers. **Bad:** ```php -function emailClients($clients) { +function emailClients($clients) +{ foreach ($clients as $client) { $clientRecord = $db->find($client); if ($clientRecord->isActive()) { @@ -309,22 +331,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 @@ -381,7 +409,7 @@ function parseBetterJSAlternative($code) $regexes = [ // ... ]; - + $statements = explode(' ', $code); $tokens = []; foreach ($regexes as $regex) { @@ -389,12 +417,12 @@ function parseBetterJSAlternative($code) // ... } } - + $ast = []; foreach ($tokens as $token) { // lex... } - + foreach ($ast as $node) { // parse... } @@ -411,7 +439,7 @@ function tokenize($code) $regexes = [ // ... ]; - + $statements = explode(' ', $code); $tokens = []; foreach ($regexes as $regex) { @@ -419,7 +447,7 @@ function tokenize($code) $tokens[] = /* ... */; } } - + return $tokens; } @@ -429,7 +457,7 @@ function lexer($tokens) foreach ($tokens as $token) { $ast[] = /* ... */; } - + return $ast; } @@ -467,9 +495,7 @@ class Tokenizer return $tokens; } } -``` -```php class Lexer { public function lexify($tokens) @@ -482,9 +508,7 @@ class Lexer return $ast; } } -``` -```php class BetterJSAlternative { private $tokenizer; @@ -528,7 +552,7 @@ function createFile($name, $temp = false) } ``` -**Good**: +**Good:** ```php function createFile($name) @@ -597,6 +621,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. @@ -646,6 +671,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:** @@ -709,7 +735,7 @@ if ($article->state === 'published') { } ``` -**Good**: +**Good:** ```php if ($article->isPublished()) { @@ -845,7 +871,7 @@ function travelToTexas($vehicle) } ``` -**Good**: +**Good:** ```php function travelToTexas(Traveler $vehicle) @@ -881,7 +907,7 @@ function combine($val1, $val2) } ``` -**Good**: +**Good:** ```php function combine(int $val1, int $val2) @@ -930,7 +956,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. @@ -1030,13 +1057,14 @@ echo 'Employee name: '.$employee->name; // Employee name: John Doe class Employee { private $name; - + public function __construct($name) { $this->name = $name; } - - public function getName() { + + public function getName() + { return $this->name; } } @@ -1047,10 +1075,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 @@ -1061,6 +1089,7 @@ it can be difficult to understand how that will affect other dependent modules i your codebase. **Bad:** + ```php class UserSettings { @@ -1070,14 +1099,14 @@ class UserSettings { $this->user = $user; } - + public function changeSettings($settings) { if ($this->verifyCredentials()) { // ... } } - + private function verifyCredentials() { // ... @@ -1086,6 +1115,7 @@ class UserSettings ``` **Good:** + ```php class UserAuth { @@ -1102,7 +1132,6 @@ class UserAuth } } - class UserSettings { private $user; @@ -1122,6 +1151,7 @@ class UserSettings } } ``` + **[⬆ back to top](#table-of-contents)** ### Open/Closed Principle (OCP) @@ -1184,12 +1214,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 } @@ -1228,7 +1258,7 @@ class HttpRequester { $this->adapter = $adapter; } - + public function fetch($url) { return $this->adapter->request($url); @@ -1345,7 +1375,7 @@ class Rectangle extends Shape class Square extends Shape { - protected $length = 0; + private $length = 0; public function setLength($length) { @@ -1367,7 +1397,7 @@ function renderLargeRectangles($rectangles) $rectangle->setWidth(4); $rectangle->setHeight(5); } - + $area = $rectangle->getArea(); $rectangle->render($area); } @@ -1490,10 +1520,10 @@ it makes your code hard to refactor. ```php class Employee { - public function work() - { - // ....working - } + public function work() + { + // ....working + } } class Robot extends Employee @@ -1563,6 +1593,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 @@ -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. **Bad:** + ```php class Car { @@ -1606,6 +1638,7 @@ $car->dump(); ``` **Good:** + ```php class Car { @@ -1649,9 +1682,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. @@ -1670,12 +1705,13 @@ relationship (Human->Animal vs. User->UserDetails). (Change the caloric expenditure of all animals when they move). **Bad:** + ```php class Employee { private $name; private $email; - + public function __construct($name, $email) { $this->name = $name; @@ -1706,12 +1742,13 @@ class EmployeeTaxData extends Employee ``` **Good:** + ```php class EmployeeTaxData { private $ssn; private $salary; - + public function __construct($ssn, $salary) { $this->ssn = $ssn; @@ -1733,12 +1770,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) @@ -1771,7 +1811,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(); @@ -1780,14 +1820,14 @@ function showDeveloperList($developers) $experience, $githubLink ]; - + render($data); } } function showManagerList($managers) { -    foreach ($managers as $manager) { + foreach ($managers as $manager) { $expectedSalary = $manager->calculateExpectedSalary(); $experience = $manager->getExperience(); $githubLink = $manager->getGithubLink(); @@ -1796,7 +1836,7 @@ function showManagerList($managers) $experience, $githubLink ]; - + render($data); } } @@ -1807,16 +1847,16 @@ 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, $githubLink ]; - + render($data); } } @@ -1829,7 +1869,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(), @@ -1841,13 +1881,12 @@ function showList($employees) **[⬆ back to top](#table-of-contents)** - - ## Translations 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) - - [php-cpm/clean-code-php](https://github.com/php-cpm/clean-code-php) + + * ![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) + * [php-cpm/clean-code-php](https://github.com/php-cpm/clean-code-php) **[⬆ back to top](#table-of-contents)**