【15日目】テストとリファクタリング【作曲の補助ツールを作るまでの日記】

プログラミング

2023年8月10日~2023年8月13日

リファクタリングを少し進めたいのと、本番環境に向けた準備を進める。
といっても、どちらも知識がないので結構困る

テスト環境を作る

リファクタリングをする前に、テスト環境を作ることにする。

理由は、リファクタリングしたコードが同じように動くという保証が気軽に欲しいから。
※これは追記だけど、実際にテストを作っている間にバグを見つけたり、リファクタリングの影響範囲が数秒でわかったりするので、めっちゃ便利。

実際にリファクタリングしようと思ったんだけど、「コードを変える=動かなくなる可能性がある」ということなので、毎回テストしたくなるんだよね。
でも、大量のテストを人力でやるのはめっちゃ大変そうなので、いっそここでLaravelのテスト機能に手を出してみる。

テストに関しては日本語ドキュメントももちろんなんだけど、こちらのZennで販売されている本がわかりやすい。
序盤の14/51章が無料で公開されていて、とりあえず12章まで読めばいいのではと思う。所要時間は15分くらい?

とりあえず私はこれを購入したので、これを元に進める。
勿論、本の中身をそのまま下に綴ったりはしてない。

メモ程度で学んだことは残していく。

Laravelテストの基礎知識

LaravelのテストにはFeature(機能)テストと、Unit(単体)テストがある。(他にもブラウザテストもあるけど、一回置いとく)
機能テストの方が多くなる傾向があるらしい。

実際に日本語ドキュメントでも
「一般的に、ほとんどのテストは機能テストである必要があります。これらのタイプのテストは、システム全体が意図したとおりに機能しているという信頼性を一番提供します。」
とある。

位置的にはapp/tests。

また、テストはLaravelがしてくれているのではなく、PHPUnitというライブラリがしてくれている。この、PHPUnitを使いやすくするヘルパ関数があったりと、橋渡し的な位置にLaravelは存在する。

テストコードの例を見てみる

以下は初期からある機能テストのコード

<?php

namespace Tests\Feature;

// use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;

class ExampleTest extends TestCase
{
    /**
     * A basic test example.
     *
     * @return void
     */
    public function test_the_application_returns_a_successful_response()
    {
        $response = $this->get('/');
        $response->assertStatus(200);
    }
}

functionの中身がテストコードのロジック部分で、
$response = $this->get(‘/’);
の部分では、自身のgetメソッドで、’/’にGETリクエストを送り、レスポンスを取得している。

次の
$response->assertStatus(200);
では、このレスポンスがステータスコード200であることを宣言してる。
この宣言がFalseだとそのテストコードは失敗で、Trueだと成功になる。みたいな。

テストの実行は
php artisan test tests/Feature/ExampleTest.php
で指定したコードを実行できるし、
php artisan test
で全部のテストコードを実行できる。

例えば私の環境で、さっきのステータス200を期待するテストコードを実行すると。

302(リダイレクト)のステータスコードだから、失敗してるよ! って感じでテストしてくれる。

トレイトとは何か

某ダークファンタジーのトレントなら知ってるんだけどね。
なんか単語として出てきたので調べる。

“トレイト”はPHPの多重継承が出来ないという悩みを解決するために出来たもの。

そもそも、クラスには継承という概念があって、親クラスから継承した子クラスは親クラスのメソッドやプロパティを定義しなくても使えるようになる。

でも、PHPはこの親を複数持つことが出来ないので、複数のクラスのメソッドを使うことが出来ない。そのため、異なるクラス間で特定のメソッドを共有したいという時、子クラスが既に別の親を継承していると、別のメソッドを継承して使うことが出来なかった。

そこで、「インスタンスは作れないけど、メソッドを作ることのできるクラス」のようなものを作って、共有できるようにしたのが、トレイトというもの。

具体例は以下の通り

