mergeの実装, MergeTestの実装

[oyashiro846]

#11

array_mergeのラッパーとしてmergeを実装しました。
array_mergeでは可変長引数として配列を受け取れるように設計されていますが、mergeでは$modeとの共存のため、配列の配列として渡す方式にしました。

$mode === Mode::MODE_AUTOのとき

配列、連想配列の区別なく結合し、数字のキーはすべて番号が振りなおされ、文字列のキーは維持された状態でマージ結果を返します。

$mode === Mode::MODE_LISTのとき

配列、連想配列は、すべてのキーが数字として振りなおされ、配列としてマージ結果を返します。

$mode === Mode::MODE_ASSOCのとき

配列、連想配列は、キーを維持し重複したキーがある場合は上書きされ、連想配列としてマージ結果を返します。

[claude]

Claude finished @oyashiro846's task in 1m 0s —— View job

---

📋 コードレビュー結果

✅ 概要

merge() 関数の実装とテストコードの追加を確認しました。実装は堅牢で、テストカバレッジも十分です。承認可能です。

---

✅ 実装の確認

src/merge.php - 動作はすべて正しく実装されています:

1. MODE_LIST（リスト化）: Line 31-41

   • すべてのキーを無視して値を順序付けで再収集
   • テストで確認: testMergeWithNumberKeyAssocInputWithListMode() で非連番キーが 0-indexed に変換される

2. MODE_ASSOC（連想配列化）: Line 43-45

   • array_replace() を使用してキーを維持し、重複時は上書き
   • テストで確認: testMergeWithNumberKeyAssocInputWithAssocMode() で数字キーが保持される

3. MODE_AUTO（自動）: Line 47

   • array_merge() を委譲 → 正しい動作
     • 数字キーは再番号付け（0から連続）
     • 文字列キーは維持、重複時は上書き
   • テストで確認: testMergeWithStringKeyAssocInputWithAutoMode() で文字列キーが維持される

---

✅ テストカバレッジ（MergeTest.php）

8つのテストケースで十分に検証されています:

• 空配列入力（全モード）
• リスト入力（AUTO モード）
• 文字列キー入力（AUTO モード）
• 数字キー入力（LIST/ASSOC モード）
• 混合キー入力（全3モード） ← 複雑なケースをカバー

すべてのテストが CI で合格しています。

---

✅ コード品質

項目	状態
PHP 型ヒント	✅ 完全（PhpStan対応）
PHPStan 条件型	✅ 適切（Mode別の戻り型を正確に定義）
Namespace/宣言	✅ 正しい
ドキュメント	✅ 日本語で明確
エラーハンドリング	✅ 設計と一貫（フレームワーク内での信頼）

---

✅ 設定ファイル

composer.json - 正しく更新されています:

• Line 27: src/merge.php をオートロード一覧に追加
• 配置: アルファベット順（map_keys.php の後）

---

📝 細かい指摘（実装の問題ではありません）

1. Line 27: \count($inputs) <= 0 について

   • count() は常に >= 0 なので <= 0 は === 0 と同義
   • 明示性のため === 0 の方が一般的ですが、コード動作に問題なし

2. 入力型チェック について

   • 関数は $inputs が配列の配列であることを前提（他の関数と同じ設計）
   • 本プロジェクトではフレームワーク内での信頼原則により問題なし

---

🎯 最終判定

✅ マージ可能 - 実装、テスト、品質すべてが基準を満たしています。

---

[Javakky]

外側は list

[Javakky]

外側は常にリストなのでこの条件はおかしい

[oyashiro846]

他の関数からコピペしてきたやつなので修正しておきます

[Javakky]

挙動の差分について把握できていますか？
https://qiita.com/mpyw/items/578e1a14879e679d2141
仕様としてarray_mergeと差分があるのであれば、ドキュメンテーションコメントで説明があるといいです

[oyashiro846]

array_mergeは、数字キーに対して自動的にインデックスを振りなおす仕様になっているという理解です。
これを直にラップした場合、例えば一意なidとしてキーに数字を指定している連想配列的なリストのインデックスが再割り当てされてしまうので、MODE_ASSOCの場合はキーを維持するように処理を分けています。

挙動の差分については$modeの部分に簡単な説明を記載していますが、別段落で詳細に書いておきます。

[Javakky]

他の関数において、AUTOは導出された型に合わせる。という意味だったと思いますが、このコードだけを見るとmergeの仕様に丸投げしているだけのように見えます

[oyashiro846]

そうですね。
今の実装ではMODE_AUTOは、数字キーと文字列キーが共存したコレクションを返すフラグとしてしか機能していません。

check_modeで$modeを判定しない実装にした理由は、

1. $inputsの各リストをすべて判定する or $inputsを結合してから判定する -> 計算コストがかかる
2. 数字キーと文字列キーが共存したコレクションを作りたい場合がある
   ため、第3のフラグとして動作させた方が柔軟だと判断したからです。

ただ、他の関数とmodeの扱いの毛色が違うのは確かなので、MODE_AUTOの際は従来通りの判定を行い、数字キーと文字列キーのミックスは別のフラグ変数で処理を分けるなどの手が考えられると思います。