以下はClean Code PHPの日本語訳です。

Introduction

Robert C. Martinの著書Clean Codeは、PHPにも当てはまることばかりです。
これはスタイルガイドではありません。
PHPで3R(Readable、Reusable、Refactorable)なコードを推進するためのガイドです。

ここに書いてあることの全てに従わねばならないわけではなく、普遍的に合意されているわけでもありません。
ただのガイドラインであり、それ以上のものではありません。
しかしこれらは、Clean Codeの著者らが長年の集合知の結果をまとめたものです。

このガイドはclean-code-javascriptに影響されました。

多くの開発者は未だにPHP5を使っていますが、このガイドはPHP7.1以上を前提としています。

Variables

Use meaningful and pronounceable variable names

意味があって、発音可能な名前を使いましょう。

Bad

	$ymdstr = $moment->format('y-m-d');

Good

	$currentDate = $moment->format('y-m-d');

Use the same vocabulary for the same type of variable

同じような内容には同じ名前を使いましょう。

Bad

	getUserInfo();
	getUserData();
	getUserRecord();
	getUserProfile();

Good

	getUser();

Use searchable names (part 1)

コードは書く量より読む量のほうが多いです。
従ってコードは読みやすく、検索しやすく書いていくことが重要です。
意味のある名前を使用しないことによって、コードを解読する側は不必要な努力を強いられます。

Bad

	// 448って何？
	$result = $serializer->serialize($data, 448);

Good

	$json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);

Use searchable names (part 2)

Bad

	// 4って何？
	if ($user->access & 4) {
	    // ...
	}

Good

	class User
	{
	    const ACCESS_READ = 1;
	    const ACCESS_CREATE = 2;
	    const ACCESS_UPDATE = 4;
	    const ACCESS_DELETE = 8;
	}

	if ($user->access & User::ACCESS_UPDATE) {
	    // 正直ビット演算も微妙じゃね？
	}

Use explanatory variables

変数は中身を表す名前にする。

Bad

	$address = 'One Infinite Loop, Cupertino 95014';
	$cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/';
	preg_match($cityZipCodeRegex, $address, $matches);

	saveCityZipCode($matches[1], $matches[2]);

名前を付けましたが、依然として正規表現の結果に依存しています。

Not_Bad

	$address = 'One Infinite Loop, Cupertino 95014';
	$cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/';
	preg_match($cityZipCodeRegex, $address, $matches);

	[, $city, $zipCode] = $matches;
	saveCityZipCode($city, $zipCode);

サブパターンに名前を付けて、正規表現への依存度を減らします。

Good

	$address = 'One Infinite Loop, Cupertino 95014';
	$cityZipCodeRegex = '/^[^,]+,\s*(?<city>.+?)\s*(?<zipCode>\d{5})$/';
	preg_match($cityZipCodeRegex, $address, $matches);

	saveCityZipCode($matches['city'], $matches['zipCode']);

Avoid nesting too deeply and return early (part 1)

if/elseが多すぎると理解しづらくなります。
ガード節を使いましょう。

Bad

	function isShopOpen($day): bool
	{
	    if ($day) {
	        if (is_string($day)) {
	            $day = strtolower($day);
	            if ($day === 'friday') {
	                return true;
	            } elseif ($day === 'saturday') {
	                return true;
	            } elseif ($day === 'sunday') {
	                return true;
	            } else {
	                return false;
	            }
	        } else {
	            return false;
	        }
	    } else {
	        return false;
	    }
	}

Good

	function isShopOpen(string $day): bool
	{
	    if (empty($day)) {
	        return false;
	    }

	    $openingDays = [
	        'friday', 'saturday', 'sunday'
	    ];

	    return in_array(strtolower($day), $openingDays, true);
	}

Avoid nesting too deeply and return early (part 2)

Bad

	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;
	        }
	    } else {
	        return 'Not supported';
	    }
	}

Good

	function fibonacci(int $n): int
	{
	    if ($n === 0 || $n === 1) {
	        return $n;
	    }

	    if ($n > 50) {
	        throw new \Exception('Not supported');
	    }

	    return fibonacci($n - 1) + fibonacci($n - 2);
	}

Avoid Mental Mapping

仮名より、ちゃんとした名前を付けた方がいい。

Bad

	$l = ['Austin', 'New York', 'San Francisco'];

	for ($i = 0; $i < count($l); $i++) {
	    $li = $l[$i];
	    doStuff();
	    doSomeOtherStuff();
	    // ...
	    // ...
	    // ...
	    // あれ、liって何だったっけ？
	    dispatch($li);
	}

Good

	$locations = ['Austin', 'New York', 'San Francisco'];

	foreach ($locations as $location) {
	    doStuff();
	    doSomeOtherStuff();
	    // ...
	    // ...
	    // ...
	    dispatch($location);
	}

Don't add unneeded context

クラスやオブジェクト名に既に意味がついているなら、変数名で同じことを繰り返す必要はありません。

Bad

	class Car
	{
	    public $carMake;
	    public $carModel;
	    public $carColor;

	    //...
	}