<?php
trait ExampleTrait {
    public function exampleMethod() {
        return “Hello trait!”;
    }
}

class MyClass {
    use ExampleTrait;
}

$obj = new MyClass();
echo $obj->exampleMethod();  // Outputs: Hello trait!

つまり、クラス内で”use ○○”とあったらトレイトを利用していると認識すればいいのかな。

テスト時のDBの扱い

テストするときDBはどうするんだと思ったんだけど、DBはテスト専用の物を作るみたい。

また、DBはテスト毎にDBは毎回リセットするらしいんだけど、その時に”RefreshDatabase”というトレントを利用する。

テスト用DBの作成と設定をする

テスト用のDBは普通にMySQLで作成してもらえばよくて、テーブルはマイグレーションが勝手にやってくれる。

設定方法は2通りあって、

  1. phpunit.xmlに記述
    テストにはphpunitが使われるので、phpunitの設定ファイルにテスト用DBを設定する。
  2. .env.testingを作成し記述
    DBって.envで設定したよね。それと同じ要領で.env.testingというenvファイルを作り、そこにテスト用のenvを記述すると、テスト時はそっちを読み込んでくれるみたい。

おすすめは1番の方法らしいので、phpunit.xmlに記述する。

qtm_testというDBを作成し、phpunit.xmlは以下のようにした

また、DBを使うテストの場合、毎回use RefreshDatabase;でDBを1からにするんだけど、テスト毎に記述するのは面倒なので、親クラスのTestCase.phpに記述しておくと幸せになれる。

use Illuminate\Foundation\Testing\RefreshDatabase;
use RefreshDatabase;

をTestCase.phpに追加

Factoryとは何か

ちょくちょく聞いたことはあったけど、ここで使う時が来たFactory。

Factoryは”モデルのインスタンスを生成する為のテンプレ処理”のこと。

だから、テストデータの生成でよく使われる。
日本語ドキュメントの例を見てみる。

namespace Database\Factories;

use Illuminate\Database\Eloquent\Factories\Factory;
use Illuminate\Support\Str;

class UserFactory extends Factory
{
    /**
     * モデルのデフォルト状態の定義
     *
     * @return array
     */
    public function definition()
    {
        return [
            'name' => fake()->name(),
            'email' => fake()->unique()->safeEmail(),
            'email_verified_at' => now(),
            'password' => '$2y$10$92IXUNpkjO0rOQ5byMi.Ye4oKoEa3Ro9llC/.og/at2.uheWG/igi', // password
            'remember_token' => Str::random(10),
        ];
    }

このコードのfake()はnameやmailなどのランダムな値を簡単に作ることが出来る関数。

nameやmail以外にも沢山の関数があり、公式ドキュメントに色々ある。

また、config/app.phpのfaker_locateを”ja_JP”に変更することで名前とかが日本になるみたい。

Factoryを利用するときは以下の通りで、create()を使うとsave()を使ってDBに保存される。

// Factoryを使ってUserモデルの新しいインスタンスを生成するが、DBには保存しない
$userInstance = \App\Models\User::factory()->make();

// Factoryを使ってUserモデルの新しいインスタンスを生成し、DBに保存する
$userInDb = \App\Models\User::factory()->create();

これは、普通にDBに保存されるので、勿論だけど本番環境でやると無駄データ増えるし、保存されたDBはテスト毎に”RefreshDatabase”で削除するのが常識らしいので、尚更本番環境でやっちゃダメだよね。

私の場合、どんなテストが必要なのか

ログイン関係のテストコードはBreezeを入れた際に勝手に作られていた?

ので、ここら辺は新しく作る必要はない。

その上でどんなテストを作るかという話なんだけど、今回テストを作る理由はリファクタリングをする為なので、リファクタリングをする所のテストを作っていけばいいかな。

しかし、王道っぽいテストは欲しいので、各モデルのCRUD処理のテストだけ書いておく。

ArticleControllerテストの作成

とりあえず、骨組みだけを以下のように作った。

