Laravelでよりクリーンなコードを書く
- 2022年4月07日
- 未分類
行を正しく分割する
行は適当に分けないが、長くしすぎないようにする。配列を[]で開き、値をインデントするとうまくいく傾向がある。長い関数のパラメータ値も同様です。他にも、連鎖した呼び出しやクロージャも行を分割するのに適しています。
Bad
// No line split
return $this->request->session()->get($this->config->get('analytics.campaign_session_key'));
// Meaningless line split
return $this->request
->session()->get($this->config->get('analytics.campaign_session_key'));
// Good
return $this->request->session()->get(
$this->config->get('analytics.campaign_session_key')
);
// Closure
new EventCollection($this->events->map(function (Event $event) {
return new Entries\Event($event->code, $event->pivot->data);
}));
// Array
$this->validate($request, [
'code' => 'string|required',
'name' => 'string|required',
]);
ルックアップテーブルを使用する
else if ]ステートメントを繰り返し書く代わりに、配列を使って、持っているキーに基づいて、欲しい値を探します。コードはよりすっきりして読みやすくなり、何か問題が発生したときには理解しやすい例外を見ることができます。中途半端なエッジケースはありません。
// Bad
if ($order->product->option->type === 'pdf') {
$type = 'book';
} else if ($order->product->option->type === 'epub') {
$type = 'book';
} else if ($order->product->option->type === 'license') {
$type = 'license';
} else if ($order->product->option->type === 'artwork') {
$type = 'creative';
} else if $order->product->option->type === 'song') {
$type = 'creative';
} else if ($order->product->option->type === 'physical') {
$type = 'physical';
}
if ($type === 'book') {
$downloadable = true;
} else if ($type === 'license') {
$downloadable = true;
} else if $type === 'creative') {
$downloadable = true;
} else if ($type === 'physical') {
$downloadable = false;
}
// Good
$type = [
'pdf' => 'book',
'epub' => 'book',
'license' => 'license',
'artwork' => 'creative',
'song' => 'creative',
'physical' => 'physical',
][$order->product->option->type];
$downloadable = [
'book' => true,
'license' => true,
'creative' => true,
'physical' => false,
][$type];
可読性を向上させる場合は変数を作成する
複雑な呼び出しから値が得られることもあり、そのような場合は変数を作成すると可読性が向上し、コメントの必要性がなくなります。文脈が重要であり、最終的な目標は読みやすさであることを忘れないでください。
// Bad
Visit::create([
'url' => $visit->url,
'referer' => $visit->referer,
'user_id' => $visit->userId,
'ip' => $visit->ip,
'timestamp' => $visit->timestamp,
])->conversion_goals()->attach($conversionData);
// Good
$visit = Visit::create([
'url' => $visit->url,
'referer' => $visit->referer,
'user_id' => $visit->userId,
'ip' => $visit->ip,
'timestamp' => $visit->timestamp,
]);
$visit->conversion_goals()->attach($conversionData);
アクションクラスの作成
一つのアクションのためにクラスを作ることで、物事がきれいになることもあります。モデルは、それに関連するビジネスロジックをカプセル化する必要がありますが、あまり大きくなりすぎるのもよくありません。
// Bad
public function createInvoice(): Invoice
{
if ($this->invoice()->exists()) {
throw new OrderAlreadyHasAnInvoice('Order already has an invoice.');
}
return DB::transaction(function () use ($order) {
$invoice = $order->invoice()->create();
$order->pushStatus(new AwaitingShipping);
return $invoice;
});
}
// Good
// Order model
public function createInvoice(): Invoice {
if ($this->invoice()->exists()) {
throw new OrderAlreadyHasAnInvoice('Order already has an invoice.');
}
return app(CreateInvoiceForOrder::class)($this);
}
// Action class
class CreatelnvoiceForOrder
{
public function _invoke(Order $order): Invoice
{
return DB::transaction(function () use ($order) {
$invoice = $order->invoice()->create();
$order->pushStatus(new AwaitingShipping);
return $invoice;
});
}
}
イベント利用
コントローラからイベントへのロジックのオフロードを検討します。例えば、モデルを作成するときです。この利点は、モデルの作成がどこでも(コントローラ、ジョブ、…)同じように動作し、コントローラはDBスキーマの詳細について心配する必要がなくなるということです。
// Bad
// Only works in this place & concerns it with
// details that the model should care about.
if (! isset($data['name'])) {
$data['name'] = $data['code'];
}
$conversion = Conversion::create($data);
// Good
$conversion = Conversion::create($data);
// Model
class ConversionGoal extends Model
{
public static function booted()
{
static::creating(function (self $model) {
$model->name ??= $model->code;
});
}
}
来週は、クリーンコードに関するより多くのヒントを紹介する予定です。
By Tsuki
tsuki at 2022年04月07日 10:51:15