Good

	class Car
	{
	    public $make;
	    public $model;
	    public $color;

	    //...
	}

Use default arguments instead of short circuiting or conditionals

短絡や条件式を使う代わりにデフォルト引数を使いましょう。

Not_Good

	// 引数にNULLを入れられるのでよくない
	function createMicrobrewery($breweryName = 'Hipster Brew Co.'): void
	{
	    // ...
	}

Not_Bad

	// 悪くはないが、より適切に制御できる
	function createMicrobrewery($name = null): void
	{
	    $breweryName = $name ?: 'Hipster Brew Co.';
	    // ...
	}

Good

	// PHP7でプリミティブ型宣言が導入された
	function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void
	{
	    // ...
	}

Functions

Function arguments (2 or fewer ideally)

関数の引数を制限することは、関数のテストが容易になるため重要です。
引数が3個以上になると各変数を様々なケースでテストすることが必要になり、組合せ爆発に直結します。

引数は0個が理想です。
1個か2個は問題ありません。
3個は可能な限り避ける必要があります。
それ以上が必要になるのであれば引数をまとめましょう。
引数が2個より多く必要ならば、その関数の行う仕事は多すぎます。
そうではないというのであれば、引数としてオブジェクトを渡すようにしましょう。

Bad

	function createMenu(string $title, string $body, string $buttonText, bool $cancellable): void
	{
	    // ...
	}

Good

	class MenuConfig
	{
	    public $title;
	    public $body;
	    public $buttonText;
	    public $cancellable = false;
	}

	$config = new MenuConfig();
	$config->title = 'Foo';
	$config->body = 'Bar';
	$config->buttonText = 'Baz';
	$config->cancellable = true;

	function createMenu(MenuConfig $config): void
	{
	    // ...
	}

Functions should do one thing

ひとつの関数はひとつの処理だけを行うこと。
これはソフトウェア工学の最も重要なルールです。
関数に複数の処理を書いてしまうと、関数の作成、テスト、そして処理内容の推測が難しくなります。
1関数は1アクションだけに定義してしまえば、リファクタリングも簡単になり、コードはより洗練されたものになります。
このガイドの他の部分は全く守らなかったとしても、ここにだけ気をつけていればよりよい開発者になれることでしょう。

Bad

	function emailClients(array $clients): void
	{
	    foreach ($clients as $client) {
	        $clientRecord = $db->find($client);
	        if ($clientRecord->isActive()) {
	            email($client);
	        }
	    }
	}

Good

	function emailClients(array $clients): void
	{
	    $activeClients = activeClients($clients);
	    array_walk($activeClients, 'email');
	}

	function activeClients(array $clients): array
	{
	    return array_filter($clients, 'isClientActive');
	}

	function isClientActive(int $client): bool
	{
	    $clientRecord = $db->find($client);

	    return $clientRecord->isActive();
	}

Function names should say what they do

関数名は、それが何をしているのかをつけましょう。

Bad

	class Email
	{
	    //...

	    public function handle(): void
	    {
	        mail($this->to, $this->subject, $this->body);
	    }
	}

	$message = new Email(...);
	// handleってなに？なんかファイルに書き込んでるの？
	$message->handle();

Good

	class Email 
	{
	    //...

	    public function send(): void
	    {
	        mail($this->to, $this->subject, $this->body);
	    }
	}

	$message = new Email(...);
	// 目的が一目瞭然
	$message->send();

Functions should only be one level of abstraction

1関数内にデータの抽象化が複数ある場合、その関数は仕事をしすぎています。
機能を分割することで、再利用性やテストのしやすさが高まります。

Bad

	function parseBetterJSAlternative(string $code): void
	{
	    $regexes = [
	        // ...
	    ];

	    $statements = explode(' ', $code);
	    $tokens = [];
	    foreach ($regexes as $regex) {
	        foreach ($statements as $statement) {
	            // ...
	        }
	    }

	    $ast = [];
	    foreach ($tokens as $token) {
	        // lex...
	    }

	    foreach ($ast as $node) {
	        // parse...
	    }
	}

多少リファクタリングされたものの、parseBetterJSAlternative()は依然複雑すぎます。

Bad_too

	function tokenize(string $code): array
	{
	    $regexes = [
	        // ...
	    ];

	    $statements = explode(' ', $code);
	    $tokens = [];
	    foreach ($regexes as $regex) {
	        foreach ($statements as $statement) {
	            $tokens[] = /* ... */;
	        }
	    }

	    return $tokens;
	}

	function lexer(array $tokens): array
	{
	    $ast = [];
	    foreach ($tokens as $token) {
	        $ast[] = /* ... */;
	    }

	    return $ast;
	}

	function parseBetterJSAlternative(string $code): void
	{
	    $tokens = tokenize($code);
	    $ast = lexer($tokens);
	    foreach ($ast as $node) {
	        // parse...
	    }
	}

依存を取り除くのが最適解です。

