環境
Laravel v9.5.1 (PHP v8.1.3)
前提
恥ずかしながらリファクタリングをまともにやったことがなく、最近覚えたリファクタリングの際の小ネタをまとめる。
小ネタ①
$request->user()
の情報を使わないならそのクラスに置くべきでない。
before
- ポイントを加算し、ポイント履歴をつける
addPoints
メソッドのリファクタリング - User - PointWallet=
1:1
- User - PointHistory=
1:多
user
がaddPoints
するものだと思い、addPoints
メソッドをUserモデルに置いていた。
addPoints
メソッドで$request->user()
の情報が使われていないなら、このメソッドがUserモデルのインスタンスメソッドであるとはいえないので、適したクラスに置こう。
AddPointController.php
class AddPointController extends Controller
{
public function store(Request $request): JsonResponse
{
$request->user()->addPoints($request->user_id, $request->points);
return response()->json(Response::HTTP_OK);
}
}
User.php
class User extends Authenticatable
{
...
public function addPoints(int $userId, float $points): void
{
self::find($userId)->pointWallet()->increment('points', $points);
PointHistory::create([
'user_id' => $userId,
'amount' => $points,
]);
}
}
after
UserモデルではなくPointWalletモデルに置くことに。
AddPointController.php
class AddPointController extends Controller
{
...
public function store(Request $request): JsonResponse
{
$pointWallet = User::find($request->user_id)->pointWallet;
$pointWallet->addPoints($request->points);
return response()->json(Response::HTTP_OK);
}
}
PointWallet.php
class PointWallet extends Model
{
...
public function addPoints(float $points): void
{
$this->increment('points', $points);
PointHistory::create([
'user_id' => $this->user_id,
'amount' => $points,
]);
}
}
小ネタ②
if文の中は文章になるように条件文を命名する。
before
「is_null($request->user())
=アクセスユーザーがnullだったら」はまだ単純なのでぱっとみで理解できそうだが、なるべく読み手(他人かもしれないし未来の自分かもしれない)の脳のリソースを削がないようにする。
PostController.php
class PostController extends Controller
{
public function store(Request $request)
{
if (is_null($request->user())) {
// 何か処理
}
return response()->json(Response::HTTP_OK);
}
}
after
「アクセスユーザーがnullはこの仕様でいうとつまりどういうことか?」を英語にする。
ここでは「アクセスユーザーがnull=ゲストユーザー(新規登録してない)かどうか」ということにした。
PostController.php
class PostController extends Controller
{
public function store(Request $request)
{
$isGuestUser = is_null($request->user());
if ($isGuestUser) {
// 何か処理
}
return response()->json(Response::HTTP_OK);
}
}