    public function test_ゲスト時記事一覧が正常に表示される()
    {
        //
    }
    public function test_ログイン時記事一覧が正常に表示される()
    {
        //
    }
    public function test_ログイン時新規記事作成画面を表示()
    {
        //
    }
    public function test_ゲスト時新規記事作成画面を非表示()
    {
        //
    }
    public function test_ログイン時新規記事を作成できる()
    {
        //
    }
    public function test_ゲスト時新規記事を作成できない()
    {
        //
    }
    public function test_自分の記事は編集画面を開ける()
    {
        //
    }
    public function test_他人の記事は編集画面を開けない()
    {
        //
    }
    public function test_自分の記事は更新できる()
    {
        //
    }
    public function test_他人の記事は更新できない()
    {
        //
    }
    public function test_自分の記事は削除できる()
    {
        //
    }
    public function test_他人の記事は削除できない()
    {
        //
    }

試しに、ArticleのFactoryを作り、ゲスト時記事一覧が正常に表示されるかのテストを作ってみる。

ArticleのFactory

    public function definition()
    {
        return [
            "user_id" => User::factory(),
            "title" => fake()->realText(10),
            "body" => fake()->realText(20),
        ];
    }

タグはnullでも問題ないので、とりあえずこんな感じにしてみた。

User::factory()でUserのクエリビルダを呼び出しているので、
use App\Models\User;
が必要。

また、この呼び出し方はArticleがUserとbelongsToの関係であることをモデルで宣言しとかないといけないので注意。

ゲスト時記事一覧が正常に表示されるテスト

以下みたいな感じ。

    public function test_ゲスト時記事一覧が正常に表示される()
    {
        $article = Article::factory()->create();

        $response = $this->get("/articles");

        $response
            ->assertStatus(200)
            ->assertSee($article->title);
    }

GETリクエストを送る前に、Article::factory()のcreate()メソッドで記事を作成しておく。この時、”user_id” => User:factory()と定義しているので、ユーザーも勝手に作られる。

$response = $this->get(“/articles”);で記事一覧画面のレスポンスを取得して、ステータスが200かと、作成した記事のタイトルが含まれるかを見ている。

実際にこのテストを実行すると

みたいな感じで、passedされる。

ログインが必要なテスト

ログインが必要系のテストは以下のようにする

$user = User::factory()->create();
$this->actingAs($user);

1行目でユーザー作って、$this->actingAs($user)で、そのテスト関数コード内はログインした状態になる。

ログイン時記事が正常に表示されるかテスト

なので、このテストは以下のように書ける

    {
        $user = User::factory()->create();
        $article = Article::factory()->for($user)->create();
        $this->actingAs($user);

        $response = $this->get("/articles");

        $response
            ->assertStatus(200)
            ->assertSee($article->title);
    }

一応ここから省略もできるんだけど、最初だからちょっと長ったらしく書いた。

forはbelongsToの関係を宣言しているとき、factoryで生成するインスタンスに必要な関連する他のモデルインスタンスを定義できる機能。
なので、
$article = Article::factory()->for($user)->create();
では、for($user)で記事の作成者を指定している。

そのため、このテストコードは「自分で生成した記事を記事一覧ページで閲覧できるか」というテストコードになっている。

とりあえず、そんな感じで記述していった。

一応全てpassすることは確認したけど、けっこうすんなりpassしちゃうのが逆に怖い。

ファイルの投稿系テスト

私のサイトには今のところ音源ファイルと画像ファイルの投稿機能がある。
この場合のテストはどう記述するのか。

簡単に言えば、仮想ディスクを作ってその仮想環境上で仮想ファイルのやり取りをする。

一応、仮想ディスクを作らなくてもテストはできるんだけど、その場合しっかり実環境に保存されてしまうので、ということみたい。

仮想ディスクを作るのが以下のようなコードで
Storage::fake(“public”);
仮想ファイルを作るのが以下のようなコード
$file = UploadedFile::fake()->create(“sample.mp3”);

これで後はこのfileをpostしてあげればいい。

リファクタリング

テストがある程度出来たので、手を付けられそうなところからやっていこう。
あと、普通に若干機能を変更したりするので、厳密にはリファクタリングではないと思う。

それと、当たり前ではあるんだけど目的と手段を間違えないようにしないといけない

あくまでもリファクタリングはコードの保守性→人間の為にやるもの。
コードの圧縮≠可読性なので、これが本当に人間の為になるかを考えながらやっていく。

今回やった改善は以下の通り