Good

	class Tokenizer
	{
	    public function tokenize(string $code): array
	    {
	        $regexes = [
	            // ...
	        ];

	        $statements = explode(' ', $code);
	        $tokens = [];
	        foreach ($regexes as $regex) {
	            foreach ($statements as $statement) {
	                $tokens[] = /* ... */;
	            }
	        }

	        return $tokens;
	    }
	}

	class Lexer
	{
	    public function lexify(array $tokens): array
	    {
	        $ast = [];
	        foreach ($tokens as $token) {
	            $ast[] = /* ... */;
	        }

	        return $ast;
	    }
	}

	class BetterJSAlternative
	{
	    private $tokenizer;
	    private $lexer;

	    public function __construct(Tokenizer $tokenizer, Lexer $lexer)
	    {
	        $this->tokenizer = $tokenizer;
	        $this->lexer = $lexer;
	    }

	    public function parse(string $code): void
	    {
	        $tokens = $this->tokenizer->tokenize($code);
	        $ast = $this->lexer->lexify($tokens);
	        foreach ($ast as $node) {
	            // parse...
	        }
	    }
	}

Don't use flags as function parameters

引数にフラグ値を使わない。
フラグを使うということは、すなわちその関数が複数の処理を行うことを示しています。
関数はひとつのことを行うべきなので、フラグではなく別々の関数にするべきです。

Bad

	function createFile(string $name, bool $temp = false): void
	{
	    if ($temp) {
	        touch('./temp/'.$name);
	    } else {
	        touch($name);
	    }
	}

Good

	function createFile(string $name): void
	{
	    touch($name);
	}

	function createTempFile(string $name): void
	{
	    touch('./temp/'.$name);
	}

Avoid Side Effects

関数が、値を受け取って加工して返す以外のことを行う場合、その関数には副作用があります。
副作用はたとえばファイルへの書き込み、グローバル変数の変更、あるいは全財産を誰かにばらまくこと等です。

どうしても副作用のある関数を書かなければならないこともあるでしょう。
ファイルに何かを書き込む必要があるかもしれません。
そういうときに行わなければならないことは、副作用を起こす処理はひとまとめにすることです。
あるファイルに書き込む処理を、複数の関数やクラスに分散させてはいけません。
それを行うサービスをひとつだけ作り、そしてそのサービスはその処理だけを行うようにしてください。

オブジェクトを使い回してデータ構造も何もなくなってしまう、何処でも書き換えられるミュータブルなデータを使う、副作用のある処理を何カ所にも書く、といったよくある落とし穴はできる限り避けましょう。
それだけで他の多くのプログラマーより、きっとより幸せになれます。

Bad

	// グローバル変数を使ってるうえに直接型変換してるので、そのうちきっと事故る
	$name = 'Ryan McDermott';

	function splitIntoFirstAndLastName(): void
	{
	    global $name;

	    $name = explode(' ', $name);
	}

	splitIntoFirstAndLastName();

	var_dump($name); // ['Ryan', 'McDermott'];

Good

	function splitIntoFirstAndLastName(string $name): array
	{
	    return explode(' ', $name);
	}

	$name = 'Ryan McDermott';
	$newName = splitIntoFirstAndLastName($name);

	var_dump($name); // 'Ryan McDermott';
	var_dump($newName); // ['Ryan', 'McDermott'];

Don't write to global functions

グローバル汚染はよくない方法です。
他のライブラリと衝突してエラーになる可能性があり、そのようなライブラリを使い続けるほどユーザは親切ではありません。
具体例として、コンフィグを設定する関数を考えてみましょう。
config()のようなグローバル関数にしてしまうと、同じことを考えた他のライブラリと衝突してしまうでしょう。

Bad

	function config(): array
	{
	    return  [
	        'foo' => 'bar',
	    ]
	}

Good

	class Configuration
	{
	    private $configuration = [];

	    public function __construct(array $configuration)
	    {
	        $this->configuration = $configuration;
	    }

	    public function get(string $key): ?string
	    {
	        return isset($this->configuration[$key]) ? $this->configuration[$key] : null;
	    }
	}
	
	$configuration = new Configuration([
    	'foo' => 'bar',
	]);

アプリではConfigurationのインスタンスを使用します。

Don't use a Singleton pattern

シングルトンはアンチパターンです。

・シングルトンは大抵グローバルインスタンスとして使われます。
これにより、本来インターフェイスによって隠しておくべき依存関係をコードに含んでしまいます。
不要なものをグローバルにしてしまうのはCode smellです。
・ライフサイクルを自分自身で管理することは、単一責任の原則に反します。
・シングルトンによって密結合が発生します。これによってテストがとても困難になります。
・シングルトンはアプリが実行終了するまで状態を維持し続けます。これはユニットテストにとって致命的な問題です。ひとつのテストは、他のテストと完全に独立している必要があるからです。

Misko Heveryによるより根源的な指摘も存在します。

