Skip to content

Commit db9aec3

Browse files
authored
Merge pull request #2749 from AtCoder-NoviSteps/#2748
feat: Add crud for getMergedTasksMap (#2748)
2 parents 96ba237 + ae6fa67 commit db9aec3

File tree

6 files changed

+575
-3
lines changed

6 files changed

+575
-3
lines changed
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
# getMergedTasksMap のリファクタリング教訓
2+
3+
## 学習内容
4+
5+
### 1. **参照 vs コピー**
6+
7+
TypeScript(JavaScript)の `const newTask = task;` は参照をコピーするため、`newTask` を変更すると元の `task` も変更されます。
8+
9+
- **浅いコピー**: `const newTask = { ...task };`
10+
- **深いコピー**: `const newTask = JSON.parse(JSON.stringify(task));`
11+
12+
### 2. **TypeScript らしいコード書き方**
13+
14+
- `map()` で初期化: `new Map(tasks.map(task => [task.task_id, task]))`
15+
- ループではなく関数型メソッド (`filter()`, `map()`, `flatMap()`)
16+
- スプレッド演算子で Map をマージ: `new Map([...map1, ...map2])`
17+
18+
### 3. **`flatMap()` vs `map()`**
19+
20+
`flatMap()` は返した配列を1段階フラット化するため、条件付きの可変長結果に最適:
21+
22+
```typescript
23+
// flatMap で条件分岐を自然に表現
24+
.flatMap((pair) => {
25+
if (!task || !contestType) return [];
26+
return [createMergedTask(...)];
27+
});
28+
// 結果: 該当する要素だけが含まれる
29+
```
30+
31+
### 4. **読みやすさ > 1行でまとめる**
32+
33+
無理やり `return` や1行で書く必要はない:
34+
35+
- 複雑な条件は `if` 文で早期リターン
36+
- オブジェクト生成は `key``value` を分けて記述
37+
- 難しいロジックはヘルパー関数に抽出
38+
39+
### 5. **計算量の分析**
40+
41+
- 全体: **O(N + M)** (N: タスク数、M: ペア数)
42+
- `Map.has()`, `Map.get()`**O(1)** なのでループ内で複数回呼んでもOK
43+
44+
### 6. **ドキュメント化のポイント**
45+
46+
- 関数の目的と副作用を明確に
47+
- **計算量と根拠** を記載
48+
- **使用例**`@example` で示す
49+
- 戻り値の構造を詳しく説明
50+
51+
## コード例(改善版)
52+
53+
src/lib/services/tasks.ts を参照
54+
55+
## キーポイント
56+
57+
- ✅ 非破壊的な変更(スプレッド演算子)
58+
- ✅ 関数型パラダイム(`filter()`, `flatMap()` 使用)
59+
- ✅ 早期リターンで複雑さを減らす
60+
- ✅ ヘルパー関数で責任分離
61+
- ✅ 明確なドキュメント化
62+
63+
---
64+
65+
# createContestTaskPairKey のテスト設計教訓
66+
67+
## テスト作成で学んだこと
68+
69+
### 1. **ヘルパー関数で重複削減**
70+
71+
同じパターンのテストコードは **ヘルパー関数** に抽出:
72+
73+
```typescript
74+
// ❌ Before: 重複が多い
75+
const key1 = createTestKey(pair);
76+
const key2 = createTestKey(pair);
77+
expect(key1).toBe(key2);
78+
79+
// ✅ After: ヘルパー関数化
80+
const expectKeysToBeConsistent = (pair: TestPair): void => {
81+
const key1 = createTestKey(pair);
82+
const key2 = createTestKey(pair);
83+
expect(key1).toBe(key2);
84+
};
85+
expectKeysToBeConsistent(pair);
86+
```
87+
88+
### 2. **パラメタライズテスト(test.each)で 4 個 → 1 個に**
89+
90+
4 つの似たテストは `test.each()` で 1 つにまとめる:
91+
92+
```typescript
93+
// ❌ Before: 4 つのテスト関数
94+
test('expects empty contest_id to throw an error', () => { ... });
95+
test('expects empty task_id to throw an error', () => { ... });
96+
test('expects whitespace-only contest_id to throw an error', () => { ... });
97+
test('expects whitespace-only task_id to throw an error', () => { ... });
98+
99+
// ✅ After: 1 つのパラメタライズテスト
100+
test.each<[string, string, string]>([
101+
['', 'abc123_a', 'contestId must be a non-empty string'],
102+
[' ', 'abc123_a', 'contestId must be a non-empty string'],
103+
['abc123', '', 'taskId must be a non-empty string'],
104+
['abc123', ' ', 'taskId must be a non-empty string'],
105+
])('expects error when contest_id="%s" and task_id="%s"', (contestId, taskId, expectedError) => {
106+
expect(() => createContestTaskPairKey(contestId, taskId)).toThrow(expectedError);
107+
});
108+
```
109+
110+
### 3. **テストデータを集約して保守性向上**
111+
112+
テストデータを `pairs` オブジェクトで一元管理:
113+
114+
```typescript
115+
const pairs = {
116+
normal: [...], // 正常系ケース
117+
edge: [...], // エッジケース
118+
anomaly: [...], // 異常系ケース
119+
};
120+
```
121+
122+
### 4. **テストカバレッジの考え方**
123+
124+
- **正常系**: 期待通りに動くか
125+
- **エッジケース**: 空文字列、ホワイトスペース、長い文字列
126+
- **異常系**: 特殊文字、Unicode、改行、タブ
127+
- **キー検証**: フォーマット、一意性、可逆性
128+
129+
### 5. **べストプラクティス**
130+
131+
| 改善内容 | 効果 |
132+
| -------------------- | -------------------- |
133+
| ヘルパー関数化 | コード重複 -40% |
134+
| パラメタライズテスト | テスト関数数 削減 |
135+
| テストデータ集約 | 保守性向上 |
136+
| beforeEach で初期化 | テスト間の独立性確保 |
137+
138+
### 6. **コード レビュー フィードバック対応**
139+
140+
#### 指摘事項
141+
142+
| 項目 | 内容 | 対応 |
143+
| ------------------ | ------------------------------------ | ------------------ |
144+
| 弱い Assertion | `toContain()` では不正確 | `toBe()` に変更 |
145+
| 冗長テスト | O(n²) の全ペア比較テストは不要 | O(n) Set検証に統一 |
146+
| beforeEach削減 | イミュータブルデータの不要なコピー | 削除 |
147+
| 特殊文字カバレッジ | **デリミタ文字(コロン)が未テスト** | 3ケース追加 |
148+
149+
#### 学んだことと根拠
150+
151+
- **アサーション強度**: `toContain()` は部分一致で誤検知の可能性 → `toBe()` で完全一致を保証
152+
- **テスト効率**: 冗長な検証は実装と同じ複雑さ → 代表的パターンだけ実装
153+
- **パーサビリティ脆弱性**: デリミタ文字(`:`)が ID に含まれると `split(':')` で分割失敗 → **デリミタ自体のテストケースが必須**
154+
155+
#### 対応結果
156+
157+
- ✅ Assertion を 4 個改善(`toContain()``toBe()` 統一)
158+
- ✅ 冗長テスト 1 個削除(O(n²) → O(n))
159+
- ✅ コロン文字テスト 3 個追加(`contestId` のみ、`taskId` のみ、両方)
160+
-**テスト総数: 26 → 29 個**(全成功 ✅)
161+
162+
#### 重要な発見
163+
164+
**コロン文字は関数内で保存されるが、デリミタと同じため使用時に注意が必要**
165+
166+
```typescript
167+
// 実装例:コロンを含む ID
168+
const key = createContestTaskPairKey('abc:123', 'task_a');
169+
// 結果: "abc:123:task_a" (コロン3個)
170+
171+
// ⚠️ split(':') での分割に注意
172+
const [contestId, ...taskIdParts] = key.split(':');
173+
// contestId = 'abc', taskIdParts = ['123', 'task_a'] ❌ 失敗!
174+
```
175+
176+
**教訓**: デリミタ文字も含めてテストし、実装側で検証ルールを明確にすべき。
177+
178+
## テスト統計
179+
180+
- **総テスト数**: 29 個(全成功 ✅)
181+
- **パラメタライズテスト**: 2 グループ(合計 11 ケース)
182+
- **ヘルパー関数**: 5 個
183+
- **テストデータセット**: 3 グループ(normal, edge, anomaly)
184+
- **特殊文字カバレッジ**: パイプ 4 ケース + コロン 3 ケース + その他 8 ケース

src/lib/services/tasks.ts

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
import { default as db } from '$lib/server/database';
2+
3+
import { getContestTaskPairs } from '$lib/services/contest_task_pairs';
4+
5+
import { ContestType } from '$lib/types/contest';
6+
import type { Task, TaskGrade } from '$lib/types/task';
7+
import type {
8+
ContestTaskPair,
9+
ContestTaskPairKey,
10+
TaskMapByContestTaskPair,
11+
} from '$lib/types/contest_task_pair';
12+
213
import { classifyContest } from '$lib/utils/contest';
3-
import type { TaskGrade } from '$lib/types/task';
4-
import type { Task, Tasks } from '$lib/types/task';
14+
import { createContestTaskPairKey } from '$lib/utils/contest_task_pair';
515

616
// See:
717
// https://www.prisma.io/docs/concepts/components/prisma-client/filtering-and-sorting
@@ -11,6 +21,77 @@ export async function getTasks(): Promise<Task[]> {
1121
return tasks;
1222
}
1323

24+
/**
25+
* Fetches and merges tasks based on contest-task pairs.
26+
*
27+
* @returns A promise that resolves to a map of merged tasks keyed by contest-task pair.
28+
*
29+
* @note This function merges tasks with the same task_id but different contest_id
30+
* from the contest-task pairs table. It enriches existing tasks with
31+
* contest-specific information (contest_type, task_table_index, etc.).
32+
* @note Time Complexity: O(N + M)
33+
* - N: number of tasks from the database
34+
* - M: number of contest-task pairs
35+
* - Map operations (has, get, set) are O(1)
36+
* @example
37+
* const mergedTasksMap = await getMergedTasksMap();
38+
* const task = mergedTasksMap.get(createContestTaskPairKey('tessoku-book', 'typical90_s'));
39+
*/
40+
export async function getMergedTasksMap(): Promise<TaskMapByContestTaskPair> {
41+
const tasks = await getTasks();
42+
const contestTaskPairs = await getContestTaskPairs();
43+
44+
const baseTaskMap = new Map<ContestTaskPairKey, Task>(
45+
tasks.map((task) => [createContestTaskPairKey(task.contest_id, task.task_id), task]),
46+
);
47+
// Unique task_id in database
48+
const taskMap = new Map(tasks.map((task) => [task.task_id, task]));
49+
50+
// Filter task(s) only the same task_id but different contest_id
51+
const additionalTaskMap = contestTaskPairs
52+
.filter((pair) => !baseTaskMap.has(createContestTaskPairKey(pair.contestId, pair.taskId)))
53+
.flatMap((pair) => {
54+
const task = taskMap.get(pair.taskId);
55+
const contestType = classifyContest(pair.contestId);
56+
57+
if (!task || !contestType || !pair.taskTableIndex) {
58+
return [];
59+
}
60+
61+
return [createMergedTask(task, pair, contestType)];
62+
});
63+
64+
return new Map([...baseTaskMap, ...additionalTaskMap]);
65+
}
66+
67+
/**
68+
* Creates a merged task from the original task and contest-task pair.
69+
*
70+
* @param task The original task to be enriched with contest-specific information.
71+
* @param pair The contest-task pair containing contestId, taskTableIndex and taskId.
72+
* @param contestType The type of contest (e.g., ABC, ARC) derived from contest_id.
73+
* @returns A tuple [key, mergedTask] where:
74+
* - key: the unique identifier for the contestId:taskId pair
75+
* - mergedTask: the task with contest-specific fields updated
76+
*/
77+
function createMergedTask(
78+
task: Task,
79+
pair: ContestTaskPair,
80+
contestType: ContestType,
81+
): [ContestTaskPairKey, Task] {
82+
const key = createContestTaskPairKey(pair.contestId, pair.taskId);
83+
84+
const mergedTask: Task = {
85+
...task,
86+
contest_type: contestType,
87+
contest_id: pair.contestId,
88+
task_table_index: pair.taskTableIndex,
89+
title: task.title.replace(task.task_table_index, pair.taskTableIndex),
90+
};
91+
92+
return [key, mergedTask];
93+
}
94+
1495
/**
1596
* Fetches tasks with the specified task IDs.
1697
* @param selectedTaskIds - An array of task IDs to filter the tasks.

src/lib/types/contest_task_pair.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { ContestTaskPair as ContestTaskPairOrigin } from '@prisma/client';
22

3-
import type { TaskResult } from '$lib/types/task';
3+
import type { Task, TaskResult } from '$lib/types/task';
44

55
export type ContestTaskPair = ContestTaskPairOrigin;
66

@@ -20,4 +20,6 @@ export type ContestTaskPairUpdate = ContestTaskPairCreate;
2020
// For mapping and identification
2121
export type ContestTaskPairKey = `${string}:${string}`; // "contest_id:task_id"
2222

23+
export type TaskMapByContestTaskPair = Map<ContestTaskPairKey, Task>;
24+
2325
export type TaskResultMapByContestTaskPair = Map<ContestTaskPairKey, TaskResult>;

src/lib/types/task.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Import original enum as type.
22
import type { TaskGrade as TaskGradeOrigin } from '@prisma/client';
3+
import type { ContestType } from '$lib/types/contest';
34

45
export interface Task {
6+
contest_type?: ContestType;
57
contest_id: string;
68
task_table_index: string;
79
task_id: string;

src/lib/utils/contest_task_pair.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type { ContestTaskPairKey } from '$lib/types/contest_task_pair';
2+
3+
/**
4+
* Creates a unique key for a ContestTaskPair using contestId and taskId.
5+
* Throws an error if either argument is an empty string.
6+
*
7+
* @param contestId - The ID of the contest.
8+
* @param taskId - The ID of the task.
9+
*
10+
* @returns A string in the format "contestId:taskId".
11+
*
12+
* @throws Will throw an error if contestId or taskId is empty.
13+
*/
14+
export function createContestTaskPairKey(contestId: string, taskId: string): ContestTaskPairKey {
15+
if (!contestId || contestId.trim() === '') {
16+
throw new Error('contestId must be a non-empty string');
17+
}
18+
if (!taskId || taskId.trim() === '') {
19+
throw new Error('taskId must be a non-empty string');
20+
}
21+
22+
return `${contestId}:${taskId}`;
23+
}

0 commit comments

Comments
 (0)