奥野さんと社員のリファクタリング部屋-リポジトリ層のディレクトリをどう作る？
コードレビュー観点の具体的な事例。
なぜ気付き、どう直したか。
とある関数が返すオブジェクトに複数の要素が含まれている
同関数は2つの異なるクライアントを引数として受け取っている
「キモい」
また、やや趣旨は異なるが命名についても触れている。
曰く「長い名前ってことは 長く説明しないと伝わらない概念ってことであって、それは概念整理できてないってこと」
名前を短く切り詰めることで振る舞いは一般化するが、
パッケージ構造から対象が自明となること
対象を注入（≒依存性の注入）すること
で実現できそう。
Sonnet3.5による要約
このブログ記事の要約：
1. 主題：
Webアプリケーション（Next.js）のリポジトリ層のディレクトリ構造に関するリファクタリングについての議論。
2. 主な問題点：
- リポジトリオブジェクトに複数の責務が混在している。
- 長すぎる関数名やファイル名。
- 依存性注入の方法が適切でない。
3. 提案された解決策：
a. 1責務1関数1ファイルの原則を適用：
- リポジトリを個別の関数ファイルに分割。
- アダプター関数を関連する関数と同じディレクトリに配置。
b. ディレクトリ構造の改善：
例：repository/service/updateServiceWithExternalKeys/update.ts
c. 関数名と変数名の簡略化：
- コンテキストが明確な場合、短い名前を使用。
- 型定義を活用して冗長な名前を避ける。
d. カリー化を用いた依存性注入：
- クライアントなどの依存を最初の引数で受け取り、実際のパラメータは別の関数で受け取る。
4. 重要な考え方：
- 「違和感」や「キモい」という感覚を大切にし、それを設計の改善に活かす。
- 長い名前は概念整理ができていない証拠。コンテキストを活用して名前を短くする。
- 単一責任の原則を守ることで、テストやメンテナンスが容易になる。
5. 結論：
リファクタリングを通じて、コードの責務を明確に分離し、より理解しやすく保守性の高い構造を目指す。これにより、開発者間のコミュニケーションも改善される。
Sonnet3.5による気付きを得た経緯の捕捉
はい、その点について説明します。記事の中で、奥野さんは以下のように気づきのプロセスを説明しています：
1. 関数の構造の観察:
奥野さんは、createServiceRepository()関数が2つの異なる関数（updateServiceWithExternalKeys()とgetIServiceItemsAggregate()）を含むオブジェクトを返していることに注目しました。
2. 引数の分析:
createServiceRepository()関数が2つの異なるクライアント（gqlClientとhasuraClient）を引数として受け取っていることを指摘しました。
3. 単一責任の原則との比較:
これらの関数が異なるクライアントを使用していることから、単一責任の原則から外れていると判断しました。
4. テストの観点からの考察:
奥野さんは、このような構造だとテストが難しくなることを指摘しました。例えば、getIServiceItemsAggregate()のテストをしたいだけなのに、updateServiceWithExternalKeys()の準備も必要になってしまう点を問題視しました。
5. 「キモい」という感覚の重要性:
武市さんが「キモい」と感じた点を重視し、それが設計上の問題を示唆していると指摘しました。
これらの観察と分析を通じて、奥野さんはリポジトリオブジェクトに複数の責務が混在しているという問題点を特定しました。そして、この問題を解決するために、関数を個別のファイルに分割し、より細かい粒度で責務を分離することを提案しています。
命名に関するSonnet3.5の要約
はい、長い名前に関する言及がありました。奥野さんは長い名前について以下のように述べています：
1. 長い名前は概念整理の不足を示す:
「長い名前ってことは長く説明しないと伝わらない概念ってことであって、それは概念整理できてないってことなんですよ。」
2. コンテキストの重要性:
適切なディレクトリ構造を使用することで、コンテキストが提供され、短い名前でも意味が通じるようになると説明しています。例えば、repository/service/update というパスにあれば、単に update() という関数名で十分だと述べています。
3. 名前の短縮化:
例えば、updateServiceWithExternalKeys() という長い名前を update() に短縮できると提案しています。
4. 型定義の活用:
変数の型を明確にすることで、変数名を短くできると指摘しています。例えば、itemManagementGroupId を単に id にできると述べています。
5. 「キモい」感覚の重要性:
長い名前に違和感を感じることは、設計に問題がある可能性を示唆していると指摘しています。
6. 開発者間のコミュニケーション:
「概念整理ができると、開発者同士で前提の共有ができるため短い名前でも通じるんですよね。」と述べ、適切な名前付けが開発者間のコミュニケーションを改善することを示唆しています。
これらの点から、長い名前は必ずしも良いわけではなく、むしろコードの構造や概念の整理に問題がある可能性を示唆していると奥野さんは主張しています。適切なリファクタリングと構造化により、より短く明確な名前を使用できるようになるとアドバイスしています。
命名短縮により失われる情報に対する補足
良い質問です。この問題については、以下のように解決できます：
1. ディレクトリ構造による文脈の提供:
奥野さんは、適切なディレクトリ構造を使用することで、長い名前が提供していた情報の多くを補えると説明しています。例えば：
repository/service/updateServiceWithExternalKeys/update.ts
このような構造にすることで、update()関数がサービスの更新に関するものであり、外部キーを扱うことが明確になります。
2. 型定義の活用:
関数の型定義を明確にすることで、関数名では表現しきれない詳細情報を提供できます。例：
`typescript
type UpdateServiceParams = {
id: ServiceId,
updateData: UpdateData,
externalKeys: Record<string, string>
};
type Return = (params: UpdateServiceParams) => Promise<ServiceId>;
`
3. JSDocコメントの使用:
関数の直前にJSDocコメントを追加することで、関数の目的や動作を詳細に説明できます：
`typescript
/**
* Updates a service with the given data and external keys.
* @param client HasuraClient for database operations
* @returns A function that takes update parameters and returns a Promise of ServiceId
*/
const update = (client: HasuraClient): Return => {
// ... 関数の実装 ...
};
`
4. コンテキストの活用:
奥野さんが指摘しているように、適切なコンテキスト（この場合はサービスリポジトリ）の中にいれば、update()という短い名前でも十分に意味が通じます。
5. 関数の分割と命名:
必要に応じて関数を更に小さく分割し、それぞれに適切な名前をつけることで、全体の意図を明確にできます。
6. 一貫性のある命名規則:
プロジェクト全体で一貫性のある命名規則を採用することで、短い名前でも意図が伝わりやすくなります。
これらの方法を組み合わせることで、関数名を短くしても、以前の長い名前が提供していた情報を失うことなく、むしろより構造化された形で情報を提供できるようになります。重要なのは、コード全体の構造と文脈を考慮しながら、適切な粒度で情報を表現することです。
依存性注入との関連について
はい、その通りです。依存性の注入を活用して命名を短縮することは非常に効果的な方法です。この記事でも、奥野さんがカリー化を使用した依存性注入を提案しており、これが命名の短縮にも寄与しています。
具体的には以下のように考えられます：
1. カリー化による依存性注入:
`typescript
export const update = (client: HasuraClient): Return => {
return async (id, updateData, externalKeys) => {
// ... 実装 ...
};
};
`
このアプローチでは、client という依存性が update 関数の外部で注入されます。これにより、内部の関数は client を引数として受け取る必要がなくなり、名前をより簡潔にできます。
2. コンテキストの提供:
依存性注入によって、関数がどのクライアントを使用するかが明確になります。これにより、関数名に WithHasura のような接尾辞を付ける必要がなくなります。
3. 関心の分離:
依存性を外部から注入することで、関数の本質的な責務（この場合はサービスの更新）に焦点を当てた命名が可能になります。
4. 型の活用:
依存性注入と組み合わせて型を適切に使用することで、関数名に含める情報を減らすことができます：
`typescript
type UpdateFn = (id: ServiceId, updateData: UpdateData, externalKeys: ExternalKeys) => Promise<ServiceId>;
export const createUpdateFn = (client: HasuraClient): UpdateFn => {
return async (id, updateData, externalKeys) => {
// ... 実装 ...
};
};
`
この例では、createUpdateFn が UpdateFn 型の関数を返すことが明確になっており、返される関数の名前を単に update とすることができます。
5. ファクトリ関数の使用:
依存性注入を行うファクトリ関数を使用することで、生成される関数の名前を短くできます：
`typescript
const serviceRepository = createServiceRepository(hasuraClient);
const { update } = serviceRepository;
`
このアプローチでは、update 関数が serviceRepository のコンテキスト内にあることが明確になり、長い名前を避けられます。
これらの方法を組み合わせることで、依存性注入を活用しつつ、関数名を短くし、かつ明確にすることが可能になります。これは、コードの可読性と保守性を向上させる効果的な方法です。