Bad

	// このサンプルコード、Too few argumentsのFatal errorになるんだが
	class DBConnection
	{
	    private static $instance;

	    private function __construct(string $dsn)
	    {
	        // ...
	    }

	    public static function getInstance(): DBConnection
	    {
	        if (self::$instance === null) {
	            self::$instance = new self();
	        }

	        return self::$instance;
	    }

	    // ...
	}

	$singleton = DBConnection::getInstance();

Good

	class DBConnection
	{
	    public function __construct(string $dsn)
	    {
	        // ...
	    }

	     // ...
	}
	
	$connection = new DBConnection($dsn);

DSNを渡してDBConnectionインスタンスを作成し、プログラム中ではそれを使います。

Encapsulate conditionals

条件はカプセル化する。

Bad

	if ($article->state === 'published') {
	    // ...
	}

Good

	if ($article->isPublished()) {
	    // ...
	}

Avoid negative conditionals

否定条件は避ける。

Bad

	function isDOMNodeNotPresent(\DOMNode $node): bool
	{
	    // ...
	}

	if (!isDOMNodeNotPresent($node))
	{
	    // ...
	}

Good

	function isDOMNodePresent(\DOMNode $node): bool
	{
	    // ...
	}

	if (isDOMNodePresent($node)) {
	    // ...
	}

Avoid conditionals

条件分岐を避ける。
これは不可能なことのように思えます。
この話をすると、『if文無しでいったい何ができるんだ？』とまず最初に聞かれます。
答えは、たいていの場合ポリモーフィズムで同じことができる、です。
次に来る質問は、『へえ～すごいですネ！ところで何故それが必要なんだ？』
この答えはクリーンコードのコンセプト、すなわち、ひとつの関数はひとつのことだけをする、です。
if文のあるクラスや関数は、多くの仕事をしすぎています。
何度も言いますが、ひとつのことだけを行ってください。

Bad

	class Airplane
	{
	    // ...

	    public function getCruisingAltitude(): int
	    {
	        switch ($this->type) {
	            case '777':
	                return $this->getMaxAltitude() - $this->getPassengerCount();
	            case 'Air Force One':
	                return $this->getMaxAltitude();
	            case 'Cessna':
	                return $this->getMaxAltitude() - $this->getFuelExpenditure();
	        }
	    }
	}

Good

	interface Airplane
	{
	    // ...

	    public function getCruisingAltitude(): int;
	}

	class Boeing777 implements Airplane
	{
	    // ...

	    public function getCruisingAltitude(): int
	    {
	        return $this->getMaxAltitude() - $this->getPassengerCount();
	    }
	}

	class AirForceOne implements Airplane
	{
	    // ...

	    public function getCruisingAltitude(): int
	    {
	        return $this->getMaxAltitude();
	    }
	}

	class Cessna implements Airplane
	{
	    // ...

	    public function getCruisingAltitude(): int
	    {
	        return $this->getMaxAltitude() - $this->getFuelExpenditure();
	    }
	}

Avoid type-checking (part 1)

PHPには型がありません。
つまり、関数は任意の型の引数を取ることができます。
フリーダムすぎるこの仕様に足を取られないように、どうしても引数の型チェックを行いたくなることがあります。
しかし、それを行うことを避ける方法はいくつもあります。
最初に考えるべきことは、一貫したAPIの仕様です。

Bad

	function travelToTexas($vehicle): void
	{
	    if ($vehicle instanceof Bicycle) {
	        $vehicle->pedalTo(new Location('texas'));
	    } elseif ($vehicle instanceof Car) {
	        $vehicle->driveTo(new Location('texas'));
	    }
	}

Good

	function travelToTexas(Traveler $vehicle): void
	{
	    $vehicle->travelTo(new Location('texas'));
	}

Avoid type-checking (part 2)

引数が文字列や数値のようなプリミティブ型の場合、ポリモーフィズムを使うことはできません。
型チェックを行いたい場合は型宣言を使うことができます。
手作業による型チェックによって得られる型安全性は、コードの追加によって得た読みにくさを補えるほどのものではありません。
PHPのコードは常にクリーンに保ち、テストを書き、コードレビューを行いましょう。
型チェックには厳密な型宣言を使いましょう。

Bad

	function combine($val1, $val2): int
	{
	    if (!is_numeric($val1) || !is_numeric($val2)) {
	        throw new \Exception('Must be of type Number');
	    }

	    return $val1 + $val2;
	}

Good

	function combine(int $val1, int $val2): int
	{
	    return $val1 + $val2;
	}

Remove dead code

デッドコードはコードのコピペと同じくらいの悪です。
このようなものを残しておく必要は一切ありません。
呼び出されていないコードは削除する、鉄則です。
もし後で必要になったとしても、バージョン履歴から拾ってこれます。

Bad

	function oldRequestModule(string $url): void
	{
	    // ...
	}

	function newRequestModule(string $url): void
	{
	    // ...
	}

	$request = newRequestModule($requestUrl);
	inventoryTracker('apples', $request, 'www.inventory-awesome.io');

Good

	function requestModule(string $url): void
	{
	    // ...
	}

	$request = requestModule($requestUrl);
	inventoryTracker('apples', $request, 'www.inventory-awesome.io');

