# 2021-08-12 メンターセッション

## 質問

### 特大課題のレビューをお願いします。（by 永井）
- コントローラー層
- privateでメソッドを定義している。
- この場合、凝集度を計算するエレコム数の数値が悪化してしまう
- 使うところでconst定義すべき
- エンドポイント
- ./remove/:idにすると良い

- コマンドクエリ
- 取得して画面に見せるだけのものは最初からqueryServiceにすると、要件が増えたときに対応しやすい。
- 画面に見せるだけならクエリサービスにすると良い。

- 神DTOになりそう。参照が多い
- DTOは用途ごとに特化すべき

- N+1問題
- inなどのクエリを使って工夫すると良い
-

- ユースケースにドメインのルールが漏れている
- addParticipantInPairUsecase
- 何人だったら〜のようなロジックをaddParticipantに全部入れてあげるとよかったかも

- マジックナンバーがある

- 仕様クラス
- バリデーション目的でドメインサービスにするんじゃなくて仕様クラスにすると名前が明確

- ペアを移動させるとき
- removeさせてからadd？
- そうした場合、removeだけ成功したりしたらデータの不整合が起きるかも
- トランザクションとして削除と追加が同時に起きることを担保したほうがいいかも
- エンドポイントを１つにして、一つのトランザクションでremove,addする。

- テストのためのダミーデータ
- 一つのダミーデータが色々なところから参照されているため、テストとテストの間に依存関係が出来てしまう
- テストケースの中で使うデータは独立させたほうが良い（1テスト1データ）
- 別のテストから参照されないようにするために、テストデータの中で作ってしまってもよいかも
- pairRepository createのテスト
- `test`のなかで`expect`している内容（何をテストしているのか？）を意識するとよいかも
- テストケース名で明示するとわかりやすいかも（作成できる、だと抽象的かも）

- pairRepository updateのテスト
- ORMのテストみたいになっているかも？（かなり細かい）
- テストを仕様書のようにしたかった
- ドメイン層を仕様のようにするほうがコスパよいかも
- テストが落ちてしまう原因になりそうな要素が多い
- addが落ちたらupdateは落ちる
- removeが落ちたらupdateは落ちる
- updateのせいじゃないのにupdateがコケてしまう
- findAllも同様

- pairRepository updateの仕様
- participantを全員野良にしてからpairを再編成する処理にしている？
- participantとpairの両方にpairNameがいる？

- schemaに「集約」という言葉が入ってきているのが気になった
- schemaと集約が必ずしも対になるとは限らない

###### Tags: `Team-2`