diff --git a/README.md b/README.md index 3ee84876..38ff728f 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) @@ -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*(?.+?)\s*(?\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; @@ -401,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) @@ -448,8 +374,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 +390,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 +409,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 +420,6 @@ if (isset($_GET['name'])) { **Good:** ```php -declare(strict_types=1); - $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ``` @@ -509,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 @@ -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 information you can read the [blog post](http://fabien.potencier.org/pr **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, ]); };