Objects and Data Structures

Use object encapsulation

PHPでは、メソッドとプロパティにpublic/protected/privateの可視性を指定し、変更を制御することができます。
・プロパティを取得する以外のことを行いたい場合に、毎回全てのアクセサを書く必要はありません。
・set時のバリデーションを安全に追加できます。
・プロパティをカプセル化します。
・set/get時のロギング、エラー処理を簡単に追加できます。
・継承することで、デフォルトの処理をオーバーライドできます。
・値をサーバから取得する際などに、プロパティを遅延ロードさせることができます。

これは開放/閉鎖原則の一部でもあります。

Bad

	class BankAccount
	{
	    public $balance = 1000;
	}

	$bankAccount = new BankAccount();

	// Buy shoes...
	$bankAccount->balance -= 100;

Good

	class BankAccount
	{
	    private $balance;

	    public function __construct(int $balance = 1000)
	    {
	      $this->balance = $balance;
	    }

	    public function withdraw(int $amount): void
	    {
	        if ($amount > $this->balance) {
	            throw new \Exception('Amount greater than available balance.');
	        }

	        $this->balance -= $amount;
	    }

	    public function deposit(int $amount): void
	    {
	        $this->balance += $amount;
	    }

	    public function getBalance(): int
	    {
	        return $this->balance;
	    }
	}

	$bankAccount = new BankAccount();

	// Buy shoes...
	$bankAccount->withdraw($shoesPrice);

	// Get balance
	$balance = $bankAccount->getBalance();

Make objects have private/protected members

publicメソッド/プロパティは変更に対して最も脆弱です。
外部から中身を簡単に変更することができてしまい、その変更をクラス内からは制御できないからです。

protectedは子クラスから自由に操作できるため、publicと同じくらい危険です。
publicとprotectedの違いは実質アクセス方法だけで、カプセル化に対しての保証は同じでしかありません。
private修正子は、コードがそのひとつのクラスだけで安全ではないことになります。
コードの変更が他に波及するジェンガ効果は起こらないでしょう。

従ってデフォルトではprivateを使い、外部に公開するメソッド/プロパティのみをpublic/protectedとするべきです。
これ以上についてはFabien Potencierによって書かれたブログポストを読むとよいでしょう。

Bad

	class Employee
	{
	    public $name;

	    public function __construct(string $name)
	    {
	        $this->name = $name;
	    }
	}

	$employee = new Employee('John Doe');
	echo 'Employee name: '.$employee->name; // Employee name: John Doe

Good

	class Employee
	{
	    private $name;

	    public function __construct(string $name)
	    {
	        $this->name = $name;
	    }

	    public function getName(): string
	    {
	        return $this->name;
	    }
	}

	$employee = new Employee('John Doe');
	echo 'Employee name: '.$employee->getName(); // Employee name: John Doe

Classes

Prefer composition over inheritance

Gang of Fourのデザインパターンでも有名ですが、可能なかぎり継承ではなくコンポジションを使うべきです。
継承を使うべき理由はたくさんあり、コンポジションを使うべき理由もたくさんあります。
この格言のポイントは、継承を使おうと思ったときに、コンポジションを使った方がよりよく書けるのではないか考えてみましょう、ということです。
そして、場合によってはそのとおりになります。

ではいつ継承を使うべきなのかというと、それはもちろん対象の問題によってかわりますが、コンポジションより継承を使ったほうがよいケースは以下のようになります。
・関係が"has-a"(ユーザはユーザ詳細を持つ)ではなく"is-a"(人間は動物である)である。
・ベースクラスのコードを再利用できる(人間はほぼ動物と同様に動く)
・ベースクラスを書き換えて子クラスの挙動を一括で変更したい(全動物の消費カロリー量を変更する)

Bad

	class Employee 
	{
	    private $name;
	    private $email;

	    public function __construct(string $name, string $email)
	    {
	        $this->name = $name;
	        $this->email = $email;
	    }

	    // ...
	}

	// Employeeはtaxを"have"しているので継承は適切ではない
	class EmployeeTaxData extends Employee 
	{
	    private $ssn;
	    private $salary;
	    
	    public function __construct(string $name, string $email, string $ssn, string $salary)
	    {
	        parent::__construct($name, $email);

	        $this->ssn = $ssn;
	        $this->salary = $salary;
	    }

	    // ...
	}

Good

	class EmployeeTaxData 
	{
	    private $ssn;
	    private $salary;

	    public function __construct(string $ssn, string $salary)
	    {
	        $this->ssn = $ssn;
	        $this->salary = $salary;
	    }

	    // ...
	}

	class Employee 
	{
	    private $name;
	    private $email;
	    private $taxData;

	    public function __construct(string $name, string $email)
	    {
	        $this->name = $name;
	        $this->email = $email;
	    }

	    public function setTaxData(string $ssn, string $salary)
	    {
	        $this->taxData = new EmployeeTaxData($ssn, $salary);
	    }

	    // ...
	}

Avoid fluent interfaces

