Laravelでクエリ数を70から7にした時に行ったこと
はじめに
先日、Laravelのプロジェクトにて、クエリ数を70から7まで減らすリファクタリングを行いました。
また、クエリ数の削減だけではなく、コントローラーのスリム化や可読性の向上も意識したため、今回のリファクタリングで行ったことをまとめたいと思います。
注) 初歩的な内容になっています。ご了承ください。
クエリ発行数を減らすために行ったこと
1. constructに書いてあったクエリ発行処理を廃止
現在のプロジェクトにはLaravelデバックバーが使用されているのですが、すぐに気になったのが、使用されていないはずのテーブルのクエリまで発行されていること。
使ってないはずなのに、なぜ？と思いcontrollerを見てみると、まさかのconstruct内で大量のクエリを発行する処理が書かれていました。
そのため、まずはどの関数で必要な処理なのかを1つずつ確認をし、地道に削除することで、大幅にクエリ数が減りました。
2. Repository内の1つの関数で複数のデータをまとめて取得する関数を削除
controller内にあった$getAllDataという変数。
何のデータをとってきてるんだろう。と思い追ってみると、下記のような実装になっていました。
// controller
$getALLData = $this->bookService->getAllData($user_id);
// bookService
public function getAllData($user_id)
{
return [
'comments' => $this->bookRepository->getAllComment(),
'fans' => $this->bookRepository->getAllFans(),
'data' => $this->bookRepository->getAllData(),
'titles' => $this->bookRepository->getAllTitles(),
'book_stores' => $this->bookRepository->getAllBookStores(),
];
}
そしてbookRepositoryでそれぞれのテーブルからデータを取得する処理が書かれています。
でも、実際に使用されているデータはこの中の1つ、もしくは全く使用されていないにもかかわらず、決まり文句のように書かれているものもありました。
こちらは1つの関数では1つのデータ取得のみ行うようにし、使用していないものに関しては速やかに削除しました。
3. リレーション先のデータはwith()を使用する
今回使用したwith()に関しては下記記事にまとめているので、こちらをご参照いただけると嬉しいです。
withを使用した様々なデータの取得方法
1つのレコードを取得する場合はjoinでも問題ないかと思いますが、複数のレコードを取得する場合には、joinで取得をすると大量のクエリを発行します。
データ取得時にはwith()が使用されていなかったため、joinで取得していたものをwith()へ変更しました。
コントローラーのスリム化や可読性向上のために取り組んだこと
1. クリーンアーキテクチャに従いコードを移動させる
クリーンアーキテクチャに関しては、こちらの記事を参考にさせていただきました。
【5分でざっくり理解！】Laravelでクリーンアーキテクチャ超入門
リファクタ前のコントローラーでは、 ルーティング以外のDB操作やバリデーション等も書いている状態だったため、まずは本来コントローラーに書くべきではないとされているものをお引っ越ししました。
また、処理をService層やRepository層に分けることによって、コントローラー内に繰り返し書かれている処理を、1つにまとめることもできました。
2. 処理の共通化を意識する
引数が違うだけで、他は全部同じ処理を書いていたり、foreachで回している中に書いている処理かそうでないかだったり、、、
上記のような違いであれば、共通化することは難しくありません。なるべく共通化を意識してリファクタリングを進めていきました。
以下に簡単に例をあげていきます。
① 引数が違う場合の共通化
例えば先ほど説明したwith()を使用する場合、処理によってwith()に入るリレーション先が異なる場合もあるかもしれません。
その時は、with()に入るリレーションを変数に入れて渡すことで、処理を共通化することができます。
$comments = $this->bookService->getComments('comments');
// bookService
public function getComments($with)
{
return $this->bookRepository($with);
}
// bookRepository
public function getComments($with)
{
DB:table('books')->with($with)->get();
}
② foreach内で行っている処理を他の処理と共通化
foreach内で行っている処理かどうかだけで処理を分けている場合は、その処理も共通化することができます。
// bookService
public function checkAllBooks($books)
{
foreach ($books as $book) {
$comment = $this->checkHavingComment($book);
}
}
public function checkHavingComment($book)
{
// 処理を書く
}
また、同じファイル内において複数の関数で使用されているコードに関しては、private関数にまとめるようにしました。
例えば、selectするカラム名や、検索条件等です。そうすることで、可読性も高くなったように思えます。
3. 検索や絞り込み条件はscopeを使用しModelに記述する
今回のプロジェクトは複数条件の検索機能が実装されており、検索機能を書いたコードはかなり膨大になっていました。
そのため、関数名で何を検索しているかわかるようにし、実際の処理はModelに分けて書きました。
public function searchBooksByConditions($keyword)
{
$this->books
->searchTitle($keyword['title'] ?? '')
->searchAuthor($keyword['author'] ?? '')
->searchPulishDate($keyword['publish_date'] ?? '');
}
// Book.php
public function scopeSearchTitle(Builder $query, $keyword)
{
if ($keyword) {
$query->where('title', 'like', '%' . $keyword . '%');
}
return $query;
}
4. その他 (早期リターンの活用、不要な変数の使用廃止など)
異常系の処理はなるべく手前でリターンをさせる。elseifを乱用しない、elseを使用している処理は、本当に必要なのか再考する。
といったことを心がけ、なるべくネストが深くならないように意識しました。
// 異常系の処理を最後の方に書いた例
function searchBooks()
{
$books = $this->getAllBooks();
foreach ($books as $book) {
// 処理が続く
}
if ($books->isEmpty()) {
return false;
}
return $all_books;
}
// 異常系の処理を手前に書いた例
function searchBooks()
{
$books = $this->getAllBooks();
if ($books->isEmpty()) return false;
foreach ($books as $book) {
// 処理が続く
}
return $all_books;
}
異常系処理の際には、早期リターンをすることで不要な処理を行わずにリターンできるし、後者の方が読みやすいかと思います。
また、不要な変数使用は廃止をし、変数を定義する場所も実際に使用する処理の直前に書くなど、可読性向上も意識しています。
// 不要な変数を使用している例
$books = $this->getAllBooks();
$all_books = $books; // $all_booksへの変数定義は不要
foreach ($books as $book) {
// 処理が続く
}
(本当に上記のようなコードがあったのです、、、)
おわりに
今回はリファクタリングにあたり、主に取り組んだことや意識したことをまとめてみました！
コードが読みにくかったり冗長していると、仕様変更や機能追加があった際には思わぬバグにつながりやすいので、リファクタリングをすることが大切だと感じました。
(何度もため息が出て、心が折れそうになりましたが、基本的には楽しかったです！)
最後まで読んでいただきありがとうございました！
Discussion