  1. 更新をPUTリクエストに
  2. ルートのグループ化
  3. コントローラのダイエット
  4. compactに変更

更新をPUTリクエストに

現在のupdateメソッドへのルートが以下の通り。

Route::post("/article/update/{id}", [ArticleController::class, "update"])->name("article.update");

そう。postでやってるんだよね。

テストコードの作り方を紹介している記事ではこれをputでやっている。
実際これは記事の更新なのでそれを意味するputリクエストを利用するべきだよね。

といっても、ルートのpostをputにして、Viewのformに@method(‘PUT’)を追加すればおk。

ちなみに、更新後テストを実行すると更新系のテストが全部落ちた。

これは、postからputに変更したから、メソッド未対応を示す405を返している。

逆に言えば、しっかりテストが動いている証拠でもあるので、良き。

更新系テストのpostをputに変えて完了。

ルートのグループ化

ルートのグルーピングは普通に日本語ドキュメントがわかりやすいが、今回はリソースという方法を使った。

現在のルートは以下の通り(一部省略)

Route::get("/articles", [ArticleController::class, "index"])->name("articles.index");
Route::get("/article/create", [ArticleController::class, "create"])->name("article.create");
Route::post("/article/store", [ArticleController::class, "store"])->name("article.store");
Route::get("/article/edit/{id}", [ArticleController::class, "edit"])->name("article.edit");
Route::put("/article/update/{id}", [ArticleController::class, "update"])->name("article.update");
Route::get("/article/{id}", [ArticleController::class, "show"])->name("article.show");
Route::delete("/article/{id}", [ArticleController::class, "destroy"])->name("article.destroy");

Route::post("/article/{id}/like", [LikeController::class, "store"])->name("article.like");
Route::delete("/article/{id}/unlike", [LikeController::class, "destroy"])->name("article.unlike");

Route::get('/audios', [AudioController::class, "index"])->name("audios.index");
Route::get('/audio/create', [AudioController::class, "create"])->name("audio.create");
Route::post('/audio/store', [AudioController::class, "store"])->name("audio.store");
Route::delete('/audio/{id}', [AudioController::class, "delete"])->name('audio.delete');

これを、グループ化して

Route::resource("article", ArticleController::class);

Route::post("/article/{id}/like", [LikeController::class, "store"])->name("article.like");
Route::delete("/article/{id}/unlike", [LikeController::class, "destroy"])->name("article.unlike");

Route::resource("audio", AudioController::class)->only(["index","create","store","destroy"]);

こんな感じにした。
すっごいスッキリ。

これはRoute::resourceというのを使っていて、日本語ドキュメントから引用なんだけど、コントローラに関する以下のようなルートを自動で作成してくれる。

これはRoute::resource(‘photos’, PhotoController::class);の場合。

でも、見てもらえれば分かるんだけど、URIの構造とか、ルート名が私の仕様と若干変わるんだよね。
ここら辺はテストを作っているので、テスト回しながら修正したり、「ctrl+shift+f」でできるファイル全体検索みたいのを使って、変わったところを修正したりした。

加えて、今までidを渡しているけどこれって上の例だとphotoのインスタンスを渡しているよね。
そんなことしてURLどうなっちゃうの? 
とか
重くならないの? 
とかそういう疑問を考える。

結論から言えば、重くならないしURLも変わらない。

実際にURLを見てみると

みたいな感じでidの番号が割り振られている。これは、route(“articles.show”, $article)としても、内部的にはそのidを取得するよう構造されているからみたいなんだけど、内部構造の話になるとややこしそうなので、一旦そういうもんだと思っておく。


また、重くならないのかという話なんだけど、上の通り内部的にはそのidを渡していて、処理途中でDBクエリでそのidに一致するデータを取得するということをやっている

結果、Controllerにその取得した$articleインスタンスを渡しているので、やっていることはコントローラの中で

$article = Article::find($id);
をやるのと変わりない。

コントローラのダイエット

MVCを学んだ時にもあったけど、”コントローラにビジネスロジックは書かない”という鉄則的なものがある。
理由は、関数に持たせる責任を少なくすることで、可読性・再利用性の向上につながって結果保守性の向上につながるから。

ArticleController_indexのリファクタリング

ここで、現在の私のArticleController_indexメソッドを見てみる。

