From 2c94c03dd2d6215b252acffd005e0b53c93b57d4 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 28 Oct 2020 23:22:48 +0100 Subject: [PATCH 01/21] [ci] switch travis dummy cs check, to Github Actions and ECS with markdown check --- .github/workflows/coding_standard.yaml | 20 +++++++ .gitignore | 2 + .travis-build.php | 77 -------------------------- .travis.yml | 11 ---- README.md | 2 +- composer.json | 10 ++++ ecs.php | 23 ++++++++ 7 files changed, 56 insertions(+), 89 deletions(-) create mode 100644 .github/workflows/coding_standard.yaml create mode 100644 .gitignore delete mode 100644 .travis-build.php delete mode 100644 .travis.yml create mode 100644 composer.json create mode 100644 ecs.php diff --git a/.github/workflows/coding_standard.yaml b/.github/workflows/coding_standard.yaml new file mode 100644 index 00000000..79d93b3a --- /dev/null +++ b/.github/workflows/coding_standard.yaml @@ -0,0 +1,20 @@ +name: Coding Standard + +on: + pull_request: null + +jobs: + coding_standard: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + # see https://github.com/shivammathur/setup-php + - uses: shivammathur/setup-php@v1 + with: + php-version: 7.4 + coverage: none + + - run: composer install --no-progress + + - run: vendor/bin/ecs check README.md --ansi \ No newline at end of file diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..49c63d28 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +composer.lock +/vendor \ No newline at end of file diff --git a/.travis-build.php b/.travis-build.php deleted file mode 100644 index 17a63fee..00000000 --- a/.travis-build.php +++ /dev/null @@ -1,77 +0,0 @@ -setFlags(SplFileObject::DROP_NEW_LINE); - -$cliRedBackground = "\033[37;41m"; -$cliReset = "\033[0m"; -$exitStatus = 0; - -$indentationSteps = 3; -$manIndex = 0; -$linesWithSpaces = []; -$tableOfContentsStarted = null; -$currentTableOfContentsChapters = []; -$chaptersFound = []; -foreach ($readMeFile as $lineNumber => $line) { - if (preg_match('/\s$/', $line)) { - $linesWithSpaces[] = sprintf('%5s: %s', 1 + $lineNumber, $line); - } - if (preg_match('/^(?##+)\s(?.+)/', $line, $matches)) { - if (null === $tableOfContentsStarted) { - $tableOfContentsStarted = true; - continue; - } - $tableOfContentsStarted = false; - - if (strlen($matches['depth']) === 2) { - $depth = sprintf(' %s.', ++$manIndex); - } else { - $depth = sprintf(' %s*', str_repeat(' ', strlen($matches['depth']) - 1)); - } - - // ignore links in title - $matches['title'] = preg_replace('/\[([^\]]+)\]\((?:[^\)]+)\)/u', '$1', $matches['title']); - - $link = $matches['title']; - $link = strtolower($link); - $link = str_replace(' ', '-', $link); - $link = preg_replace('/[^-\w]+/u', '', $link); - - $chaptersFound[] = sprintf('%s [%s](#%s)', $depth, $matches['title'], $link); - } - if ($tableOfContentsStarted === true && isset($line[0])) { - $currentTableOfContentsChapters[] = $line; - } -} - -if (count($linesWithSpaces)) { - fwrite(STDERR, sprintf("${cliRedBackground}The following lines end with a space character:${cliReset}\n%s\n\n", - implode(PHP_EOL, $linesWithSpaces) - )); - $exitStatus = 1; -} - -$currentTableOfContentsChaptersFilename = __DIR__ . '/current-chapters'; -$chaptersFoundFilename = __DIR__ . '/chapters-found'; - -file_put_contents($currentTableOfContentsChaptersFilename, implode(PHP_EOL, $currentTableOfContentsChapters)); -file_put_contents($chaptersFoundFilename, implode(PHP_EOL, $chaptersFound)); - -$tableOfContentsDiff = shell_exec(sprintf('diff --unified %s %s', - escapeshellarg($currentTableOfContentsChaptersFilename), - escapeshellarg($chaptersFoundFilename) -)); - -@ unlink($currentTableOfContentsChaptersFilename); -@ unlink($chaptersFoundFilename); - -if (!empty($tableOfContentsDiff)) { - fwrite(STDERR, sprintf("${cliRedBackground}The table of contents is not aligned:${cliReset}\n%s\n\n", - $tableOfContentsDiff - )); - $exitStatus = 1; -} - -exit($exitStatus); diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index e4ea3575..00000000 --- a/.travis.yml +++ /dev/null @@ -1,11 +0,0 @@ -language: php - -sudo: false - -php: - - nightly - -script: php .travis-build.php - -notifications: - email: false diff --git a/README.md b/README.md index 0e7551ad..a3318888 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Clean Code PHP +# Clean Code PHP ## Table of Contents diff --git a/composer.json b/composer.json new file mode 100644 index 00000000..733367b9 --- /dev/null +++ b/composer.json @@ -0,0 +1,10 @@ +{ + "require": { + "php": "^7.2", + "symplify/easy-coding-standard": "^8.3" + }, + "scripts": { + "check-cs": "vendor/bin/ecs check README.md", + "fix-cs": "vendor/bin/ecs check README.md --fix" + } +} diff --git a/ecs.php b/ecs.php new file mode 100644 index 00000000..5d564993 --- /dev/null +++ b/ecs.php @@ -0,0 +1,23 @@ +<?php + +declare(strict_types=1); + +use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; +use Symplify\EasyCodingStandard\Configuration\Option; +use Symplify\EasyCodingStandard\ValueObject\Set\SetList; + +return static function (ContainerConfigurator $containerConfigurator): void +{ + $parameters = $containerConfigurator->parameters(); + $parameters->set(Option::PATHS, [__DIR__ . '/src', __DIR__ . '/config', __DIR__ . '/ecs.php']); + + $parameters->set(Option::SETS, [ + SetList::COMMON, + SetList::CLEAN_CODE, + SetList::DEAD_CODE, + SetList::PSR_12, + SetList::PHP_70, + SetList::PHP_71, + SetList::SYMPLIFY, + ]); +}; From d60f4a61454fcd5e3ef9bde9ac9432107d9783c8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba <tomas.vot@gmail.com> Date: Wed, 28 Oct 2020 23:35:53 +0100 Subject: [PATCH 02/21] apply coding standard on README --- .github/workflows/coding_standard.yaml | 2 +- README.md | 260 +++++++++++++++++++------ composer.json | 4 +- ecs.php | 6 +- 4 files changed, 206 insertions(+), 66 deletions(-) diff --git a/.github/workflows/coding_standard.yaml b/.github/workflows/coding_standard.yaml index 79d93b3a..5e34e77c 100644 --- a/.github/workflows/coding_standard.yaml +++ b/.github/workflows/coding_standard.yaml @@ -17,4 +17,4 @@ jobs: - run: composer install --no-progress - - run: vendor/bin/ecs check README.md --ansi \ No newline at end of file + - run: vendor/bin/ecs check-markdown README.md --ansi \ No newline at end of file diff --git a/README.md b/README.md index a3318888..fee6bba4 100644 --- a/README.md +++ b/README.md @@ -69,12 +69,16 @@ Although many developers still use PHP 5, most of the examples in this article o **Bad:** ```php +declare(strict_types=1); + $ymdstr = $moment->format('y-m-d'); ``` **Good:** ```php +declare(strict_types=1); + $currentDate = $moment->format('y-m-d'); ``` @@ -85,6 +89,8 @@ $currentDate = $moment->format('y-m-d'); **Bad:** ```php +declare(strict_types=1); + getUserInfo(); getUserData(); getUserRecord(); @@ -94,6 +100,8 @@ getUserProfile(); **Good:** ```php +declare(strict_types=1); + getUser(); ``` @@ -109,6 +117,8 @@ Make your names searchable. **Bad:** ```php +declare(strict_types=1); + // What the heck is 448 for? $result = $serializer->serialize($data, 448); ``` @@ -116,6 +126,8 @@ $result = $serializer->serialize($data, 448); **Good:** ```php +declare(strict_types=1); + $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` @@ -124,6 +136,8 @@ $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT **Bad:** ```php +declare(strict_types=1); + class User { // What the heck is 7 for? @@ -142,11 +156,16 @@ $user->access ^= 2; **Good:** ```php +declare(strict_types=1); + class User { public const ACCESS_READ = 1; + public const ACCESS_CREATE = 2; + public const ACCESS_UPDATE = 4; + public const ACCESS_DELETE = 8; // User as default can read, create and update something @@ -168,6 +187,8 @@ $user->access ^= User::ACCESS_CREATE; **Bad:** ```php +declare(strict_types=1); + $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/'; preg_match($cityZipCodeRegex, $address, $matches); @@ -180,6 +201,8 @@ saveCityZipCode($matches[1], $matches[2]); It's better, but we are still heavily dependent on regex. ```php +declare(strict_types=1); + $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/'; preg_match($cityZipCodeRegex, $address, $matches); @@ -193,6 +216,8 @@ saveCityZipCode($city, $zipCode); Decrease dependence on regex by naming subpatterns. ```php +declare(strict_types=1); + $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,]+,\s*(?<city>.+?)\s*(?<zipCode>\d{5})$/'; preg_match($cityZipCodeRegex, $address, $matches); @@ -210,6 +235,8 @@ than implicit. **Bad:** ```php +declare(strict_types=1); + function isShopOpen($day): bool { if ($day) { @@ -221,30 +248,27 @@ function isShopOpen($day): bool return true; } elseif ($day === 'sunday') { return true; - } else { - return false; } - } else { return false; } - } else { return false; } + return false; } ``` **Good:** ```php +declare(strict_types=1); + function isShopOpen(string $day): bool { if (empty($day)) { return false; } - $openingDays = [ - 'friday', 'saturday', 'sunday' - ]; + $openingDays = ['friday', 'saturday', 'sunday']; return in_array(strtolower($day), $openingDays, true); } @@ -257,27 +281,28 @@ function isShopOpen(string $day): bool **Bad:** ```php +declare(strict_types=1); + 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; + return 1; } - } else { - return 'Not supported'; + return 0; } + return 'Not supported'; } ``` **Good:** ```php +declare(strict_types=1); + function fibonacci(int $n): int { if ($n === 0 || $n === 1) { @@ -285,7 +310,7 @@ function fibonacci(int $n): int } if ($n >= 50) { - throw new \Exception('Not supported'); + throw new Exception('Not supported'); } return fibonacci($n - 1) + fibonacci($n - 2); @@ -319,6 +344,8 @@ for ($i = 0; $i < count($l); $i++) { **Good:** ```php +declare(strict_types=1); + $locations = ['Austin', 'New York', 'San Francisco']; foreach ($locations as $location) { @@ -341,10 +368,14 @@ variable name. **Bad:** ```php +declare(strict_types=1); + class Car { public $carMake; + public $carModel; + public $carColor; //... @@ -354,10 +385,14 @@ class Car **Good:** ```php +declare(strict_types=1); + class Car { public $make; + public $model; + public $color; //... @@ -413,11 +448,13 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void The simple comparison will convert the string in an integer. ```php +declare(strict_types=1); + $a = '42'; $b = 42; -if ($a != $b) { - // The expression will always pass +if ($a !== $b) { + // The expression will always pass } ``` @@ -429,6 +466,8 @@ The string `42` is different than the integer `42`. The identical comparison will compare type and value. ```php +declare(strict_types=1); + $a = '42'; $b = 42; @@ -448,6 +487,8 @@ Null coalescing is a new operator [introduced in PHP 7](https://www.php.net/manu **Bad:** ```php +declare(strict_types=1); + if (isset($_GET['name'])) { $name = $_GET['name']; } elseif (isset($_POST['name'])) { @@ -459,6 +500,8 @@ if (isset($_GET['name'])) { **Good:** ```php +declare(strict_types=1); + $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ``` @@ -480,6 +523,8 @@ of the time a higher-level object will suffice as an argument. **Bad:** ```php +declare(strict_types=1); + class Questionnaire { public function __construct( @@ -500,10 +545,14 @@ class Questionnaire **Good:** ```php +declare(strict_types=1); + class Name { private $firstname; + private $lastname; + private $patronymic; public function __construct(string $firstname, string $lastname, string $patronymic) @@ -519,7 +568,9 @@ class Name class City { private $region; + private $district; + private $city; public function __construct(string $region, string $district, string $city) @@ -535,6 +586,7 @@ class City class Contact { private $phone; + private $email; public function __construct(string $phone, string $email) @@ -606,6 +658,8 @@ testing. **Bad:** ```php +declare(strict_types=1); + function parseBetterPHPAlternative(string $code): void { $regexes = [ @@ -744,10 +798,12 @@ based on a boolean. **Bad:** ```php +declare(strict_types=1); + function createFile(string $name, bool $temp = false): void { if ($temp) { - touch('./temp/'.$name); + touch('./temp/' . $name); } else { touch($name); } @@ -757,6 +813,8 @@ function createFile(string $name, bool $temp = false): void **Good:** ```php +declare(strict_types=1); + function createFile(string $name): void { touch($name); @@ -764,7 +822,7 @@ function createFile(string $name): void function createTempFile(string $name): void { - touch('./temp/'.$name); + touch('./temp/' . $name); } ``` @@ -789,6 +847,8 @@ than the vast majority of other programmers. **Bad:** ```php +declare(strict_types=1); + // 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'; @@ -802,12 +862,15 @@ function splitIntoFirstAndLastName(): void splitIntoFirstAndLastName(); -var_dump($name); // ['Ryan', 'McDermott']; +// ['Ryan', 'McDermott']; +var_dump($name); ``` **Good:** ```php +declare(strict_types=1); + function splitIntoFirstAndLastName(string $name): array { return explode(' ', $name); @@ -816,8 +879,10 @@ function splitIntoFirstAndLastName(string $name): array $name = 'Ryan McDermott'; $newName = splitIntoFirstAndLastName($name); -var_dump($name); // 'Ryan McDermott'; -var_dump($newName); // ['Ryan', 'McDermott']; +// 'Ryan McDermott'; +var_dump($name); +// ['Ryan', 'McDermott']; +var_dump($newName); ``` **[⬆ back to top](#table-of-contents)** @@ -833,9 +898,11 @@ that tried to do the same thing. **Bad:** ```php +declare(strict_types=1); + function config(): array { - return [ + return [ 'foo' => 'bar', ]; } @@ -844,6 +911,8 @@ function config(): array **Good:** ```php +declare(strict_types=1); + class Configuration { private $configuration = []; @@ -855,7 +924,7 @@ class Configuration public function get(string $key): ?string { - // null coalescing operator + // null coalescing operator return $this->configuration[$key] ?? null; } } @@ -864,6 +933,8 @@ class Configuration Load configuration and create instance of `Configuration` class ```php +declare(strict_types=1); + $configuration = new Configuration([ 'foo' => 'bar', ]); @@ -886,6 +957,8 @@ There is also very good thoughts by [Misko Hevery](http://misko.hevery.com/about **Bad:** ```php +declare(strict_types=1); + class DBConnection { private static $instance; @@ -895,7 +968,7 @@ class DBConnection // ... } - public static function getInstance(): DBConnection + public static function getInstance(): self { if (self::$instance === null) { self::$instance = new self(); @@ -913,6 +986,8 @@ $singleton = DBConnection::getInstance(); **Good:** ```php +declare(strict_types=1); + class DBConnection { public function __construct(string $dsn) @@ -920,13 +995,15 @@ class DBConnection // ... } - // ... + // ... } ``` Create instance of `DBConnection` class and configure it with [DSN](http://php.net/manual/en/pdo.construct.php#refsect1-pdo.construct-parameters). ```php +declare(strict_types=1); + $connection = new DBConnection($dsn); ``` @@ -939,6 +1016,8 @@ And now you must use instance of `DBConnection` in your application. **Bad:** ```php +declare(strict_types=1); + if ($article->state === 'published') { // ... } @@ -947,6 +1026,8 @@ if ($article->state === 'published') { **Good:** ```php +declare(strict_types=1); + if ($article->isPublished()) { // ... } @@ -959,13 +1040,14 @@ if ($article->isPublished()) { **Bad:** ```php -function isDOMNodeNotPresent(\DOMNode $node): bool +declare(strict_types=1); + +function isDOMNodeNotPresent(DOMNode $node): bool { // ... } -if (!isDOMNodeNotPresent($node)) -{ +if (! isDOMNodeNotPresent($node)) { // ... } ``` @@ -973,7 +1055,9 @@ if (!isDOMNodeNotPresent($node)) **Good:** ```php -function isDOMNodePresent(\DOMNode $node): bool +declare(strict_types=1); + +function isDOMNodePresent(DOMNode $node): bool { // ... } @@ -999,6 +1083,8 @@ just do one thing. **Bad:** ```php +declare(strict_types=1); + class Airplane { // ... @@ -1020,6 +1106,8 @@ class Airplane **Good:** ```php +declare(strict_types=1); + interface Airplane { // ... @@ -1070,6 +1158,8 @@ The first thing to consider is consistent APIs. **Bad:** ```php +declare(strict_types=1); + function travelToTexas($vehicle): void { if ($vehicle instanceof Bicycle) { @@ -1083,6 +1173,8 @@ function travelToTexas($vehicle): void **Good:** ```php +declare(strict_types=1); + function travelToTexas(Vehicle $vehicle): void { $vehicle->travelTo(new Location('texas')); @@ -1106,10 +1198,12 @@ Otherwise, do all of that but with PHP strict type declaration or strict mode. **Bad:** ```php +declare(strict_types=1); + function combine($val1, $val2): int { - if (!is_numeric($val1) || !is_numeric($val2)) { - throw new \Exception('Must be of type Number'); + if (! is_numeric($val1) || ! is_numeric($val2)) { + throw new Exception('Must be of type Number'); } return $val1 + $val2; @@ -1119,6 +1213,8 @@ function combine($val1, $val2): int **Good:** ```php +declare(strict_types=1); + function combine(int $val1, int $val2): int { return $val1 + $val2; @@ -1136,6 +1232,8 @@ in your version history if you still need it. **Bad:** ```php +declare(strict_types=1); + function oldRequestModule(string $url): void { // ... @@ -1153,6 +1251,8 @@ inventoryTracker('apples', $request, 'www.inventory-awesome.io'); **Good:** ```php +declare(strict_types=1); + function requestModule(string $url): void { // ... @@ -1186,6 +1286,8 @@ Additionally, this is part of [Open/Closed](#openclosed-principle-ocp) principle **Bad:** ```php +declare(strict_types=1); + class BankAccount { public $balance = 1000; @@ -1253,6 +1355,8 @@ For more informations you can read the [blog post](http://fabien.potencier.org/p **Bad:** ```php +declare(strict_types=1); + class Employee { public $name; @@ -1264,12 +1368,15 @@ class Employee } $employee = new Employee('John Doe'); -echo 'Employee name: '.$employee->name; // Employee name: John Doe +// Employee name: John Doe +echo 'Employee name: ' . $employee->name; ``` **Good:** ```php +declare(strict_types=1); + class Employee { private $name; @@ -1286,7 +1393,8 @@ class Employee } $employee = new Employee('John Doe'); -echo 'Employee name: '.$employee->getName(); // Employee name: John Doe +// Employee name: John Doe +echo 'Employee name: ' . $employee->getName(); ``` **[⬆ back to top](#table-of-contents)** @@ -1315,9 +1423,12 @@ relationship (Human->Animal vs. User->UserDetails). **Bad:** ```php +declare(strict_types=1); + class Employee { private $name; + private $email; public function __construct(string $name, string $email) @@ -1335,6 +1446,7 @@ class Employee class EmployeeTaxData extends Employee { private $ssn; + private $salary; public function __construct(string $name, string $email, string $ssn, string $salary) @@ -1352,9 +1464,12 @@ class EmployeeTaxData extends Employee **Good:** ```php +declare(strict_types=1); + class EmployeeTaxData { private $ssn; + private $salary; public function __construct(string $ssn, string $salary) @@ -1369,7 +1484,9 @@ class EmployeeTaxData class Employee { private $name; + private $email; + private $taxData; public function __construct(string $name, string $email) @@ -1378,7 +1495,7 @@ class Employee $this->email = $email; } - public function setTaxData(EmployeeTaxData $taxData) + public function setTaxData(EmployeeTaxData $taxData): void { $this->taxData = $taxData; } @@ -1411,10 +1528,14 @@ on this topic written by [Marco Pivetta](https://github.com/Ocramius). **Bad:** ```php +declare(strict_types=1); + class Car { private $make = 'Honda'; + private $model = 'Accord'; + private $color = 'white'; public function setMake(string $make): self @@ -1448,19 +1569,23 @@ class Car } $car = (new Car()) - ->setColor('pink') - ->setMake('Ford') - ->setModel('F-150') - ->dump(); + ->setColor('pink') + ->setMake('Ford') + ->setModel('F-150') + ->dump(); ``` **Good:** ```php +declare(strict_types=1); + class Car { private $make = 'Honda'; + private $model = 'Accord'; + private $color = 'white'; public function setMake(string $make): void @@ -1510,6 +1635,8 @@ For more informations you can read [the blog post](https://ocramius.github.io/bl **Bad:** ```php +declare(strict_types=1); + final class Car { private $color; @@ -1532,6 +1659,8 @@ final class Car **Good:** ```php +declare(strict_types=1); + interface Vehicle { /** @@ -1549,9 +1678,6 @@ final class Car implements Vehicle $this->color = $color; } - /** - * {@inheritdoc} - */ public function getColor() { return $this->color; @@ -1585,6 +1711,8 @@ your codebase. **Bad:** ```php +declare(strict_types=1); + class UserSettings { private $user; @@ -1611,6 +1739,8 @@ class UserSettings **Good:** ```php +declare(strict_types=1); + class UserAuth { private $user; @@ -1629,6 +1759,7 @@ class UserAuth class UserSettings { private $user; + private $auth; public function __construct(User $user) @@ -1658,6 +1789,8 @@ add new functionalities without changing existing code. **Bad:** ```php +declare(strict_types=1); + abstract class Adapter { protected $name; @@ -1723,6 +1856,8 @@ class HttpRequester **Good:** ```php +declare(strict_types=1); + interface Adapter { public function request(string $url): Promise; @@ -1780,9 +1915,12 @@ get into trouble. **Bad:** ```php +declare(strict_types=1); + class Rectangle { protected $width = 0; + protected $height = 0; public function setWidth(int $width): void @@ -1820,7 +1958,7 @@ function printArea(Rectangle $rectangle): void $rectangle->setHeight(5); // BAD: Will return 25 for Square. Should be 20. - echo sprintf('%s has area %d.', get_class($rectangle), $rectangle->getArea()).PHP_EOL; + echo sprintf('%s has area %d.', get_class($rectangle), $rectangle->getArea()) . PHP_EOL; } $rectangles = [new Rectangle(), new Square()]; @@ -1903,6 +2041,8 @@ all of the settings. Making them optional helps prevent having a "fat interface" **Bad:** ```php +declare(strict_types=1); + interface Employee { public function work(): void; @@ -1942,6 +2082,8 @@ class RobotEmployee implements Employee Not every worker is an employee, but every employee is a worker. ```php +declare(strict_types=1); + interface Workable { public function work(): void; @@ -1999,6 +2141,8 @@ it makes your code hard to refactor. **Bad:** ```php +declare(strict_types=1); + class Employee { public function work(): void @@ -2034,6 +2178,8 @@ class Manager **Good:** ```php +declare(strict_types=1); + interface Employee { public function work(): void; @@ -2101,17 +2247,15 @@ updating multiple places any time you want to change one thing. **Bad:** ```php +declare(strict_types=1); + function showDeveloperList(array $developers): void { foreach ($developers as $developer) { $expectedSalary = $developer->calculateExpectedSalary(); $experience = $developer->getExperience(); $githubLink = $developer->getGithubLink(); - $data = [ - $expectedSalary, - $experience, - $githubLink - ]; + $data = [$expectedSalary, $experience, $githubLink]; render($data); } @@ -2123,11 +2267,7 @@ function showManagerList(array $managers): void $expectedSalary = $manager->calculateExpectedSalary(); $experience = $manager->getExperience(); $githubLink = $manager->getGithubLink(); - $data = [ - $expectedSalary, - $experience, - $githubLink - ]; + $data = [$expectedSalary, $experience, $githubLink]; render($data); } @@ -2137,17 +2277,15 @@ function showManagerList(array $managers): void **Good:** ```php +declare(strict_types=1); + function showList(array $employees): void { foreach ($employees as $employee) { $expectedSalary = $employee->calculateExpectedSalary(); $experience = $employee->getExperience(); $githubLink = $employee->getGithubLink(); - $data = [ - $expectedSalary, - $experience, - $githubLink - ]; + $data = [$expectedSalary, $experience, $githubLink]; render($data); } @@ -2159,14 +2297,12 @@ function showList(array $employees): void It is better to use a compact version of the code. ```php +declare(strict_types=1); + function showList(array $employees): void { foreach ($employees as $employee) { - render([ - $employee->calculateExpectedSalary(), - $employee->getExperience(), - $employee->getGithubLink() - ]); + render([$employee->calculateExpectedSalary(), $employee->getExperience(), $employee->getGithubLink()]); } } ``` diff --git a/composer.json b/composer.json index 733367b9..a9d75033 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "symplify/easy-coding-standard": "^8.3" }, "scripts": { - "check-cs": "vendor/bin/ecs check README.md", - "fix-cs": "vendor/bin/ecs check README.md --fix" + "check-cs": "vendor/bin/ecs check-markdown README.md", + "fix-cs": "vendor/bin/ecs check-markdown README.md --fix" } } diff --git a/ecs.php b/ecs.php index 5d564993..82c2f242 100644 --- a/ecs.php +++ b/ecs.php @@ -3,7 +3,7 @@ declare(strict_types=1); use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; -use Symplify\EasyCodingStandard\Configuration\Option; +use Symplify\EasyCodingStandard\ValueObject\Option; use Symplify\EasyCodingStandard\ValueObject\Set\SetList; return static function (ContainerConfigurator $containerConfigurator): void @@ -20,4 +20,8 @@ SetList::PHP_71, SetList::SYMPLIFY, ]); + + $parameters->set(Option::SKIP, [ + \PhpCsFixer\Fixer\PhpTag\BlankLineAfterOpeningTagFixer::class => null, + ]); }; From 9df6c175c9c65255451f11983ab1e39c28319429 Mon Sep 17 00:00:00 2001 From: Tomas Votruba <tomas.vot@gmail.com> Date: Wed, 28 Oct 2020 23:47:53 +0100 Subject: [PATCH 03/21] workflow: update php --- .github/workflows/coding_standard.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coding_standard.yaml b/.github/workflows/coding_standard.yaml index 5e34e77c..4e9ee45c 100644 --- a/.github/workflows/coding_standard.yaml +++ b/.github/workflows/coding_standard.yaml @@ -10,7 +10,7 @@ jobs: steps: - uses: actions/checkout@v2 # see https://github.com/shivammathur/setup-php - - uses: shivammathur/setup-php@v1 + - uses: shivammathur/setup-php@v2 with: php-version: 7.4 coverage: none From 43dfecd7d6ae7f65a1e655a0d6005f2120dd811c Mon Sep 17 00:00:00 2001 From: Tomas Votruba <tomas.vot@gmail.com> Date: Wed, 28 Oct 2020 23:50:49 +0100 Subject: [PATCH 04/21] restore required mallforms --- .github/workflows/coding_standard.yaml | 2 +- README.md | 9 +++++---- ecs.php | 5 ++++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/coding_standard.yaml b/.github/workflows/coding_standard.yaml index 4e9ee45c..fa949ec7 100644 --- a/.github/workflows/coding_standard.yaml +++ b/.github/workflows/coding_standard.yaml @@ -15,6 +15,6 @@ jobs: php-version: 7.4 coverage: none - - run: composer install --no-progress + - run: composer install --no-progress --ansi - run: vendor/bin/ecs check-markdown README.md --ansi \ No newline at end of file diff --git a/README.md b/README.md index fee6bba4..3fd40d93 100644 --- a/README.md +++ b/README.md @@ -453,7 +453,7 @@ declare(strict_types=1); $a = '42'; $b = 42; -if ($a !== $b) { +if ($a != $b) { // The expression will always pass } ``` @@ -862,8 +862,8 @@ function splitIntoFirstAndLastName(): void splitIntoFirstAndLastName(); -// ['Ryan', 'McDermott']; var_dump($name); +// ['Ryan', 'McDermott']; ``` **Good:** @@ -879,10 +879,11 @@ function splitIntoFirstAndLastName(string $name): array $name = 'Ryan McDermott'; $newName = splitIntoFirstAndLastName($name); -// 'Ryan McDermott'; var_dump($name); -// ['Ryan', 'McDermott']; +// 'Ryan McDermott'; + var_dump($newName); +// ['Ryan', 'McDermott']; ``` **[⬆ back to top](#table-of-contents)** diff --git a/ecs.php b/ecs.php index 82c2f242..9555953f 100644 --- a/ecs.php +++ b/ecs.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use PhpCsFixer\Fixer\PhpTag\BlankLineAfterOpeningTagFixer; +use PhpCsFixer\Fixer\Strict\StrictComparisonFixer; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use Symplify\EasyCodingStandard\ValueObject\Option; use Symplify\EasyCodingStandard\ValueObject\Set\SetList; @@ -22,6 +24,7 @@ ]); $parameters->set(Option::SKIP, [ - \PhpCsFixer\Fixer\PhpTag\BlankLineAfterOpeningTagFixer::class => null, + BlankLineAfterOpeningTagFixer::class => null, + StrictComparisonFixer::class => null, ]); }; From c4bdcb6a5e49085f4f221aaf2fb8cba05c688ba9 Mon Sep 17 00:00:00 2001 From: Peter Gribanov <info@peter-gribanov.ru> Date: Fri, 15 Jan 2021 20:21:34 +0300 Subject: [PATCH 05/21] add link to persian translation --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 3fd40d93..7c9dff0c 100644 --- a/README.md +++ b/README.md @@ -2333,5 +2333,7 @@ This is also available in other languages: * [yujineeee/clean-code-php](https://github.com/yujineeee/clean-code-php) * :tr: **Turkish:** * [anilozmen/clean-code-php](https://github.com/anilozmen/clean-code-php) +* :iran: **Persian:** + * [amirshnll/clean-code-php](https://github.com/amirshnll/clean-code-php) **[⬆ back to top](#table-of-contents)** From 3218b7e11740bd7e117f8dece898f71983a39c47 Mon Sep 17 00:00:00 2001 From: diegoe <diegoe@gnome.org> Date: Sat, 6 Feb 2021 03:32:54 -0500 Subject: [PATCH 06/21] Misc spelling and grammar touch ups --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3fd40d93..3ee84876 100644 --- a/README.md +++ b/README.md @@ -1351,7 +1351,7 @@ $balance = $bankAccount->getBalance(); Therefore, use `private` by default and `public/protected` when you need to provide access for external classes. -For more informations you can read the [blog post](http://fabien.potencier.org/pragmatism-over-theory-protected-vs-private.html) on this topic written by [Fabien Potencier](https://github.com/fabpot). +For more information you can read the [blog post](http://fabien.potencier.org/pragmatism-over-theory-protected-vs-private.html) on this topic written by [Fabien Potencier](https://github.com/fabpot). **Bad:** @@ -1523,7 +1523,7 @@ more often it comes at some costs: 3. Is harder to [mock](https://en.wikipedia.org/wiki/Mock_object) in a test suite. 4. Makes diffs of commits harder to read. -For more informations you can read the full [blog post](https://ocramius.github.io/blog/fluent-interfaces-are-evil/) +For more information you can read the full [blog post](https://ocramius.github.io/blog/fluent-interfaces-are-evil/) on this topic written by [Marco Pivetta](https://github.com/Ocramius). **Bad:** @@ -1621,13 +1621,13 @@ $car->dump(); ### Prefer final classes -The `final` should be used whenever possible: +The `final` keyword should be used whenever possible: -1. It prevents uncontrolled inheritance chain. +1. It prevents an uncontrolled inheritance chain. 2. It encourages [composition](#prefer-composition-over-inheritance). 3. It encourages the [Single Responsibility Pattern](#single-responsibility-principle-srp). -4. It encourages developers to use your public methods instead of extending the class to get access on protected ones. -5. It allows you to change your code without any break of applications that use your class. +4. It encourages developers to use your public methods instead of extending the class to get access to protected ones. +5. It allows you to change your code without breaking applications that use your class. The only condition is that your class should implement an interface and no other public methods are defined. @@ -1974,7 +1974,7 @@ foreach ($rectangles as $rectangle) { The best way is separate the quadrangles and allocation of a more general subtype for both shapes. Despite the apparent similarity of the square and the rectangle, they are different. -A square has much in common with a rhombus, and a rectangle with a parallelogram, but they are not subtype. +A square has much in common with a rhombus, and a rectangle with a parallelogram, but they are not subtypes. A square, a rectangle, a rhombus and a parallelogram are separate shapes with their own properties, albeit similar. ```php From 0d478312fdae6045de6c17474bf5b0cf7ff88581 Mon Sep 17 00:00:00 2001 From: andyexeter <palmer.andy@gmail.com> Date: Thu, 15 Apr 2021 10:04:57 +0100 Subject: [PATCH 07/21] Remove declare strict_types from code snippets --- README.md | 136 ------------------------------------------------------ ecs.php | 2 + 2 files changed, 2 insertions(+), 136 deletions(-) diff --git a/README.md b/README.md index 3fd40d93..bf1beccc 100644 --- a/README.md +++ b/README.md @@ -69,16 +69,12 @@ Although many developers still use PHP 5, most of the examples in this article o **Bad:** ```php -declare(strict_types=1); - $ymdstr = $moment->format('y-m-d'); ``` **Good:** ```php -declare(strict_types=1); - $currentDate = $moment->format('y-m-d'); ``` @@ -89,8 +85,6 @@ $currentDate = $moment->format('y-m-d'); **Bad:** ```php -declare(strict_types=1); - getUserInfo(); getUserData(); getUserRecord(); @@ -100,8 +94,6 @@ getUserProfile(); **Good:** ```php -declare(strict_types=1); - getUser(); ``` @@ -117,8 +109,6 @@ Make your names searchable. **Bad:** ```php -declare(strict_types=1); - // What the heck is 448 for? $result = $serializer->serialize($data, 448); ``` @@ -126,8 +116,6 @@ $result = $serializer->serialize($data, 448); **Good:** ```php -declare(strict_types=1); - $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` @@ -136,8 +124,6 @@ $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT **Bad:** ```php -declare(strict_types=1); - class User { // What the heck is 7 for? @@ -156,8 +142,6 @@ $user->access ^= 2; **Good:** ```php -declare(strict_types=1); - class User { public const ACCESS_READ = 1; @@ -187,8 +171,6 @@ $user->access ^= User::ACCESS_CREATE; **Bad:** ```php -declare(strict_types=1); - $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/'; preg_match($cityZipCodeRegex, $address, $matches); @@ -201,8 +183,6 @@ saveCityZipCode($matches[1], $matches[2]); It's better, but we are still heavily dependent on regex. ```php -declare(strict_types=1); - $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/'; preg_match($cityZipCodeRegex, $address, $matches); @@ -216,8 +196,6 @@ saveCityZipCode($city, $zipCode); Decrease dependence on regex by naming subpatterns. ```php -declare(strict_types=1); - $address = 'One Infinite Loop, Cupertino 95014'; $cityZipCodeRegex = '/^[^,]+,\s*(?<city>.+?)\s*(?<zipCode>\d{5})$/'; preg_match($cityZipCodeRegex, $address, $matches); @@ -235,8 +213,6 @@ than implicit. **Bad:** ```php -declare(strict_types=1); - function isShopOpen($day): bool { if ($day) { @@ -260,8 +236,6 @@ function isShopOpen($day): bool **Good:** ```php -declare(strict_types=1); - function isShopOpen(string $day): bool { if (empty($day)) { @@ -281,8 +255,6 @@ function isShopOpen(string $day): bool **Bad:** ```php -declare(strict_types=1); - function fibonacci(int $n) { if ($n < 50) { @@ -301,8 +273,6 @@ function fibonacci(int $n) **Good:** ```php -declare(strict_types=1); - function fibonacci(int $n): int { if ($n === 0 || $n === 1) { @@ -344,8 +314,6 @@ for ($i = 0; $i < count($l); $i++) { **Good:** ```php -declare(strict_types=1); - $locations = ['Austin', 'New York', 'San Francisco']; foreach ($locations as $location) { @@ -368,8 +336,6 @@ variable name. **Bad:** ```php -declare(strict_types=1); - class Car { public $carMake; @@ -385,8 +351,6 @@ class Car **Good:** ```php -declare(strict_types=1); - class Car { public $make; @@ -448,8 +412,6 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void The simple comparison will convert the string in an integer. ```php -declare(strict_types=1); - $a = '42'; $b = 42; @@ -466,8 +428,6 @@ The string `42` is different than the integer `42`. The identical comparison will compare type and value. ```php -declare(strict_types=1); - $a = '42'; $b = 42; @@ -487,8 +447,6 @@ Null coalescing is a new operator [introduced in PHP 7](https://www.php.net/manu **Bad:** ```php -declare(strict_types=1); - if (isset($_GET['name'])) { $name = $_GET['name']; } elseif (isset($_POST['name'])) { @@ -500,8 +458,6 @@ if (isset($_GET['name'])) { **Good:** ```php -declare(strict_types=1); - $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ``` @@ -523,8 +479,6 @@ of the time a higher-level object will suffice as an argument. **Bad:** ```php -declare(strict_types=1); - class Questionnaire { public function __construct( @@ -545,8 +499,6 @@ class Questionnaire **Good:** ```php -declare(strict_types=1); - class Name { private $firstname; @@ -658,8 +610,6 @@ testing. **Bad:** ```php -declare(strict_types=1); - function parseBetterPHPAlternative(string $code): void { $regexes = [ @@ -798,8 +748,6 @@ based on a boolean. **Bad:** ```php -declare(strict_types=1); - function createFile(string $name, bool $temp = false): void { if ($temp) { @@ -813,8 +761,6 @@ function createFile(string $name, bool $temp = false): void **Good:** ```php -declare(strict_types=1); - function createFile(string $name): void { touch($name); @@ -847,8 +793,6 @@ than the vast majority of other programmers. **Bad:** ```php -declare(strict_types=1); - // 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'; @@ -869,8 +813,6 @@ var_dump($name); **Good:** ```php -declare(strict_types=1); - function splitIntoFirstAndLastName(string $name): array { return explode(' ', $name); @@ -899,8 +841,6 @@ that tried to do the same thing. **Bad:** ```php -declare(strict_types=1); - function config(): array { return [ @@ -912,8 +852,6 @@ function config(): array **Good:** ```php -declare(strict_types=1); - class Configuration { private $configuration = []; @@ -934,8 +872,6 @@ class Configuration Load configuration and create instance of `Configuration` class ```php -declare(strict_types=1); - $configuration = new Configuration([ 'foo' => 'bar', ]); @@ -958,8 +894,6 @@ There is also very good thoughts by [Misko Hevery](http://misko.hevery.com/about **Bad:** ```php -declare(strict_types=1); - class DBConnection { private static $instance; @@ -987,8 +921,6 @@ $singleton = DBConnection::getInstance(); **Good:** ```php -declare(strict_types=1); - class DBConnection { public function __construct(string $dsn) @@ -1003,8 +935,6 @@ class DBConnection Create instance of `DBConnection` class and configure it with [DSN](http://php.net/manual/en/pdo.construct.php#refsect1-pdo.construct-parameters). ```php -declare(strict_types=1); - $connection = new DBConnection($dsn); ``` @@ -1017,8 +947,6 @@ And now you must use instance of `DBConnection` in your application. **Bad:** ```php -declare(strict_types=1); - if ($article->state === 'published') { // ... } @@ -1027,8 +955,6 @@ if ($article->state === 'published') { **Good:** ```php -declare(strict_types=1); - if ($article->isPublished()) { // ... } @@ -1041,8 +967,6 @@ if ($article->isPublished()) { **Bad:** ```php -declare(strict_types=1); - function isDOMNodeNotPresent(DOMNode $node): bool { // ... @@ -1056,8 +980,6 @@ if (! isDOMNodeNotPresent($node)) { **Good:** ```php -declare(strict_types=1); - function isDOMNodePresent(DOMNode $node): bool { // ... @@ -1084,8 +1006,6 @@ just do one thing. **Bad:** ```php -declare(strict_types=1); - class Airplane { // ... @@ -1107,8 +1027,6 @@ class Airplane **Good:** ```php -declare(strict_types=1); - interface Airplane { // ... @@ -1159,8 +1077,6 @@ The first thing to consider is consistent APIs. **Bad:** ```php -declare(strict_types=1); - function travelToTexas($vehicle): void { if ($vehicle instanceof Bicycle) { @@ -1174,8 +1090,6 @@ function travelToTexas($vehicle): void **Good:** ```php -declare(strict_types=1); - function travelToTexas(Vehicle $vehicle): void { $vehicle->travelTo(new Location('texas')); @@ -1199,8 +1113,6 @@ Otherwise, do all of that but with PHP strict type declaration or strict mode. **Bad:** ```php -declare(strict_types=1); - function combine($val1, $val2): int { if (! is_numeric($val1) || ! is_numeric($val2)) { @@ -1214,8 +1126,6 @@ function combine($val1, $val2): int **Good:** ```php -declare(strict_types=1); - function combine(int $val1, int $val2): int { return $val1 + $val2; @@ -1233,8 +1143,6 @@ in your version history if you still need it. **Bad:** ```php -declare(strict_types=1); - function oldRequestModule(string $url): void { // ... @@ -1252,8 +1160,6 @@ inventoryTracker('apples', $request, 'www.inventory-awesome.io'); **Good:** ```php -declare(strict_types=1); - function requestModule(string $url): void { // ... @@ -1287,8 +1193,6 @@ Additionally, this is part of [Open/Closed](#openclosed-principle-ocp) principle **Bad:** ```php -declare(strict_types=1); - class BankAccount { public $balance = 1000; @@ -1356,8 +1260,6 @@ For more informations you can read the [blog post](http://fabien.potencier.org/p **Bad:** ```php -declare(strict_types=1); - class Employee { public $name; @@ -1376,8 +1278,6 @@ echo 'Employee name: ' . $employee->name; **Good:** ```php -declare(strict_types=1); - class Employee { private $name; @@ -1424,8 +1324,6 @@ relationship (Human->Animal vs. User->UserDetails). **Bad:** ```php -declare(strict_types=1); - class Employee { private $name; @@ -1465,8 +1363,6 @@ class EmployeeTaxData extends Employee **Good:** ```php -declare(strict_types=1); - class EmployeeTaxData { private $ssn; @@ -1529,8 +1425,6 @@ on this topic written by [Marco Pivetta](https://github.com/Ocramius). **Bad:** ```php -declare(strict_types=1); - class Car { private $make = 'Honda'; @@ -1579,8 +1473,6 @@ $car = (new Car()) **Good:** ```php -declare(strict_types=1); - class Car { private $make = 'Honda'; @@ -1636,8 +1528,6 @@ For more informations you can read [the blog post](https://ocramius.github.io/bl **Bad:** ```php -declare(strict_types=1); - final class Car { private $color; @@ -1660,8 +1550,6 @@ final class Car **Good:** ```php -declare(strict_types=1); - interface Vehicle { /** @@ -1712,8 +1600,6 @@ your codebase. **Bad:** ```php -declare(strict_types=1); - class UserSettings { private $user; @@ -1740,8 +1626,6 @@ class UserSettings **Good:** ```php -declare(strict_types=1); - class UserAuth { private $user; @@ -1790,8 +1674,6 @@ add new functionalities without changing existing code. **Bad:** ```php -declare(strict_types=1); - abstract class Adapter { protected $name; @@ -1857,8 +1739,6 @@ class HttpRequester **Good:** ```php -declare(strict_types=1); - interface Adapter { public function request(string $url): Promise; @@ -1916,8 +1796,6 @@ get into trouble. **Bad:** ```php -declare(strict_types=1); - class Rectangle { protected $width = 0; @@ -2042,8 +1920,6 @@ all of the settings. Making them optional helps prevent having a "fat interface" **Bad:** ```php -declare(strict_types=1); - interface Employee { public function work(): void; @@ -2083,8 +1959,6 @@ class RobotEmployee implements Employee Not every worker is an employee, but every employee is a worker. ```php -declare(strict_types=1); - interface Workable { public function work(): void; @@ -2142,8 +2016,6 @@ it makes your code hard to refactor. **Bad:** ```php -declare(strict_types=1); - class Employee { public function work(): void @@ -2179,8 +2051,6 @@ class Manager **Good:** ```php -declare(strict_types=1); - interface Employee { public function work(): void; @@ -2248,8 +2118,6 @@ updating multiple places any time you want to change one thing. **Bad:** ```php -declare(strict_types=1); - function showDeveloperList(array $developers): void { foreach ($developers as $developer) { @@ -2278,8 +2146,6 @@ function showManagerList(array $managers): void **Good:** ```php -declare(strict_types=1); - function showList(array $employees): void { foreach ($employees as $employee) { @@ -2298,8 +2164,6 @@ function showList(array $employees): void It is better to use a compact version of the code. ```php -declare(strict_types=1); - function showList(array $employees): void { foreach ($employees as $employee) { diff --git a/ecs.php b/ecs.php index 9555953f..9b945f6b 100644 --- a/ecs.php +++ b/ecs.php @@ -3,6 +3,7 @@ declare(strict_types=1); use PhpCsFixer\Fixer\PhpTag\BlankLineAfterOpeningTagFixer; +use PhpCsFixer\Fixer\Strict\DeclareStrictTypesFixer; use PhpCsFixer\Fixer\Strict\StrictComparisonFixer; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use Symplify\EasyCodingStandard\ValueObject\Option; @@ -26,5 +27,6 @@ $parameters->set(Option::SKIP, [ BlankLineAfterOpeningTagFixer::class => null, StrictComparisonFixer::class => null, + DeclareStrictTypesFixer::class => null, ]); }; From 8572f4c5ba5d930dba7d85b7007514ab45ca4df0 Mon Sep 17 00:00:00 2001 From: Peter Gribanov <wcode404@gmail.com> Date: Wed, 28 Apr 2021 15:11:17 +0300 Subject: [PATCH 08/21] move "Use default arguments instead of short circuiting or conditionals" section into "Functions" section --- README.md | 78 +++++++++++++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index bf1beccc..58a9e35c 100644 --- a/README.md +++ b/README.md @@ -13,11 +13,11 @@ * [Avoid nesting too deeply and return early (part 2)](#avoid-nesting-too-deeply-and-return-early-part-2) * [Avoid Mental Mapping](#avoid-mental-mapping) * [Don't add unneeded context](#dont-add-unneeded-context) - * [Use default arguments instead of short circuiting or conditionals](#use-default-arguments-instead-of-short-circuiting-or-conditionals) 3. [Comparison](#comparison) * [Use identical comparison](#use-identical-comparison) * [Null coalescing operator](#null-coalescing-operator) 4. [Functions](#functions) + * [Use default arguments instead of short circuiting or conditionals](#use-default-arguments-instead-of-short-circuiting-or-conditionals) * [Function arguments (2 or fewer ideally)](#function-arguments-2-or-fewer-ideally) * [Function names should say what they do](#function-names-should-say-what-they-do) * [Functions should only be one level of abstraction](#functions-should-only-be-one-level-of-abstraction) @@ -365,44 +365,6 @@ class Car **[⬆ back to top](#table-of-contents)** -### Use default arguments instead of short circuiting or conditionals - -**Not good:** - -This is not good because `$breweryName` can be `NULL`. - -```php -function createMicrobrewery($breweryName = 'Hipster Brew Co.'): void -{ -    // ... -} -``` - -**Not bad:** - -This opinion is more understandable than the previous version, but it better controls the value of the variable. - -```php -function createMicrobrewery($name = null): void -{ -    $breweryName = $name ?: 'Hipster Brew Co.'; - // ... -} -``` - -**Good:** - - 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`. - -```php -function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void -{ -    // ... -} -``` - -**[⬆ back to top](#table-of-contents)** - ## Comparison ### Use [identical comparison](http://php.net/manual/en/language.operators.comparison.php) @@ -465,6 +427,44 @@ $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ## Functions +### Use default arguments instead of short circuiting or conditionals + +**Not good:** + +This is not good because `$breweryName` can be `NULL`. + +```php +function createMicrobrewery($breweryName = 'Hipster Brew Co.'): void +{ + // ... +} +``` + +**Not bad:** + +This opinion is more understandable than the previous version, but it better controls the value of the variable. + +```php +function createMicrobrewery($name = null): void +{ + $breweryName = $name ?: 'Hipster Brew Co.'; + // ... +} +``` + +**Good:** + + 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`. + +```php +function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void +{ + // ... +} +``` + +**[⬆ back to top](#table-of-contents)** + ### Function arguments (2 or fewer ideally) Limiting the amount of function parameters is incredibly important because it makes From b12e9867039be9cb85f0d3d2421006219ae17a4e Mon Sep 17 00:00:00 2001 From: Ilyes Chouia <celyes02@gmail.com> Date: Tue, 25 May 2021 14:12:39 +0100 Subject: [PATCH 09/21] fix typo change 'Single Responsibility Pattern' to 'Single Responsibility Principle' in `Prefer final classes` chapter --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index bf1beccc..2d94352a 100644 --- a/README.md +++ b/README.md @@ -1511,13 +1511,14 @@ $car->dump(); **[⬆ back to top](#table-of-contents)** -### Prefer final classes +### Prefer +classes The `final` should be used whenever possible: 1. It prevents uncontrolled inheritance chain. 2. It encourages [composition](#prefer-composition-over-inheritance). -3. It encourages the [Single Responsibility Pattern](#single-responsibility-principle-srp). +3. It encourages the [Single Responsibility Principle](#single-responsibility-principle-srp). 4. It encourages developers to use your public methods instead of extending the class to get access on protected ones. 5. It allows you to change your code without any break of applications that use your class. From c6808b9448aebcf4a50afff1027e97e380fde5c9 Mon Sep 17 00:00:00 2001 From: Ilyes Chouia <celyes02@gmail.com> Date: Tue, 25 May 2021 14:16:05 +0100 Subject: [PATCH 10/21] fix link issue --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 2d94352a..149f49c3 100644 --- a/README.md +++ b/README.md @@ -1511,8 +1511,7 @@ $car->dump(); **[⬆ back to top](#table-of-contents)** -### Prefer -classes +### Prefer final classes The `final` should be used whenever possible: From 0374baa8a8299780f5284c44da6d23440b081f3d Mon Sep 17 00:00:00 2001 From: TomasVotruba <tomas.vot@gmail.com> Date: Wed, 26 May 2021 11:24:36 +0100 Subject: [PATCH 11/21] update ECS and composer.json, allow PHP 7.2+ --- .github/workflows/coding_standard.yaml | 4 ++-- composer.json | 6 ++++-- ecs.php | 15 +++++---------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/.github/workflows/coding_standard.yaml b/.github/workflows/coding_standard.yaml index fa949ec7..2fa10664 100644 --- a/.github/workflows/coding_standard.yaml +++ b/.github/workflows/coding_standard.yaml @@ -12,9 +12,9 @@ jobs: # see https://github.com/shivammathur/setup-php - uses: shivammathur/setup-php@v2 with: - php-version: 7.4 + php-version: 8.0 coverage: none - run: composer install --no-progress --ansi - - run: vendor/bin/ecs check-markdown README.md --ansi \ No newline at end of file + - run: vendor/bin/ecs check-markdown README.md --ansi diff --git a/composer.json b/composer.json index a9d75033..9c258aa3 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,9 @@ { + "name": "jupeter/clean-code-php", + "description": "Clean Code concepts adapted for PHP", "require": { - "php": "^7.2", - "symplify/easy-coding-standard": "^8.3" + "php": ">=7.2", + "symplify/easy-coding-standard": "^9.3" }, "scripts": { "check-cs": "vendor/bin/ecs check-markdown README.md", diff --git a/ecs.php b/ecs.php index 9b945f6b..b2e209ef 100644 --- a/ecs.php +++ b/ecs.php @@ -11,19 +11,14 @@ return static function (ContainerConfigurator $containerConfigurator): void { + $containerConfigurator->import(SetList::COMMON); + $containerConfigurator->import(SetList::CLEAN_CODE); + $containerConfigurator->import(SetList::PSR_12); + $containerConfigurator->import(SetList::SYMPLIFY); + $parameters = $containerConfigurator->parameters(); $parameters->set(Option::PATHS, [__DIR__ . '/src', __DIR__ . '/config', __DIR__ . '/ecs.php']); - $parameters->set(Option::SETS, [ - SetList::COMMON, - SetList::CLEAN_CODE, - SetList::DEAD_CODE, - SetList::PSR_12, - SetList::PHP_70, - SetList::PHP_71, - SetList::SYMPLIFY, - ]); - $parameters->set(Option::SKIP, [ BlankLineAfterOpeningTagFixer::class => null, StrictComparisonFixer::class => null, From 9fab86d007d62dbb9fcf85c70b9c8398371099ce Mon Sep 17 00:00:00 2001 From: TomasVotruba <tomas.vot@gmail.com> Date: Wed, 26 May 2021 11:26:19 +0100 Subject: [PATCH 12/21] [CI] run on push too --- .github/workflows/coding_standard.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/coding_standard.yaml b/.github/workflows/coding_standard.yaml index 2fa10664..262b528b 100644 --- a/.github/workflows/coding_standard.yaml +++ b/.github/workflows/coding_standard.yaml @@ -2,6 +2,7 @@ name: Coding Standard on: pull_request: null + push: null jobs: coding_standard: From c829a21050e0c37f09ddaa51c0c030e7a84e746a Mon Sep 17 00:00:00 2001 From: Nayeem <nayeem9812@gmail.com> Date: Sat, 16 Oct 2021 02:58:50 +0600 Subject: [PATCH 13/21] Add link to Bengali translation --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3a6994fb..9ec2dc46 100644 --- a/README.md +++ b/README.md @@ -2198,6 +2198,8 @@ This is also available in other languages: * :tr: **Turkish:** * [anilozmen/clean-code-php](https://github.com/anilozmen/clean-code-php) * :iran: **Persian:** - * [amirshnll/clean-code-php](https://github.com/amirshnll/clean-code-php) + * [amirshnll/clean-code-php](https://github.com/amirshnll/clean-code-php) +* :bangladesh: **Bangla:** + * [nayeemdev/clean-code-php](https://github.com/nayeemdev/clean-code-php) **[⬆ back to top](#table-of-contents)** From c46dfa4816efdd510e30322f3102a036de99bb99 Mon Sep 17 00:00:00 2001 From: Igor Popov <igrppv@gmail.com> Date: Wed, 17 Nov 2021 14:27:02 +0300 Subject: [PATCH 14/21] Add colon after Vietnamese translation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9ec2dc46..030fcfc8 100644 --- a/README.md +++ b/README.md @@ -2191,7 +2191,7 @@ This is also available in other languages: * [panuwizzle/clean-code-php](https://github.com/panuwizzle/clean-code-php) * :fr: **French:** * [errorname/clean-code-php](https://github.com/errorname/clean-code-php) -* :vietnam: **Vietnamese** +* :vietnam: **Vietnamese:** * [viethuongdev/clean-code-php](https://github.com/viethuongdev/clean-code-php) * :kr: **Korean:** * [yujineeee/clean-code-php](https://github.com/yujineeee/clean-code-php) From 51ed366286b8c3cf6deea3b1226d470014a931f3 Mon Sep 17 00:00:00 2001 From: Igor Popov <igrppv@gmail.com> Date: Thu, 25 Nov 2021 23:57:37 +0300 Subject: [PATCH 15/21] Rename pattern to principle --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 030fcfc8..20b0b82f 100644 --- a/README.md +++ b/README.md @@ -1517,7 +1517,7 @@ The `final` keyword should be used whenever possible: 1. It prevents an uncontrolled inheritance chain. 2. It encourages [composition](#prefer-composition-over-inheritance). -3. It encourages the [Single Responsibility Pattern](#single-responsibility-principle-srp). +3. It encourages the [Single Responsibility Principle](#single-responsibility-principle-srp). 4. It encourages developers to use your public methods instead of extending the class to get access to protected ones. 5. It allows you to change your code without breaking applications that use your class. From 6e672f6453fd8f4cc0086a9eaa479f4efa5853f5 Mon Sep 17 00:00:00 2001 From: Ahmed Joda <ahmedjoda02@gmail.com> Date: Sat, 27 Nov 2021 19:50:46 +0200 Subject: [PATCH 16/21] add arabic translation --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 20b0b82f..2708e171 100644 --- a/README.md +++ b/README.md @@ -2201,5 +2201,7 @@ This is also available in other languages: * [amirshnll/clean-code-php](https://github.com/amirshnll/clean-code-php) * :bangladesh: **Bangla:** * [nayeemdev/clean-code-php](https://github.com/nayeemdev/clean-code-php) +* :egypt: **Arabic:** + * [ahmedjoda/clean-code-php](https://github.com/ahmedjoda/clean-code-php) **[⬆ back to top](#table-of-contents)** From cb43aa240d77418f55ece3efe9d5004d34bd4d26 Mon Sep 17 00:00:00 2001 From: Hayato Ando <0du38604360418k@gmail.com> Date: Fri, 11 Feb 2022 18:14:03 +0900 Subject: [PATCH 17/21] add japanese translation --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 2708e171..e72604e1 100644 --- a/README.md +++ b/README.md @@ -2203,5 +2203,7 @@ This is also available in other languages: * [nayeemdev/clean-code-php](https://github.com/nayeemdev/clean-code-php) * :egypt: **Arabic:** * [ahmedjoda/clean-code-php](https://github.com/ahmedjoda/clean-code-php) +* :jp: **Japanese:** + * [hayato07/clean-code-php](https://github.com/hayato07/clean-code-php) **[⬆ back to top](#table-of-contents)** From 8595ff59e6ade90bb1b5457663db9c6ccce3c6d3 Mon Sep 17 00:00:00 2001 From: Igor Popov <igrppv@gmail.com> Date: Thu, 19 May 2022 01:13:51 +0300 Subject: [PATCH 18/21] change type hint url --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e72604e1..7993ede2 100644 --- a/README.md +++ b/README.md @@ -454,7 +454,7 @@ function createMicrobrewery($name = null): void **Good:** - 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`. + You can use [type hinting](https://www.php.net/manual/en/language.types.declarations.php) and be sure that the `$breweryName` will not be `NULL`. ```php function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void From fe8d53bdef70926f0852bb05f4642d38b3fea854 Mon Sep 17 00:00:00 2001 From: Igor Popov <igrppv@gmail.com> Date: Thu, 19 May 2022 11:30:44 +0300 Subject: [PATCH 19/21] change type hinting url --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7993ede2..17756a9c 100644 --- a/README.md +++ b/README.md @@ -1103,7 +1103,7 @@ function travelToTexas(Vehicle $vehicle): void If you are working with basic primitive values like strings, integers, and arrays, and you use PHP 7+ and you can't use polymorphism but you still feel the need to type-check, you should consider -[type declaration](http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration) +[type declaration](https://www.php.net/manual/en/language.types.declarations.php) or strict mode. It provides you with static typing on top of standard PHP syntax. The problem with manually type-checking is that doing it will require so much extra verbiage that the faux "type-safety" you get doesn't make up for the lost From ca4ab1ff526e088ef2339ae1995f65875c2b3445 Mon Sep 17 00:00:00 2001 From: Ahmed Almory <40957053+ahmedalmory@users.noreply.github.com> Date: Thu, 16 Jun 2022 10:59:34 +0200 Subject: [PATCH 20/21] update Arabic translation username --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 17756a9c..df4a4c4c 100644 --- a/README.md +++ b/README.md @@ -2202,7 +2202,7 @@ This is also available in other languages: * :bangladesh: **Bangla:** * [nayeemdev/clean-code-php](https://github.com/nayeemdev/clean-code-php) * :egypt: **Arabic:** - * [ahmedjoda/clean-code-php](https://github.com/ahmedjoda/clean-code-php) + * [ahmedalmory/clean-code-php](https://github.com/ahmedalmory/clean-code-php) * :jp: **Japanese:** * [hayato07/clean-code-php](https://github.com/hayato07/clean-code-php) From eedf6c68bdf7990db6636b4c7489d0cef9fe93b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20Sz=C3=A9pe?= <viktor@szepe.net> Date: Sun, 19 Jun 2022 20:21:22 +0000 Subject: [PATCH 21/21] Fix wording in Comparison Fixes #179 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index df4a4c4c..b5b80b0e 100644 --- a/README.md +++ b/README.md @@ -371,7 +371,7 @@ class Car **Not good:** -The simple comparison will convert the string in an integer. +The simple comparison will convert the string into an integer. ```php $a = '42';