Fluent interfaceは、メソッドチェーンを使うことで可読性を上昇させるオブジェクト指向の書き方です。
PHPUnit Mock BuilderやDoctrine Query Builder等のBuilderパターンでよく登場しますが、この書き方はコードの冗長性を減らしてくれる代わりにコストが上昇します。
・カプセル化を破壊する。
・Decoratorパターンを破壊する。
・テストでモックが使いにくくなる。
・コミットログのdiffが読みづらくなる。

詳細についてはMarco Pivettaによって書かれたブログポストを読むとよいでしょう。

Bad

	class Car
	{
	    private $make = 'Honda';
	    private $model = 'Accord';
	    private $color = 'white';

	    public function setMake(string $make): self
	    {
	        $this->make = $make;
	        return $this;
	    }

	    public function setModel(string $model): self
	    {
	        $this->model = $model;
	        return $this;
	    }

	    public function setColor(string $color): self
	    {
	        $this->color = $color;
	        return $this;
	    }

	    public function dump(): void
	    {
	        var_dump($this->make, $this->model, $this->color);
	    }
	}

	$car = (new Car())
	  ->setColor('pink')
	  ->setMake('Ford')
	  ->setModel('F-150')
	  ->dump();

Good

	class Car
	{
	    private $make = 'Honda';
	    private $model = 'Accord';
	    private $color = 'white';

	    public function setMake(string $make): void
	    {
	        $this->make = $make;
	    }

	    public function setModel(string $model): void
	    {
	        $this->model = $model;
	    }

	    public function setColor(string $color): void
	    {
	        $this->color = $color;
	    }

	    public function dump(): void
	    {
	        var_dump($this->make, $this->model, $this->color);
	    }
	}

	$car = new Car();
	$car->setColor('pink');
	$car->setMake('Ford');
	$car->setModel('F-150');
	$car->dump();

SOLID

SOLIDとは、Robert Martinが考え、Michael Feathersが略称を作った、オブジェクト指向プログラミングと設計についての5つの基本原則です。

Single Responsibility Principle (SRP)

Clean Codeには『クラスを変更する理由はひとつだけ』と記されています。
飛行機にスーツケースをひとつしか持ち込めない場合のように、多くの機能を詰め込んだクラスを作りたいと思うかもしれません。
しかし、そうすることでクラスにはまとまりがなくなり、クラスを変更する複数の理由ができてしまいます。
クラスを変更しなければならない理由は最小限に抑えることが重要です。
ひとつのクラスに多くの機能があると、その変更が他のモジュールにどのような影響を及ぼすかを把握することが困難になってしまいます。

Bad

	class UserSettings
	{
	    private $user;

	    public function __construct(User $user)
	    {
	        $this->user = $user;
	    }

	    public function changeSettings(array $settings): void
	    {
	        if ($this->verifyCredentials()) {
	            // ...
	        }
	    }

	    private function verifyCredentials(): bool
	    {
	        // ...
	    }
	}

Good

	class UserAuth 
	{
	    private $user;

	    public function __construct(User $user)
	    {
	        $this->user = $user;
	    }
	    
	    public function verifyCredentials(): bool
	    {
	        // ...
	    }
	}

	class UserSettings 
	{
	    private $user;
	    private $auth;

	    public function __construct(User $user) 
	    {
	        $this->user = $user;
	        $this->auth = new UserAuth($user);
	    }

	    public function changeSettings(array $settings): void
	    {
	        if ($this->auth->verifyCredentials()) {
	            // ...
	        }
	    }
	}

Open/Closed Principle (OCP)

Bertrand Meyerは、『ソフトウェア(クラス、モジュール、関数など)は拡張のために開かれていなければならないが、変更には閉じていなければならい』と言っています。
これはどういう意味でしょうか。
ここでは、既存のコードを変更せずに新たな機能を追加できるようにすべきということを表します。

Bad

	abstract class Adapter
	{
	    protected $name;

	    public function getName(): string
	    {
	        return $this->name;
	    }
	}

	class AjaxAdapter extends Adapter
	{
	    public function __construct()
	    {
	        parent::__construct();

	        $this->name = 'ajaxAdapter';
	    }
	}

	class NodeAdapter extends Adapter
	{
	    public function __construct()
	    {
	        parent::__construct();

	        $this->name = 'nodeAdapter';
	    }
	}

	class HttpRequester
	{
	    private $adapter;

	    public function __construct(Adapter $adapter)
	    {
	        $this->adapter = $adapter;
	    }

	    public function fetch(string $url): Promise
	    {
	        $adapterName = $this->adapter->getName();

	        if ($adapterName === 'ajaxAdapter') {
	            return $this->makeAjaxCall($url);
	        } elseif ($adapterName === 'httpNodeAdapter') {
	            return $this->makeHttpCall($url);
	        }
	    }

	    private function makeAjaxCall(string $url): Promise
	    {
	        // request and return promise
	    }

	    private function makeHttpCall(string $url): Promise
	    {
	        // request and return promise
	    }
	}

