1
0
mirror of https://github.com/jupeter/clean-code-php.git synced 2025-09-25 21:49:04 +02:00

Merge pull request #13 from jupeter/master

Update master
This commit is contained in:
Peter Gribanov
2017-09-15 12:10:22 +03:00
committed by GitHub

247
README.md
View File

@@ -162,7 +162,7 @@ $address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/';
preg_match($cityZipCodeRegex, $address, $matches);
list(, $city, $zipCode) = $matches;
[, $city, $zipCode] = $matches;
saveCityZipCode($city, $zipCode);
```
@@ -188,7 +188,7 @@ than implicit.
**Bad:**
```php
function isShopOpen($day)
function isShopOpen($day): bool
{
if ($day) {
if (is_string($day)) {
@@ -214,9 +214,9 @@ function isShopOpen($day)
**Good:**
```php
function isShopOpen($day)
function isShopOpen(string $day): bool
{
if (empty($day) && ! is_string($day)) {
if (empty($day)) {
return false;
}
@@ -231,7 +231,7 @@ function isShopOpen($day)
**Bad:**
```php
function fibonacci($n)
function fibonacci(int $n)
{
if ($n < 50) {
if ($n !== 0) {
@@ -252,7 +252,7 @@ function fibonacci($n)
**Good:**
```php
function fibonacci($n)
function fibonacci(int $n): int
{
if ($n === 0) {
return 0;
@@ -263,7 +263,7 @@ function fibonacci($n)
}
if ($n > 50) {
return 'Not supported';
throw new \Exception('Not supported');
}
return fibonacci($n - 1) + fibonacci($n - 2);
@@ -351,7 +351,7 @@ class Car
This is not good because `$breweryName` can be `NULL`.
```php
function createMicrobrewery($breweryName = 'Hipster Brew Co.')
function createMicrobrewery($breweryName = 'Hipster Brew Co.'): void
{
   // ...
}
@@ -362,7 +362,7 @@ function createMicrobrewery($breweryName = 'Hipster Brew Co.')
This opinion is more understandable than the previous version, but it better controls the value of the variable.
```php
function createMicrobrewery($name = null)
function createMicrobrewery($name = null): void
{
   $breweryName = $name ?: 'Hipster Brew Co.';
// ...
@@ -374,7 +374,7 @@ function createMicrobrewery($name = 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`.
```php
function createMicrobrewery(string $breweryName = 'Hipster Brew Co.')
function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void
{
   // ...
}
@@ -398,7 +398,7 @@ of the time a higher-level object will suffice as an argument.
**Bad:**
```php
function createMenu($title, $body, $buttonText, $cancellable)
function createMenu(string $title, string $body, string $buttonText, bool $cancellable): void
{
// ...
}
@@ -421,7 +421,7 @@ $config->body = 'Bar';
$config->buttonText = 'Baz';
$config->cancellable = true;
function createMenu(MenuConfig $config)
function createMenu(MenuConfig $config): void
{
// ...
}
@@ -439,7 +439,7 @@ of many developers.
**Bad:**
```php
function emailClients($clients)
function emailClients(array $clients): void
{
foreach ($clients as $client) {
$clientRecord = $db->find($client);
@@ -453,18 +453,18 @@ function emailClients($clients)
**Good:**
```php
function emailClients($clients)
function emailClients(array $clients): void
{
$activeClients = activeClients($clients);
array_walk($activeClients, 'email');
}
function activeClients($clients)
function activeClients(array $clients): array
{
return array_filter($clients, 'isClientActive');
}
function isClientActive($client)
function isClientActive(int $client): bool
{
$clientRecord = $db->find($client);
@@ -483,7 +483,7 @@ class Email
{
//...
public function handle()
public function handle(): void
{
mail($this->to, $this->subject, $this->body);
}
@@ -501,7 +501,7 @@ class Email
{
//...
public function send()
public function send(): void
{
mail($this->to, $this->subject, $this->body);
}
@@ -523,7 +523,7 @@ testing.
**Bad:**
```php
function parseBetterJSAlternative($code)
function parseBetterJSAlternative(string $code): void
{
$regexes = [
// ...
@@ -553,7 +553,7 @@ function parseBetterJSAlternative($code)
We have carried out some of the functionality, but the `parseBetterJSAlternative()` function is still very complex and not testable.
```php
function tokenize($code)
function tokenize(string $code): array
{
$regexes = [
// ...
@@ -570,7 +570,7 @@ function tokenize($code)
return $tokens;
}
function lexer($tokens)
function lexer(array $tokens): array
{
$ast = [];
foreach ($tokens as $token) {
@@ -580,7 +580,7 @@ function lexer($tokens)
return $ast;
}
function parseBetterJSAlternative($code)
function parseBetterJSAlternative(string $code): void
{
$tokens = tokenize($code);
$ast = lexer($tokens);
@@ -597,7 +597,7 @@ The best solution is move out the dependencies of `parseBetterJSAlternative()` f
```php
class Tokenizer
{
public function tokenize($code)
public function tokenize(string $code): array
{
$regexes = [
// ...
@@ -617,7 +617,7 @@ class Tokenizer
class Lexer
{
public function lexify($tokens)
public function lexify(array $tokens): array
{
$ast = [];
foreach ($tokens as $token) {
@@ -639,7 +639,7 @@ class BetterJSAlternative
$this->lexer = $lexer;
}
public function parse($code)
public function parse(string $code): void
{
$tokens = $this->tokenizer->tokenize($code);
$ast = $this->lexer->lexify($tokens);
@@ -661,7 +661,7 @@ based on a boolean.
**Bad:**
```php
function createFile($name, $temp = false)
function createFile(string $name, bool $temp = false): void
{
if ($temp) {
touch('./temp/'.$name);
@@ -674,12 +674,12 @@ function createFile($name, $temp = false)
**Good:**
```php
function createFile($name)
function createFile(string $name): void
{
touch($name);
}
function createTempFile($name)
function createTempFile(string $name): void
{
touch('./temp/'.$name);
}
@@ -710,7 +710,7 @@ than the vast majority of other programmers.
// If we had another function that used this name, now it'd be an array and it could break it.
$name = 'Ryan McDermott';
function splitIntoFirstAndLastName()
function splitIntoFirstAndLastName(): void
{
global $name;
@@ -725,7 +725,7 @@ var_dump($name); // ['Ryan', 'McDermott'];
**Good:**
```php
function splitIntoFirstAndLastName($name)
function splitIntoFirstAndLastName(string $name): array
{
return explode(' ', $name);
}
@@ -750,7 +750,7 @@ that tried to do the same thing.
**Bad:**
```php
function config()
function config(): array
{
return [
'foo' => 'bar',
@@ -770,7 +770,7 @@ class Configuration
$this->configuration = $configuration;
}
public function get($key)
public function get(string $key): ?string
{
return isset($this->configuration[$key]) ? $this->configuration[$key] : null;
}
@@ -806,12 +806,12 @@ class DBConnection
{
private static $instance;
private function __construct($dsn)
private function __construct(string $dsn)
{
// ...
}
public static function getInstance()
public static function getInstance(): DBConnection
{
if (self::$instance === null) {
self::$instance = new self();
@@ -831,7 +831,7 @@ $singleton = DBConnection::getInstance();
```php
class DBConnection
{
public function __construct(array $dsn)
public function __construct(string $dsn)
{
// ...
}
@@ -875,7 +875,7 @@ if ($article->isPublished()) {
**Bad:**
```php
function isDOMNodeNotPresent($node)
function isDOMNodeNotPresent(\DOMNode $node): bool
{
// ...
}
@@ -889,7 +889,7 @@ if (!isDOMNodeNotPresent($node))
**Good:**
```php
function isDOMNodePresent($node)
function isDOMNodePresent(\DOMNode $node): bool
{
// ...
}
@@ -919,7 +919,7 @@ class Airplane
{
// ...
public function getCruisingAltitude()
public function getCruisingAltitude(): int
{
switch ($this->type) {
case '777':
@@ -940,14 +940,14 @@ interface Airplane
{
// ...
public function getCruisingAltitude();
public function getCruisingAltitude(): int;
}
class Boeing777 implements Airplane
{
// ...
public function getCruisingAltitude()
public function getCruisingAltitude(): int
{
return $this->getMaxAltitude() - $this->getPassengerCount();
}
@@ -957,7 +957,7 @@ class AirForceOne implements Airplane
{
// ...
public function getCruisingAltitude()
public function getCruisingAltitude(): int
{
return $this->getMaxAltitude();
}
@@ -967,7 +967,7 @@ class Cessna implements Airplane
{
// ...
public function getCruisingAltitude()
public function getCruisingAltitude(): int
{
return $this->getMaxAltitude() - $this->getFuelExpenditure();
}
@@ -986,7 +986,7 @@ The first thing to consider is consistent APIs.
**Bad:**
```php
function travelToTexas($vehicle)
function travelToTexas($vehicle): void
{
if ($vehicle instanceof Bicycle) {
$vehicle->peddleTo(new Location('texas'));
@@ -999,7 +999,7 @@ function travelToTexas($vehicle)
**Good:**
```php
function travelToTexas(Traveler $vehicle)
function travelToTexas(Traveler $vehicle): void
{
$vehicle->travelTo(new Location('texas'));
}
@@ -1022,7 +1022,7 @@ Otherwise, do all of that but with PHP strict type declaration or strict mode.
**Bad:**
```php
function combine($val1, $val2)
function combine($val1, $val2): int
{
if (!is_numeric($val1) || !is_numeric($val2)) {
throw new \Exception('Must be of type Number');
@@ -1035,7 +1035,7 @@ function combine($val1, $val2)
**Good:**
```php
function combine(int $val1, int $val2)
function combine(int $val1, int $val2): int
{
return $val1 + $val2;
}
@@ -1052,12 +1052,12 @@ in your version history if you still need it.
**Bad:**
```php
function oldRequestModule($url)
function oldRequestModule(string $url): void
{
// ...
}
function newRequestModule($url)
function newRequestModule(string $url): void
{
// ...
}
@@ -1069,7 +1069,7 @@ inventoryTracker('apples', $request, 'www.inventory-awesome.io');
**Good:**
```php
function requestModule($url)
function requestModule(string $url): void
{
// ...
}
@@ -1120,12 +1120,12 @@ class BankAccount
{
private $balance;
public function __construct($balance = 1000)
public function __construct(int $balance = 1000)
{
$this->balance = $balance;
}
public function withdrawBalance($amount)
public function withdrawBalance(int $amount): void
{
if ($amount > $this->balance) {
throw new \Exception('Amount greater than available balance.');
@@ -1134,12 +1134,12 @@ class BankAccount
$this->balance -= $amount;
}
public function depositBalance($amount)
public function depositBalance(int $amount): void
{
$this->balance += $amount;
}
public function getBalance()
public function getBalance(): int
{
return $this->balance;
}
@@ -1165,7 +1165,7 @@ class Employee
{
public $name;
public function __construct($name)
public function __construct(string $name)
{
$this->name = $name;
}
@@ -1182,12 +1182,12 @@ class Employee
{
private $name;
public function __construct($name)
public function __construct(string $name)
{
$this->name = $name;
}
public function getName()
public function getName(): string
{
return $this->name;
}
@@ -1228,7 +1228,7 @@ class Employee
private $name;
private $email;
public function __construct($name, $email)
public function __construct(string $name, string $email)
{
$this->name = $name;
$this->email = $email;
@@ -1245,7 +1245,7 @@ class EmployeeTaxData extends Employee
private $ssn;
private $salary;
public function __construct($name, $email, $ssn, $salary)
public function __construct(string $name, string $email, string $ssn, string $salary)
{
parent::__construct($name, $email);
@@ -1265,7 +1265,7 @@ class EmployeeTaxData
private $ssn;
private $salary;
public function __construct($ssn, $salary)
public function __construct(string $ssn, string $salary)
{
$this->ssn = $ssn;
$this->salary = $salary;
@@ -1280,13 +1280,13 @@ class Employee
private $email;
private $taxData;
public function __construct($name, $email)
public function __construct(string $name, string $email)
{
$this->name = $name;
$this->email = $email;
}
public function setTaxData($ssn, $salary)
public function setTaxData(string $ssn, string $salary)
{
$this->taxData = new EmployeeTaxData($ssn, $salary);
}
@@ -1298,6 +1298,7 @@ class Employee
**[⬆ back to top](#table-of-contents)**
### Avoid fluent interfaces
A [Fluent interface](https://en.wikipedia.org/wiki/Fluent_interface) is an object
oriented API that aims to improve the readability of the source code by using
[Method chaining](https://en.wikipedia.org/wiki/Method_chaining).
@@ -1316,6 +1317,7 @@ For more informations you can read the full [blog post](https://ocramius.github.
on this topic written by [Marco Pivetta](https://github.com/Ocramius).
**Bad:**
```php
class Car
{
@@ -1323,7 +1325,7 @@ class Car
private $model = 'Accord';
private $color = 'white';
public function setMake($make)
public function setMake(string $make): self
{
$this->make = $make;
@@ -1331,7 +1333,7 @@ class Car
return $this;
}
public function setModel($model)
public function setModel(string $model): self
{
$this->model = $model;
@@ -1339,7 +1341,7 @@ class Car
return $this;
}
public function setColor($color)
public function setColor(string $color): self
{
$this->color = $color;
@@ -1347,7 +1349,7 @@ class Car
return $this;
}
public function dump()
public function dump(): void
{
var_dump($this->make, $this->model, $this->color);
}
@@ -1361,6 +1363,7 @@ $car = (new Car())
```
**Good:**
```php
class Car
{
@@ -1368,22 +1371,22 @@ class Car
private $model = 'Accord';
private $color = 'white';
public function setMake($make)
public function setMake(string $make): void
{
$this->make = $make;
}
public function setModel($model)
public function setModel(string $model): void
{
$this->model = $model;
}
public function setColor($color)
public function setColor(string $color): void
{
$this->color = $color;
}
public function dump()
public function dump(): void
{
var_dump($this->make, $this->model, $this->color);
}
@@ -1426,19 +1429,19 @@ class UserSettings
{
private $user;
public function __construct($user)
public function __construct(User $user)
{
$this->user = $user;
}
public function changeSettings($settings)
public function changeSettings(array $settings): void
{
if ($this->verifyCredentials()) {
// ...
}
}
private function verifyCredentials()
private function verifyCredentials(): bool
{
// ...
}
@@ -1452,12 +1455,12 @@ class UserAuth
{
private $user;
public function __construct($user)
public function __construct(User $user)
{
$this->user = $user;
}
public function verifyCredentials()
public function verifyCredentials(): bool
{
// ...
}
@@ -1468,13 +1471,13 @@ class UserSettings
private $user;
private $auth;
public function __construct($user)
public function __construct(User $user)
{
$this->user = $user;
$this->auth = new UserAuth($user);
}
public function changeSettings($settings)
public function changeSettings(array $settings): void
{
if ($this->auth->verifyCredentials()) {
// ...
@@ -1499,7 +1502,7 @@ abstract class Adapter
{
protected $name;
public function getName()
public function getName(): string
{
return $this->name;
}
@@ -1529,12 +1532,12 @@ class HttpRequester
{
private $adapter;
public function __construct($adapter)
public function __construct(Adapter $adapter)
{
$this->adapter = $adapter;
}
public function fetch($url)
public function fetch(string $url): Promise
{
$adapterName = $this->adapter->getName();
@@ -1545,12 +1548,12 @@ class HttpRequester
}
}
private function makeAjaxCall($url)
private function makeAjaxCall(string $url): Promise
{
// request and return promise
}
private function makeHttpCall($url)
private function makeHttpCall(string $url): Promise
{
// request and return promise
}
@@ -1562,12 +1565,12 @@ class HttpRequester
```php
interface Adapter
{
public function request($url);
public function request(string $url): Promise;
}
class AjaxAdapter implements Adapter
{
public function request($url)
public function request(string $url): Promise
{
// request and return promise
}
@@ -1575,7 +1578,7 @@ class AjaxAdapter implements Adapter
class NodeAdapter implements Adapter
{
public function request($url)
public function request(string $url): Promise
{
// request and return promise
}
@@ -1590,7 +1593,7 @@ class HttpRequester
$this->adapter = $adapter;
}
public function fetch($url)
public function fetch(string $url): Promise
{
return $this->adapter->request($url);
}
@@ -1622,22 +1625,22 @@ class Rectangle
protected $width = 0;
protected $height = 0;
public function render($area)
public function render(int $area): void
{
// ...
}
public function setWidth($width)
public function setWidth(int $width): void
{
$this->width = $width;
}
public function setHeight($height)
public function setHeight(int $height): void
{
$this->height = $height;
}
public function getArea()
public function getArea(): int
{
return $this->width * $this->height;
}
@@ -1645,18 +1648,18 @@ class Rectangle
class Square extends Rectangle
{
public function setWidth($width)
public function setWidth(int $width): void
{
$this->width = $this->height = $width;
}
public function setHeight(height)
public function setHeight(int $height): void
{
$this->width = $this->height = $height;
}
}
function renderLargeRectangles($rectangles)
function renderLargeRectangles(Rectangle $rectangles): void
{
foreach ($rectangles as $rectangle) {
$rectangle->setWidth(4);
@@ -1678,9 +1681,9 @@ abstract class Shape
protected $width = 0;
protected $height = 0;
abstract public function getArea();
abstract public function getArea(): int;
public function render($area)
public function render(int $area): void
{
// ...
}
@@ -1688,17 +1691,17 @@ abstract class Shape
class Rectangle extends Shape
{
public function setWidth($width)
public function setWidth(int $width): void
{
$this->width = $width;
}
public function setHeight($height)
public function setHeight(int $height): void
{
$this->height = $height;
}
public function getArea()
public function getArea(): int
{
return $this->width * $this->height;
}
@@ -1708,18 +1711,18 @@ class Square extends Shape
{
private $length = 0;
public function setLength($length)
public function setLength(int $length): void
{
$this->length = $length;
}
public function getArea()
public function getArea(): int
{
return pow($this->length, 2);
}
}
function renderLargeRectangles($rectangles)
function renderLargeRectangles(Shape $rectangles): void
{
foreach ($rectangles as $rectangle) {
if ($rectangle instanceof Square) {
@@ -1755,19 +1758,19 @@ all of the settings. Making them optional helps prevent having a "fat interface"
```php
interface Employee
{
public function work();
public function work(): void;
public function eat();
public function eat(): void;
}
class Human implements Employee
{
public function work()
public function work(): void
{
// ....working
}
public function eat()
public function eat(): void
{
// ...... eating in lunch break
}
@@ -1775,12 +1778,12 @@ class Human implements Employee
class Robot implements Employee
{
public function work()
public function work(): void
{
//.... working much more
}
public function eat()
public function eat(): void
{
//.... robot can't eat, but it must implement this method
}
@@ -1794,12 +1797,12 @@ Not every worker is an employee, but every employee is an worker.
```php
interface Workable
{
public function work();
public function work(): void;
}
interface Feedable
{
public function eat();
public function eat(): void;
}
interface Employee extends Feedable, Workable
@@ -1808,12 +1811,12 @@ interface Employee extends Feedable, Workable
class Human implements Employee
{
public function work()
public function work(): void
{
// ....working
}
public function eat()
public function eat(): void
{
//.... eating in lunch break
}
@@ -1822,7 +1825,7 @@ class Human implements Employee
// robot can only work
class Robot implements Workable
{
public function work()
public function work(): void
{
// ....working
}
@@ -1851,7 +1854,7 @@ it makes your code hard to refactor.
```php
class Employee
{
public function work()
public function work(): void
{
// ....working
}
@@ -1859,7 +1862,7 @@ class Employee
class Robot extends Employee
{
public function work()
public function work(): void
{
//.... working much more
}
@@ -1874,7 +1877,7 @@ class Manager
$this->employee = $employee;
}
public function manage()
public function manage(): void
{
$this->employee->work();
}
@@ -1886,12 +1889,12 @@ class Manager
```php
interface Employee
{
public function work();
public function work(): void;
}
class Human implements Employee
{
public function work()
public function work(): void
{
// ....working
}
@@ -1899,7 +1902,7 @@ class Human implements Employee
class Robot implements Employee
{
public function work()
public function work(): void
{
//.... working much more
}
@@ -1914,7 +1917,7 @@ class Manager
$this->employee = $employee;
}
public function manage()
public function manage(): void
{
$this->employee->work();
}
@@ -1951,7 +1954,7 @@ updating multiple places anytime you want to change one thing.
**Bad:**
```php
function showDeveloperList($developers)
function showDeveloperList(array $developers): void
{
foreach ($developers as $developer) {
$expectedSalary = $developer->calculateExpectedSalary();
@@ -1967,7 +1970,7 @@ function showDeveloperList($developers)
}
}
function showManagerList($managers)
function showManagerList(array $managers): void
{
foreach ($managers as $manager) {
$expectedSalary = $manager->calculateExpectedSalary();
@@ -1987,7 +1990,7 @@ function showManagerList($managers)
**Good:**
```php
function showList($employees)
function showList(array $employees): void
{
foreach ($employees as $employee) {
$expectedSalary = $employee->calculateExpectedSalary();
@@ -2009,7 +2012,7 @@ function showList($employees)
It is better to use a compact version of the code.
```php
function showList($employees)
function showList(array $employees): void
{
foreach ($employees as $employee) {
render([