    public function index(IndexArticleRequest $request)
    {
        // 検索キーワードから記事を取得する処理
        if ($request->filled("keyword")) {
            $keyword = $request->keyword;
            $message = "「" . $keyword . "」を含むタイトルまたはタグが付いた記事一覧";
            $articles = Article::where("title", "like", "%" . $keyword . "%")
                ->orWhereHas("tags", function ($query) use ($keyword) {
                    $query->where("name", "like", "%" . $keyword . "%");
                })->get();
        }
        else {
            $message = "Welcome to QTM";
            $articles = Article::with("tags")->get();
        }

        // いいね数を集める処理
        foreach ($articles as $article) {
            $article -> likeCount = $article->likedUsers->count();
        }
        return view("article.index",["articles"=>$articles,"message"=>$message]);
    }

どのメソッドもバリデーションはRequestでやっていて、認可処理はPolicyでやっているので、多少ましではあるんだけど、それでもビジネスロジック多め。

クエリスコープ

このメソッドの場合、検索キーワードの処理がデカいので以下のようにしてみた。

        

        // 検索キーワードから記事を取得する処理
        if ($request->filled("keyword")) {
            $keyword = $request->keyword;
            $message = "「" . $keyword . "」を含むタイトルまたはタグが付いた記事一覧";
            $articles = Article::includeKeyword($keyword)->with("tags")->get();
        }
        else {
            $message = "Welcome to QTM";
            $articles = Article::with("tags")->get();
        }

これはクエリスコープという、一定のクエリ処理を使いまわせる方法を使ったもの。

Articleモデル側で

 

    // 記事検索クエリ
    public function scopeIncludeKeyword($query, $keyword)
    {
        return $query->where("title", "like", "%" . $keyword . "%")
                    ->orWhereHas("tags", function ($query) use ($keyword) {
                        $query->where("name", "like", "%" . $keyword . "%");
                    });
    }

のように定義していて、これを呼び出すことで検索を行っている。

あと、検索時にwith(“tags”)付けるの忘れてたねこれ。

アクセサ

いいね処理も不必要にできそうなので、やってみた

        // いいね数を集める処理
        foreach ($articles as $article) {
            $article -> totalLike = $article->likedUsers->count();
        }

この、いいねを集める処理をアクセサとして実装することで、わざわざコントローラに書く必要が無くなる。

Articleモデルに以下を追加。

    // 累計いいねアクセサ
    public function getTotalLikesAttribute()
    {
        return $this->likedUsers->count();
    }

こうすると、$article->totalLikes;とすることで定義したアレクサで値を取得できるようになる。

だから、コントローラに定義する必要が無くなる。

最終的にindexメソッドは以下の通りになった。

