それ関数にした方が良くないですか?(Laravel Better Code)

2021/03/31に公開

Laravelのコードを書くときに、より良いコードを書くために考えた結果、
関数として切り出す方が、リーダブルでテスタブルな良いコードになると思ったことがあったので、記事にしてみました。

この内容に対して別の意見等ございましたら、ぜひコメント欄で議論しましょう!

ケース1: if文で判定する場合

例えば、このようなテーブルがあるとします
genderは1...男性, 2...女性, 3...その他 で定義しているとします

ユーザーを取得して、それぞれの性別で処理を分けたい場合があるとすると、
このように条件分岐をするかもしれません。

public function processEachGender()
{
     $user = User::find(1);
     
     // 男性の場合
     if ($user->gender === 1) {
     
     }
     
     // 女性の場合
     elseif ($user->gender === 2) {
     
     }
     
     // その他の場合
     else {
     
     }
}

しかし、私が考えるベストなコードはこちらです。

モデル(User.php)で男性の場合、女性の場合、その他の場合の関数を先に作っておきます


Class User extends Model 
{
    // 男性かどうかの判定
    public function isMan()
    {
        return $this->gender === 1;
    }
    
    // 女性かどうかの判定
    public function isWoman()
    {
        return $this->gender === 2;
    }
    
    // その他かどうかの判定
    public function isOther()
    {
        return $this->gender === 3;
    }
}

そしてControllerでは、定義した関数を使用します。

public function processEachGender()
{
     $user = User::find(1);
     
     // 男性の場合
     if ($user->isMan()) {
     
     }
     
     // 女性の場合
     elseif ($user->isWoman()) {
     
     }
     
     // その他の場合
     if ($user->isOther()) {
     
     }
}

こっちの方が、直感的に見やすくないですか??

$userがManの場合なんだ!と開発者に優しいコードだと思います。

ちなみに $this->gender === 1; とかいう判定の仕方をしていますが、定数に関してはconfigディレクトリ配下に置くか、モデルの中で定数を定義するか、enumを使うかした方がよりよいのでそちらは今回の記事では詳しく説明しませんが、気になる方は調べてみてください。

モデルめっちゃ関数増えませんか?と言われたら

モデルの関数が増えて、コード量めちゃくちゃ多くなりませんか?という反論が聞こえてきます。

そうなんです、それでいいんです。

重要なのは、同じ処理を行う可能性のあるものは関数にして再利用しましょうということです。その方が品質も保たれるし、関数の名前を適切につけてあげれば開発者にも読みやすいですし、関数として切り出されていたほうがテストもしやすい!!です!!

ケース2. 代入する際に関数にする

先程は条件分岐の際に関数に書き換えてみましたが、
今回は代入するケースを考えてみます。

このようなitemsテーブルがあるとします。

price×1.1 で、税金加算後の金額を知りたい場合の、
Controllerはこのようなコードになるかもしれません。

public function getPriceWithTax() 
{
    $item = Item::find(1);
    $item->price * 1.1;
    .
    .
    .
}

これでも十分コンパクトだと思いますが、私だったらこうしちゃいます

モデル内に税金を計算する関数を作って...

class Item extends Model
{
    public function priceWithTax()
    {
        return $this->price * 1.1;
    }
}

Controllerはこのように書きます

public function getPriceWithTax()
{
    $item = Item::find(1);
    $item->priceWithTax(); // 税金計算後の金額
}

もしくはアクセサを使用して getPriceWithTaxAttributeと定義しても良いかもしれません。

(再翻訳中)Laravel 8.x Eloquent:ミューテタ/キャスト

終わりに

別の意見や反論等ありましたら、ぜひみなさんで協力してより良いコードを書くためにコメント欄で議論しましょう!
より良いの定義も人によって、場面によっても変わると思うので。

質問等もお気軽にコメントでご連絡ください。

では、また!

Discussion