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テストも簡潔に書けるようになります。
ソースレビューの際に指摘点の意味がわからないことがあったら、しっかりと説明していきたいと思います。
