laravel_review

[pr-ueeda]

good

• MVCの各レイヤーの責務を踏まえた実装ができている。
• Enumを使用しロジック内にマジックナンバーを使わないようにしている。
• Modelのリレーションがコードでうまく表現できている。
• Readmeがとても簡潔でわかりやすく書かれている。

fight(細かいかもしれませんが、ご確認お願いします)

• returnの前に改行を入れるとさらに見やすくなるかと思います。

例 :

public function method()
{
    $var1 = 5;
    $var2 = 5;
    // ここに改行
    return $var1 + $var2;
}

• スペースが少し目立つ
  → 基本的に=や=>は前と後ろにスペース一個ずつがベストかと思います。

• =が揃いすぎている
  → おそらくですがコードの見やすさを意識して=を揃えて書いてくださったかと思います。見やすさを意識してコードを書くことはすごくいいことですがそれが裏目にでて逆に少し見にくいコードになってしまう場合もあります。

該当コード :
https://github.com/prcaoduc/backendkenshu3/blob/153cd8e025151a913b060c4d66d213fa63933e52/backend3/app/Http/Controllers/ArticleController.php#L26-L28

行を揃えて見やすくするという意味では以下のように1行がものすごく長くなってしまうパターン(エディターによっては目印になる線が引いてあったりします。)は特に有効かと思います。

例:

public function method(string $arg1, string $arg2, string $arg3, int $arg4, int $arg5, array $arg6): string
{
    ...
}

↓

public function method(
    string $arg1,
    string $arg2,
    string $arg3,
    int $arg4,
    int $arg5,
    array $arg6
): string {
    ...
}

• RequestがController内で記述されている
  → ここはRequestクラスを作成してその中で管理した方が良さそうです。1つのレイヤーで2つの責務を持たせるのは設計上あまりよくないのと、別のクラスに切り出して管理したほうがデバックやリファクタリング等が楽になるかと思います。

例:

class Request
{
    public function rules()
    {
        return [
            'title' => 'required'
        ];
    }
}

class Controller
{
    public function method(Request $request): View
    {
        // viewでキーがtitle設定されたformからinputで受け取る。この時Requestクラスで記述したrulesを使いバリデーションを行 
          う
        $title = $request->input('title');
    }
}

[pr-ueeda]

good

• Requestクラスに切り分けて処理が書かれている。
• コードがより綺麗になっています 👍
• テストも何をテストしたいのかが明確であり、わかりやすいコードで書かれている。

おすすめの書き方

テストコードなのですが、Mockeyを使用する時はエイリアスを定義してsetUpのなかでモック作成してあげると後々楽になります。

以下のような該当コード :
https://github.com/prcaoduc/backendkenshu3/blob/49f2c1b838ee3d26efc7d8da9d865dc9b346dc92/backend3/tests/Unit/Services/ImageServiceTest.php#L21

例 :

// as ~ でエイリアスを定義することができます。後のコードで`m`を記述すると`Mockery`が使用できます。
use Mockery as m;

class TestCode extends TestCase
{
    protected $upLoadedFile;

    protected function setUp(): void
    {
        parent::setUp();
        // `m`で`Mockery`を使用してモックを作成
        $this->upLoadedFile = m::mock(UploadedFile::class);
    }
}