LaravelのModelクラス内でAuthを使わない方がよい理由

2024年4月29日

URLをコピーしました！

Auth に限らないのですが、今回は Model クラスはできるだけ依存を少なくしたほうがよいという話し

ソースレビューの際の話し

例として App\Models\User.php の中に isActive がありました。ソースレビューで私は「AuthはModel内で使わないほうがいい」と指摘しました。

~略~
use Illuminate\Support\Facades\Auth;

class User extends Authenticatable
{
    use HasApiTokens, HasFactory, Notifiable;

~略~

    /**
     * ユーザーステータスから判定して、継続中の場合は true を返す
     *
     * @return boolean
     */
    public static function isActive() : bool
    {
        $user = Auth::user();
        if( $user->user_status_id == 2 || $user->user_status_id == 3 ) {
            return true;
        }
        return false;
    }

Authを使うとログイン情報を取得できるのですごく便利なのですが、処理内容を見ると特にAuthが必要だったわけではなく、Authで取得できるUser情報の user_status_id を使いたかっただけのようでした。

個人的な判断条件としては、Authを使ってログインしているかどうかをチェックする場合には、Authを使ってよいと思います。今回のケースではやりたいことは user_status_id を使った判定でしたので、Authを使わずに、引数として user_status_id を渡すように伝えました。Authを使うか使わないかの判断は、「Authでしかできないこと」があるかどうか？になります。

修正後のソースコードはこちらです。

~略~

class User extends Authenticatable
{
    use HasApiTokens, HasFactory, Notifiable;

~略~

    /**
     * ユーザーステータスから判定して、継続中の場合は true を返す
     *
     * @param  int $user_status_id
     * @return boolean
     */
    public static function isActive( $user_status_id ) : bool
    {
        if( $user_status_id == 2 || $user_status_id == 3 ) {
            return true;
        }
        return false;
    }

もしログインの判定が必要でしたら、 isActive の呼び出しの前に判定すればよいと思います。
依存関係を減らし、関数の役割を明確化することで、Unitテストも簡潔に書けるようになります。

ソースレビューの際に指摘点の意味がわからないことがあったら、しっかりと説明していきたいと思います。