    public function index(IndexArticleRequest $request)
    {
        // 検索キーワードから記事を取得する処理
        if ($request->filled("keyword")) {
            $keyword = $request->keyword;
            $message = "「" . $keyword . "」を含むタイトルまたはタグが付いた記事一覧";
            $articles = Article::includeKeyword($keyword)->with("tags")->get();
        }
        else {
            $message = "Welcome to QTM";
            $articles = Article::with("tags")->get();
        }

        return view("article.index",["articles"=>$articles,"message"=>$message]);
    }

まだ圧縮は出来ると思うんだけど、とりあえずこんなもんで。

ArticleController_storeのリファクタリング

こちらもまぁまぁ大きい。

しかも、今見るとトランザクション処理とか、成功メッセージの送信とかをしていないんだよね。

    public function store(StoreArticleRequest $request)
    {
        $this->authorize('store',Article::class);
        $article=new Article();
        $user = \Auth::user();

        $article->title = $request->title;
        $article->body = $request->body;
        $article->user_id = $user->id;
        $article->save();

        // タグの処理
        $tagNames = explode(",", $request->tags);
        foreach($tagNames as $tagName){
            $tagName = trim($tagName);
            $tag = Tag::firstOrCreate(["name"=>$tagName]);
            $article->tags()->attach($tag);
        }

        $article->likeCount = $article->likedUsers->count();

        return redirect()->route("article.index");
    }

これは、一回先にテストケースを追加して、作成した記事に飛ぶこと+セッションフラッシュメッセージで、「記事の作成に成功しました!」を条件に入れよう。

テストコードを以下のように更新した。

    

    public function test_ログイン時新規記事を作成できる()
    {
        $user = User::factory()->create();
        $this -> actingAs($user);

        $validData = [
            "title" => "testブログタイトル",
            "body" => "testブログ本文",
        ];

        $response = $this->post(route("article.store"),$validData);
        $response->assertRedirect(route("article.show",Article::first()));

        $this->get(route("article.show",Article::first()))
            ->assertOk()
            ->assertSee("記事の作成に成功しました!");

        $this->assertDatabaseHas("articles",
            array_merge($validData, ["user_id" => $user->id])
        );
    }

これで一回テストに失敗することを確認

その上で、storeメソッドを以下の通りにしてみた

    public function store(StoreArticleRequest $request)
    {
        $this->authorize('store',Article::class);
        $article = ""; // スコープ対策
        DB::transaction(function () use ($request, &$article) {
            $data = $request->only(["title", "body"]);
            $data["user_id"] = auth()->id();

            $article = Article::create($data);

            // タグの処理
            $tagNames = explode(",", $request->tags);
            $article->addTags($tagNames);
        });

        return redirect()->route("article.show",$article)
            ->with("success","記事の作成に成功しました!");
    }

トランザクション処理の追加と、記事の追加をcreateメソッドにしたのと、タグの処理をモデルのメソッドにしたのと、リダイレクト先・メッセージの追加をした。

一応、タグメソッドは以下の通り。

    // Tagを追加するメソッド
    public function addTags($tagNames)
    {
        foreach ($tagNames as $tagName) {
            $tagName = trim($tagName);
            $tag = Tag::firstOrCreate(["name" => $tagName]);
            $this->tags()->attach($tag);
        }
    }

とりあえずここも一回これくらいで。
テストケースもおk。

AudioController_storeのリファクタリング

とりあえず、コントローラ最後のリファクタリング。
このメソッドは音源を保存するメソッドなんだけど、音源の管理でちょっとした処理とかが重なってかなり肥大化している。

早速中身を見ていく

