じろう

2019年11月02日に参加

学習履歴詳細

[Laravel]昨日のエラーを解決。Articleコントローラのリファクタリング。画像の処理を切り出し

今日のYWT

Laravel

Travelog

昨日のエラー解決

ここを参考に、npm rebuild node-modulesをしたあと、npm run watch-pollで解決。
prettierを入れたときに関連パッケージをアップデートしたのが原因か?

ArticleControllerのリファクタリング

画像の情報をデータベースに保存する処理をリポジトリに切り出す。
コントローラにあった以下のコードをRepositoryに切り出し、コントローラでは呼び出して使うようにする。

$article->photos()->create([
    'name' => $filename,
    'storage_key' => $filepath,
]);

PhotoRepository.php
php
/**
* 画像をs3にアップロードしたあと、その画像の名前と保存先をデータベースに保存する処理
*
* @param string $filename
* @param string $filepath
* @param Article $article
* @see PhotoUploadRepository::upload
*/
public function store($filename, $filepath, $article)
{
$article->photos()->create([
'name' => $filename,
'storage_key' => $filepath,
]);
}

ArticleController.php
```php
/**
* Articleをデータベースに登録する
* 画像がアップロードされていたら画像も登録する
*
* @param \App\Http\Requests\ArticleRequest $request
* @param \App\Article $article
*
* @return Illuminate\Http\RedirectResponse
*/
public function store(ArticleRequest $request, Article $article)
{
$article->fill($request->all());

// 注意:ここの$request->user()はリレーションメソッドの呼び出しではなく、Requestクラスのインスタンス(ここでは$request)が持っているメソッドで、認証済みユーザーのインスタンスを返している
$article->user_id = $request->user()->id;

$article->save();

// 画像アップロード
if ($request->file('files')) {
    foreach ($request->file('files') as $index => $e) {
        // 配列をそのまま受け取って、それぞれの変数に格納するlist
        [$filename, $filepath] = $this->photoUploadRepo->upload(
            $e['photo'],
        );

        $this->photoRepo->store($filename, $filepath, $article);
    }
}

// タグの追加
// $requestからタグの情報を一つずつ取り出す
// 無名関数の中で$articleを使うため、use ($article)とする
$request->tags->each(function ($tagName) use ($article) {
    $tag = Tag::firstOrCreate(['name' => $tagName]);
    $article->tags()->attach($tag);
});

return redirect()->route('articles.index');

}
```

わかったこと

切り出したときのデータの受け渡し

最初はPhotoRepositorystoreに、$articleを渡していなかった。
なので、 undefined method error $articleが出ていた。
よくよく考えてみれば、中で$articleを使っているので、引数で$articleも受け取る必要があった。

public function store($filename, $filepath)
{
    $article->photos()->create([
        'name' => $filename,
        'storage_key' => $filepath,
    ]);
}

名前の付け方とファイルの分け方

上のstoreアクションの名前は、最初photoCreateだった。
$article->photo()->createでレコードを新規作成しているし、createを使っているからいいかなと思ったが、メンターさんから

あとメソッド名もcreatePhotoだと、まるで写真を生成してるみたいな感じで、呼び出し側からすると違和感がありますね!
repositoryに切り出すことで、controllerからすると、dbにレコードをcreateするのは知らなくて良い情報なのです。

コントローラからするとdbにレコードを createするのは知らなくて良い情報という説明でこの命名が不適切である、と感覚は掴めた。

また、このメソッドは、すでに作っていたPhotoUploadRepositoryの中に書いたが、このメソッドがPhotoUploadをするわけではないので、それも直したほうがいいとのことだった。

次やること

  • リファクタリングできそうなら
  • エラーページの実装
Laravel

2020年11月24日(火)

2.9時間