PHPerのための「PHPのリーダブルなコード」を語り合うPHP TechCafe
=========================================

# PHPer's NEWS

::: info
**[Twigに脆弱性 セキュリティリリース](https://symfony.com/blog/twig-security-release-possibility-to-load-a-template-outside-a-configured-directory-when-using-the-filesystem-loader)**

:::

::: info
**[PhpStormの新バージョン 2022.3 のEAP（Early Access
Program：新機能のお試し版）公開](https://blog.jetbrains.com/phpstorm/2022/09/phpstorm-2022-3-eap-is-open/)**
PHPDoc の array shapes 対応 は使えそう！！
:::

::: info
**[Git 2.38がリリース - MicrosoftのScalarリポジトリ管理ツールが追加](https://softantenna.com/blog/git-2-38-released/)**
大規模Gitリポジトリ管理ツール scalar が git に取り込まれた
:::

::: info
**[PHP Core Roundup #6 (PHP Foundation)](https://thephp.foundation/blog/2022/09/30/php-core-roundup-6/)**

:::

::: info
**[PHP Conference 2022【参加レポート】](https://tech-blog.rakus.co.jp/entry/20220929/php)**
PHPカンファレンス 2022のレポート記事です。ブログを書くまでが(ry
:::

::: info
**[ソースコード生成AI「AI Programmer」が対応言語を拡充、正規表現や日本語解説にも対応](https://forest.watch.impress.co.jp/docs/news/1445449.html)**
例えば「電話番号を抽出する」という指示を出すと、そのための正規表現を出力してくれるそうです。便利。
:::

::: info
**[PHP のすべてのイースターエッグ](https://php.watch/articles/php-easter-eggs)**

:::

::: info
**[PHP 8.2.0 RC 4 が利用可能に](https://www.php.net/archive/2022.php#2022-10-13-1)**
RC 5 のリリースは 10/27 を予定
:::

::: info
**[Docker 値上げ](https://www.docker.com/pricing/october-2022-pricing-change-faq/)**

:::

::: info
**[Signs that your PHP code needs refactoring
（PHP コードのリファクタリングが必要なサイン）](https://dev.to/jmau111/signs-that-your-php-code-needs-refactoring-5enf)**

:::

# 特集：PHPのリーダブルなコード

*　PHPのBADコードをいくつか用意
*　どこが悪いかを議論しながら（時間が許す限り）綺麗に直していく

### 参考

[jupeter/clean-code-php](https://github.com/jupeter/clean-code-php#introduction)

------------------

### 初級

#### Sample 1

##### よくないコード

if ($foo === $bar) {
    return true;
} else {
    return false;
}

##### ダメな理由
* 結果をtrueまたはfalseで返したいとき、if文を使用するのは冗長です。
##### 解消例

return $foo === $bar;

#### Sample 2

##### よくないコード

if($isOK && !hasPermission && $hoge !== "sample")
    // なんらかの処理
}

##### よくない理由

if文の条件が複雑になると、単純に読みにくくなるだけでなく、不具合も起こりやすくなる

##### 解消例

早期リターンを使う

if (!$isOK){
    return "NG";
}
if (hasPermission){
    return "OK";
}
if ($hoge !== "sample"){
    //なんらかの処理
}

関数に閉じ込める

if (条件がわかりやすく言語化された関数名()){
    // なんらかの処理
}

#### Sample 3

##### よくないコード

if ($input == 0) {
　echo('0です！');
}

##### よくない理由
PHPにおいて曖昧な比較演算子を使った比較は、想定外のバグの原因になるため注意が必要。
（特にユーザーの入力値やnullやfalseなどが入る可能性がある場合）
基本的には厳密な比較を使用した方がいい。

###### 曖昧な比較の場合

var_dump('001' == 1); // true
var_dump(' 1' == 1);  // true
var_dump(null == 0);  // true
var_dump(null == []); // true
var_dump(null == ''); // true

※PHPのバージョンが8以前の場合は、文字列と数値を比較する際に文字列が数値にキャストされるためさらに危険

var_dump(0 == "a"); // true

##### 解消例

if ($input === 0) {
　echo('0です！');
}

-------------------------

### 中級

#### Sample 4

##### よくないコード

/**
* お店が営業日かをチェックする
*
* @param $day 曜日の文字列
* @return bool
*/
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;
            }
            return false;
        }
        return false;
    }
    return false;
}

##### よくない理由

1. どういうチェックが必要なのかがわかりにくい
* 分岐をすべて読まないとチェックしたい内容がわからない
* day が以下の場合に true になる
* friday
* saturday
* sunday
* if-else で条件分岐を作る際は、単純化できないか考えるチャンス
* 分岐を進むにつれて、該当処理が実行される条件の数が多くなる
* これまでの分岐条件を記憶しなければならない
2. 引数に型が指定されていないためメソッド内で値のチェックをしなければならない
* ロジックを読み始めてから、Stringの値が前提になっている処理であることを理解する
* 引数の名前から DateTime クラスのインスタンスを受け取るのかと思った
* 実際は String を受け取ることを前提とした処理になっていることを後から理解
3. 前提部分のチェック処理はネストを深くせずガード節で対応する
* 処理の最初で return する

参考
https://github.com/jupeter/clean-code-php#avoid-nesting-too-deeply-and-return-early-part-1

##### 解消例

/**
* お店が営業日かをチェックする
*
* @param string $day 曜日の文字列
* @return bool
*/
function isShopOpen(string $day): bool
{
    // 値がセットされているか（これがガード節）
    if (empty($day)) {
        return false;
    }

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

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

#### Sample 5

##### よくないコード

###### よくないコード（修正前）

<?php
public function getRecipeListString():string {
    $recipeList = getRecipeList(); // なにかレシピのリストを配列で取得するもの
    
    return implode(",", recipeList);
}

###### よくないコード（修正後）

<?php
public function getRecipeListString(bool $isSpace):string {
    $recipeList = getRecipeList(); // なにかレシピのリストを配列で取得するもの
    
    if ($isSpace) {
        return implode(" ", recipeList);
    } else {
        return implode(",", recipeList);
    }
}

##### よくない理由

* if文をどんどん増やすことになる
* 違う要件が来た時にまたif文が増える
* 保守性も可読性も低い

##### 解消例

処理を分けるパターン

public function getRecipeListStringWithComma():string {
    $recipeList = getRecipeList(); // なにかレシピのリストを配列で取得するもの
    
    return implode(",", $recipeList);
}

public function getRecipeListStringWithSpace():string {
    $recipeList = getRecipeList(); // なにかレシピのリストを配列で取得するもの
    
    return implode(" ", $recipeList);
}

デリミタを引数にするパターン

public function getRecipeListString(string $delimiter):string {
    $recipeList = getRecipeList(); // なにかレシピのリストを配列で取得するもの
    
    return implode($delimiter, $recipeList);
}

#### Sample 6

##### よくないコード

<?php
// メルマガ購読顧客リストまたはYoutubeのチャンネル登録会員リストを更新する
function updateMailMagazineListOrYoutubeList($accountId, $MailMagazine, $Youtube, $isMailMagazine, $isYoutube) {
    if ($isMailMagazine) {
        # code...
    } else if($isYoutube) {
        # code...
    }
    
    if ($isMailMagazine) {
        $sql = "update mailMagazineList set ...";
    } else if($isYoutube) {
        $sql = "update youtubeList set ...";
    };

    
}

##### よくない理由

* 別のビジネス概念を無理に1つの関数で処理するためにフラグ引数を渡している。
* 今後更に別のビジネス概念が増えるとより複雑化すると想定される
* 実装者は共通化する意図を持っていたと思われるが、結局共通化できていない

##### 修正後のコード

<?php
// 異なるビジネス概念を扱うなら関数・クラスを別にする
function updateMailMagazineList($accountId, $MailMagazine) {
　# code ...
  $sql = "update mailMagazineList set ...";
  # fugafuga...
}


function updateYoutubeList($accountId, $Youtube) {
  
  # code ...
  $sql = "update youtubeList set ...";
  // hogehoge...
    
}

-------------------------

### 上級

#### Sample 7

##### よくないコード

<?php
/**
* PHPによる形態素解析処理
* 
* @param string $code 文章
*/
function parseBetterPHPAlternative(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...
    }
}

##### よくない理由

1. 1つの関数で複数の処理を行っている
* 修正時の影響が大きくなる
* 修正箇所の後続処理への影響を考えて修正しなければならない
2. 不具合が発生した際の問題個所の特定が困難
* 複数の処理が組み込まれていると処理の前後を理解する必要がある
* 処理が分割していれば問題箇所も特定しやすい
4. 内部処理の再利用ができない
* 処理の中に組み込まれてしまうと特定処理だけを他の処理でも利用したくても利用できない
5. テストが書きにくい
* 複数の処理がまとまっていると細かいテストコードが書けない
* 処理が分割されていれば細かな条件のテストコードが書ける

参考
https://github.com/jupeter/clean-code-php#functions-should-only-be-one-level-of-abstraction

##### 解消例

/**
* トークナイザ
*/
class Tokenizer
{
    /**
    * 文章を単語に分解する
    * 
    * @param string $code 文章
    * @return array $tokens 単語のリスト
    */
    public function tokenize(string $code): array
    {
        $regexes = [
            // ...
        ];

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

        return $tokens;
    }
}

/**
* 字句解析器 
*/
class Lexer
{
    /**
    * 単語の解析処理
    * 
    * @param string $tokens 単語のリスト
    * @return array $ast 解析結果のリスト
    */
    public function lexify(array $tokens): array
    {
        $ast = [];
        foreach ($tokens as $token) {
            $ast[] = /* ... */;
        }

        return $ast;
    }
}

/**
* PHPによる形態素解析処理
* 
* @param string $code 文章
*/
class BetterPHPAlternative
{
    /** @var Tokenizer */
    private $tokenizer;

    /** @var Lexer */
    private $lexer;

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

    /**
    * 形態素解析
    * 
    * @param string $code 文章
    */   
    public function parse(string $code): void
    {
        $tokens = $this->tokenizer->tokenize($code);
        $ast = $this->lexer->lexify($tokens);
        foreach ($ast as $node) {
            // parse...
        }
    }
}

#### Sample 8

##### よくないコード

<?php

class FavoriteRecipe {

  public function getRecipe(string $name):void {

    $limit = 10;

    $recipeRepository = new RecipeRepository();
    $recipes = $recipeRepository.findByName($name, $limit);

    foreach ($recipes as $recipe) {
      var_export($recipe);
    }
  }
}

<?php
class RecipeRepository {

  // Co●kPadからレシピを取得する
  function findByName(string $name, int $limit):Recipe {
    $cookpad = new Cookpad(new \PHPHtmlParser\Dom);
    $result = $cookpad->search($name, 1, $limit, false);
    return $result;
  }
}

### よくない理由

参考
https://github.com/jupeter/clean-code-php#dependency-inversion-principle-dip

FavoriteRecipeクラスがRecipeRepositoryクラスの実装に依存している

domain層
FavoriteRecipe

↓

infra層
RecipeRepository

この場合RecipeRepositoryクラスのfindByNameメソッドの変更がFavoriteRecipeクラスにも波及する。

(例)findByNameメソッドの引数を増やした場合
⇒FavoriteRecipeクラスのgetRecipeメソッドの`$recipeRepository.findByName($name, $limit);`も修正する必要が出てくる

RecipeRepositoryクラスのfindByNameメソッドを修正

<?php
class RecipeRepository {

  // 引数を修正
  function findByName(string $name,int $pageNum, int $limit, bool $isRandom):Recipe[] {
    $cookpad = new Cookpad(new \PHPHtmlParser\Dom);
    $result = $cookpad->search($name, $pageNum, $limit,$isRandom);
    return $result;
  }
}

FavoriteRecipeクラスのgetRecipeメソッドを修正

<?php

class FavoriteRecipe {

  public function getRecipe(string $name):void {

    $pageNum = 10;
    $limit = 10;
    $isRandom = false;

    $recipeRepository = new RecipeRepository();
    // reposiotoryの修正を受けて引数を修正
    $recipes = $recipeRepository.findByName($name, $pageNum, $limit, $isRandom);

    foreach ($recipes as $recipe) {
      var_export($recipe);
    }
  }
}

`$recipeRepository.findByName`を利用している場所が他にもあれば修正が波及する。

実装の詳細に変更は付き物。実装の詳細に依存することで変更に脆くなる。

Clean Codeの例でいうと、継承元の変更が派生クラスに影響する。

### 解消例

抽象に依存せよ

インターフェースを作成

<?php

interface RecipeRepository {
  public function findByName(string $name, $pageNum = 10, $limit = 10, $isRandom = false):Recipe[];
}

Reoisitoryクラスはインターフェースを実装するよう修正

<?php

class RecipeRepositoryImpl implements RecipeRepository {

  public function findByName(string $name, $pageNum = 10, $limit = 10, $isRandom = false): Recipe[] {
    $cookpad = new Cookpad(new \PHPHtmlParser\Dom);
    $result = $cookpad->search($name, $pageNum, $limit, $isRandom);
    return $result;
  }
}

FavoriteRecipeクラスはインターフェースを利用する。

<?php

class FavoriteRecipe {

  private RecipeRepository $recipeRepository;

  public function __construct(RecipeRepository $recipeRepository) {
    $this->recipeRepository = $recipeRepository;
  }

  public function getRecipe(string $name):void {

    $pageNum = 10;
    $limit = 10;
    $isRandom = false;

    $recipes = $this->recipeRepository.findByName($name, $pageNum, $limit, $isRandom);

    foreach ($recipes as $recipe) {
      var_export($recipe);
    }
  }
}

domain層
FavoriteRecipe
↓
infra層
RecipeRepository
↑
RecipeRepositoryImpl

RecipeRepositoryインターフェースをDomain層に移動させると

domain層
FavoriteRecipe
↓
RecipeRepository
↑
infra層
RecipeRepositoryImpl

上位モジュールが下位モジュールに依存しなくなる。