Good

	interface Adapter
	{
	    public function request(string $url): Promise;
	}

	class AjaxAdapter implements Adapter
	{
	    public function request(string $url): Promise
	    {
	        // request and return promise
	    }
	}

	class NodeAdapter implements Adapter
	{
	    public function request(string $url): Promise
	    {
	        // request and return promise
	    }
	}

	class HttpRequester
	{
	    private $adapter;

	    public function __construct(Adapter $adapter)
	    {
	        $this->adapter = $adapter;
	    }

	    public function fetch(string $url): Promise
	    {
	        return $this->adapter->request($url);
	    }
	}

Liskov Substitution Principle (LSP)

リスコフの置換原則は非常にシンプルながら深い意味のある概念です。
『SがTのサブタイプである場合、TはSで置き換えることができる』
すなわち、クラスTのオブジェクトはそのサブクラスSのオブジェクトに入れ替えても動かなければなりません。
これは恐るべき定義です。

この簡単な説明としては、親クラスと子クラスがあった場合に、うっかり間違って使うべきじゃなかった方のクラスを使ったとしても、特に問題なく動作しなければならない、ということです。
実例として、よくあるSquare-Rectangleの例を見てみましょう。
数学的にはSquareはRectangleですが、"is-a"を継承用いて表現している場合、それが問題になります。

Bad

	class Rectangle
	{
	    protected $width = 0;
	    protected $height = 0;

	    public function render(int $area): void
	    {
	        // ...
	    }

	    public function setWidth(int $width): void
	    {
	        $this->width = $width;
	    }

	    public function setHeight(int $height): void
	    {
	        $this->height = $height;
	    }

	    public function getArea(): int
	    {
	        return $this->width * $this->height;
	    }
	}

	class Square extends Rectangle
	{
	    public function setWidth(int $width): void
	    {
	        $this->width = $this->height = $width;
	    }

	    public function setHeight(int $height): void
	    {
	        $this->width = $this->height = $height;
	    }
	}

	/**
	 * @param Rectangle[] $rectangles
	 */
	function renderLargeRectangles(array $rectangles): void
	{
	    foreach ($rectangles as $rectangle) {
	        $rectangle->setWidth(4);
	        $rectangle->setHeight(5);
	        $area = $rectangle->getArea(); // 20を返してほしいのに25が返ってくる。NG
	        $rectangle->render($area);
	    }
	}

	$rectangles = [new Rectangle(), new Rectangle(), new Square()];
	renderLargeRectangles($rectangles);

Good

	abstract class Shape
	{
	    protected $width = 0;
	    protected $height = 0;

	    abstract public function getArea(): int;

	    public function render(int $area): void
	    {
	        // ...
	    }
	}

	class Rectangle extends Shape
	{
	    public function setWidth(int $width): void
	    {
	        $this->width = $width;
	    }

	    public function setHeight(int $height): void
	    {
	        $this->height = $height;
	    }

	    public function getArea(): int
	    {
	        return $this->width * $this->height;
	    }
	}

	class Square extends Shape
	{
	    private $length = 0;

	    public function setLength(int $length): void
	    {
	        $this->length = $length;
	    }

	    public function getArea(): int
	    {
	        return pow($this->length, 2);
	    }
	}

	/**
	 * @param Rectangle[] $rectangles
	 */
	function renderLargeRectangles(array $rectangles): void
	{
	    foreach ($rectangles as $rectangle) {
	        if ($rectangle instanceof Square) {
	            $rectangle->setLength(5);
	        } elseif ($rectangle instanceof Rectangle) {
	            $rectangle->setWidth(4);
	            $rectangle->setHeight(5);
	        }

	        $area = $rectangle->getArea(); 
	        $rectangle->render($area);
	    }
	}

	$shapes = [new Rectangle(), new Rectangle(), new Square()];
	renderLargeRectangles($shapes);

Interface Segregation Principle (ISP)

インターフェイス分離の原則とは、『クライアントが使用しないインターフェイスへの依存を強制してはならない』ということです。

この原則が大事なことを示すよい例は、大量の設定を必要とするクラスでしょう。
クライアントに大量の設定を行わせることは有益ではありません。
何故ならば、ほとんどの設定は大抵使用されないからです。
それらをオプショナルにすることで、"fat interface"を避けることができます。

Bad

	interface Employee
	{
	    public function work(): void;

	    public function eat(): void;
	}

	class Human implements Employee
	{
	    public function work(): void
	    {
	        // 働く
	    }

	    public function eat(): void
	    {
	        // 食事する
	    }
	}

	class Robot implements Employee
	{
	    public function work(): void
	    {
	        // すごい働く
	    }

	    public function eat(): void
	    {
	        // ロボットは食事しないのに実装を強要される
	    }
	}

全てのemployeeはworkerだが、全てのworkerがemployeeというわけではない。