    public function store(StoreAudioRequest $request)
    {
        $this->authorize('store',Audio::class);
        $path = ''; // スコープ対策

        DB::transaction(function () use ($request, &$path) {
            // アップロードとpathの取得
            $file = $request->file("audio");
            $user = \Auth::user();

            // ユーザーが指定した名前をoriginalFilenameに
            $originalFilename = $request->filename;
            // 拡張子を取得し、保存する際の名前を「指定した名前_一意ID.拡張子」にしている。
            $extension = $request->file('audio')->getClientOriginalExtension();
            $filename = $originalFilename . '_' . uniqid() . '.' . $extension;

            $path = $file->storeAs('uploads/audio', $filename, 'public');

            // データベースにパスを保存
            $audio = new Audio();
            $audio->path = $path;
            $audio->filename = $originalFilename;
            $audio->user_id = $user->id;
            $audio -> save();
        });

        // 成功メッセージとURLをセッションに格納
        return redirect('/audio/create')
        ->with('success', '音源のアップロードに成功しました!')
        ->with('audio_url', asset('storage/' . $path));
    }

うーん、流石に長いなぁと思う。

DBの保存と前処理らへんはもう少し別の書き方があるかなぁ。

サービスクラスを導入するかを考える

単語だけは聞いたことあるけど、使ったことのないサービスクラスを調べてみる。

サービスクラスはLaravelにある機能ではなく、MVCっぽいwebアプリを構築する上でよく使われる手法という感じ。
そのため、Laravelのドキュメントはない。

色々調べて為になったのは「サービスクラスは何故存在するのか」とか「Laravelで❝サービスクラス❞を排除したクリーンな設計」とか。
つまり、何が言いたいのかというと「設計は十人十色で、サービスクラスには賛否がある」ということ。

今回の場合、私一人ではサービスクラスの導入が必要かがわからなかったので、とりあえず導入せずに出来るだけリファクタリングしてみた。

修正後は以下の通り

    public function store(StoreAudioRequest $request)
    {
        $this->authorize('store',Audio::class);

        $filePath = DB::transaction(function () use ($request) {
            $audioFile = $request->file("audio");
            $originalFilename = $request->filename;

            // 拡張子を取得し一意の名前に
            $uniqueFilename = $originalFilename . '_' . uniqid() . '.' . $audioFile ->getClientOriginalExtension();
            $filePath = 'uploads/audio/'.$uniqueFilename;

            // ディスクのロールバックは出来ないので、先にDB保存。
            Audio::storeAudioInfo($filePath, $request->filename);
            // ディスクに保存
            $audioFile->storeAs('uploads/audio', $uniqueFilename, 'public');
            return $filePath;
        });

        // 成功メッセージとURLをセッションに格納
        return redirect('/audio/create')
            ->with('success', '音源のアップロードに成功しました!')
            ->with('audio_url', asset('storage/' . $filePath));
    }

変更したのは

  1. 変数名
    そもそも変数名がちょっと分かり難かったので変更した。
  2. コードの順番
    ディスクに保存された後にエラーが起きてもディスクの中身はロールバックされなかったので、コードの順番を変更。
  3. 若干の圧縮
    uniqueFileNameの生成とかを少し圧縮した。
  4. モデルに保存メソッドを追加
    Audio::storeAudioInfoで呼び出している。
    恥ずかしながら、ここで初めてstaticと普通の関数の違いを理解した。

前よりは分かりやすくなったのかなと思う。

Compact()を適用する

Compact()はPHPの標準機能で、変数名から連想配列を作ってくれる機能。

だから、
        return view(“article.show”,[“article”=>$article]);

        return view(“article.show”,compact(“article”));
と書き換えられる。

個人的に可読性が上がるのと、複数の変数を入れるときも

        return view(“article.show”,compact(“article”,”message”));

みたいな感じで短くなるので、これに変えていく。

とりあえず、1回目のリファクタリングはこんなもんで終わる。

おわりに、クラウド環境に向けて

テストとリファクタリングに結構時間かかっちゃった。

次はクラウド環境にこのLaravelプロジェクトを移行させたいと思うんだけど、思ったより記事が長くなっているので、一回区切る。

終わり、おやすみなさい。