Good

	interface Workable
	{
	    public function work(): void;
	}

	interface Feedable
	{
	    public function eat(): void;
	}

	interface Employee extends Feedable, Workable
	{
	}

	class Human implements Employee
	{
	    public function work(): void
	    {
	        // 働く
	    }

	    public function eat(): void
	    {
	        // 食事する
	    }
	}

	// ロボットはFeedableをimplementsしない
	class Robot implements Workable
	{
	    public function work(): void
	    {
	        // 超働く
	    }
	}

Dependency Inversion Principle (DIP)

この原則は2つの要素を含みます。

・『上位モジュールは下位モジュールに依存してはならない』。どちらも抽象に依存するべきです。
・『抽象は実装に依存してはならない』。実装が抽象に依存するべきです。

これは最初はよくわからないかもしれませんが、SymfonyのようなPHPフレームワークはおおむね、この問題を依存性注入という方法で実装しています。
上位モジュールが下位モジュールを直接操作するのを防ぐというDIPの考え方は、完全に同じ概念ではありませんが、DIを使って達成することができます。
この大きな利点は、モジュール間の結合度を減らすことができることです。
結合度の高いモジュールは非常に悪い作りです。
コードのリファクタリングが難しくなるからです。

Bad

	class Employee
	{
	    public function work(): void
	    {
	        // 働く
	    }
	}

	class Robot extends Employee
	{
	    public function work(): void
	    {
	        // すごい働く
	    }
	}

	class Manager
	{
	    private $employee;

	    public function __construct(Employee $employee)
	    {
	        $this->employee = $employee;
	    }

	    public function manage(): void
	    {
	        $this->employee->work();
	    }
	}

Good

	interface Employee
	{
	    public function work(): void;
	}

	class Human implements Employee
	{
	    public function work(): void
	    {
	        // 働く
	    }
	}

	class Robot implements Employee
	{
	    public function work(): void
	    {
	        // ばりばり働く
	    }
	}

	class Manager
	{
	    private $employee;

	    public function __construct(Employee $employee)
	    {
	        $this->employee = $employee;
	    }

	    public function manage(): void
	    {
	        $this->employee->work();
	    }
	}

Don’t repeat yourself (DRY)

DRYの原則を守りましょう。

コードの重複は避けなければなりません。
コードの重複は、何らかのロジックを変更したいときに、複数箇所を修正しなければならなくなるため、よくありません。

貴方はレストラン経営者で、トマト、タマネギ、ニンニク、スパイスなどの在庫を管理しているとします。
もし在庫管理台帳が複数あったとしたら、トマト料理をひとつ提供するたびに全ての台帳を書き換えなければなりません。
台帳がひとつだけであれば、書き換えが必要なのは一カ所だけです。

ほとんど同じ内容でありながらごく一部だけ違う処理を行うため、ほぼ同じコードを書かなければならないことはよくあります。
重複コードの削除とは、ひとつの関数/モジュール/クラスだけで該当の処理を行うようにコードを抽出することです。

正しく抽出することは重要で、そのためにはクラスのセクションで解説しているSOLIDの原則に従う必要があります。
下手に抽出すると、元のコードよりも悪くなる可能性があるので注意しましょう。
とはいえ、良い抽出ができるのであればしない理由はありません。
同じ処理を繰り返さないでください。
さもなければ、ひとつの内容を変更したいだけなのに複数箇所の修正が必要となってしまうことでしょう。

Bad

	function showDeveloperList(array $developers): void
	{
	    foreach ($developers as $developer) {
	        $expectedSalary = $developer->calculateExpectedSalary();
	        $experience = $developer->getExperience();
	        $githubLink = $developer->getGithubLink();
	        $data = [
	            $expectedSalary,
	            $experience,
	            $githubLink
	        ];

	        render($data);
	    }
	}

	function showManagerList(array $managers): void
	{
	    foreach ($managers as $manager) {
	        $expectedSalary = $manager->calculateExpectedSalary();
	        $experience = $manager->getExperience();
	        $githubLink = $manager->getGithubLink();
	        $data = [
	            $expectedSalary,
	            $experience,
	            $githubLink
	        ];

	        render($data);
	    }
	}

Good

	function showList(array $employees): void
	{
	    foreach ($employees as $employee) {
	        $expectedSalary = $employee->calculateExpectedSalary();
	        $experience = $employee->getExperience();
	        $githubLink = $employee->getGithubLink();
	        $data = [
	            $expectedSalary,
	            $experience,
	            $githubLink
	        ];

	        render($data);
	    }
	}

Very_Good

	function showList(array $employees): void
	{
	    foreach ($employees as $employee) {
	        render([
	            $employee->calculateExpectedSalary(),
	            $employee->getExperience(),
	            $employee->getGithubLink()
	        ]);
	    }
	}

Translations

Chinese
Russian
Spanish
Portuguese1 / Portuguese2
Thai
French

感想

半分くらい書いたところで既に半日本語訳があったことに気付いたが、見なかったことにした。

どうもサンプルコードがいまいち適切ではないものが散見される気がする。
createMenuとかConfigurationのGoodって明らかにBadだるぉ？
あとAvoid type-checkingのあたりとか文章とコードが合ってないように見える。