feat: #30 add function "pick", "omit" #31
Conversation
🦋 Changeset detectedLatest commit: a948760 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Walkthroughこの変更では、 Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files ignored due to filter (2)
- package.json
- yarn.lock
Files selected for processing (13)
- README.md (6 hunks)
- src/index.ts (1 hunks)
- src/omit.ts (1 hunks)
- src/pick.ts (1 hunks)
- src/twist.ts (1 hunks)
- src/type.ts (5 hunks)
- src/unfold.ts (4 hunks)
- src/utils.ts (1 hunks)
- test/fold.spec.ts (6 hunks)
- test/omit.spec.ts (1 hunks)
- test/pick.spec.ts (1 hunks)
- test/twist.spec.ts (3 hunks)
- test/unfold.spec.ts (5 hunks)
Files skipped from review due to trivial changes (6)
- src/index.ts
- src/unfold.ts
- test/fold.spec.ts
- test/omit.spec.ts
- test/pick.spec.ts
- test/unfold.spec.ts
Additional comments (Suppressed): 15
src/twist.ts (2)
10-13:
MoveMapの型がM extends MoveMap<D>に変更されています。この変更が意図したものであること、そしてこれがコードベース全体で適切に反映されていることを確認してください。1-2:
foldとunfoldのインポートパスが変更されました。新しいパスが正しいことを確認してください。README.md (3)
11-15: 新しい機能の説明が削除されています。
pickとomit関数についての説明を追加することを検討してください。88-126: 新たな関数
pickとomitの使用例が追加されました。これらは、指定したキーをJSONオブジェクトから選択または削除する機能を提供します。153-180: 新たなオプション
pruneArrayの使用例が追加されました。このオプションは、配列要素を削除するかどうかを制御します。src/utils.ts (1)
- 1-30:
includesKey関数は、指定したキーが元のキーに含まれているかどうかをチェックします。この関数は、配列インデックスを扱うためのオプションを受け取り、キーがブラケット形式で表現されている場合にそれをドット形式に変換します。その後、キーを分割し、元のキーと比較します。これは、キーのネストをフラットにするための効率的な方法です。ただし、この関数は、キーが部分文字列として元のキーに存在する場合でも
falseを返す可能性があります。例えば、includesKey("foo.bar.baz", "ba", { arrayIndex: "bracket" })はfalseを返します。これは意図した動作であるか確認してください。また、この関数は大量のデータに対して実行される場合、パフォーマンスに影響を及ぼす可能性があります。特に、
split関数が2回呼び出され、それぞれの呼び出しで新しい配列が生成されます。これらの配列は、最終的にはGCによってクリーンアップされますが、大量のデータに対してこの関数が頻繁に呼び出される場合、メモリ使用量とGCの負荷が増加する可能性があります。この問題を改善するためには、split関数の呼び出しを1回に減らすか、または配列の代わりにイテレータを使用することを検討してみてください。test/twist.spec.ts (5)
113-150: 新しいテストケースが追加されています。このテストは、ルートレベルで数値と文字列のキーを持つオブジェクトを処理する能力を検証します。特定のキーを変換しながら、元のオブジェクト構造の残りの部分を保持することを確認します。
152-193: 新しいテストケースが追加されています。このテストは、ルートレベルで数値と文字列のキーを持つオブジェクトを処理する能力を検証しますが、配列インデックスにドット記法を使用します。特定のキーを変換しながら、元のオブジェクト構造の残りの部分を保持することを確認します。
195-212: 新しいテストケースが追加されています。このテストは、
pruneArrayオプションがtrueに設定されている場合に、配列要素が削除されることを検証します。特定のキーを変換しながら、元のオブジェクト構造の残りの部分を保持することを確認します。214-230: 新しいテストケースが追加されています。このテストは、
pruneArrayオプションがfalseに設定されている場合に、配列要素が削除されないことを検証します。特定のキーを変換しながら、元のオブジェクト構造の残りの部分を保持することを確認します。239-250: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [233-248]
新しいテストケースが追加されています。このテストは、第二引数が空のオブジェクトである場合、
twist関数が元のオブジェクトをそのまま返すことを検証します。src/type.ts (4)
12-23: 新たに
DeepKeyOf型が追加されましたが、現在は単純な文字列として定義されています。深くネストしたキーをサポートするためのロジックがまだ実装されていないようです。このTODOコメントを確認し、必要なロジックを追加してください。30-33:
MoveMap型が更新され、DeepKeyOf<D>を使用するようになりました。これにより、深くネストしたキーをサポートできるようになります。ただし、DeepKeyOfの実装が完了していないため、この変更が期待通りに機能するか確認してください。92-113:
UnfoldOptionインターフェースに新たにpruneArrayオプションが追加されました。これにより、配列が空の場合に削除するかどうかを制御できます。この新しいオプションが正しく動作するかテストしてください。125-141:
TwistOption,OmitOption,PickOptionインターフェースが追加され、それぞれがUnfoldOptionを拡張しています。これにより、これらのオプションは新たに追加されたpruneArrayオプションも利用できます。これらの新しいインターフェースが期待通りに機能するか確認してください。
| export function pick<D extends Dictionary, K extends DeepKeyOf<D>>( | ||
| obj: D, | ||
| keys: K[], | ||
| opt?: PickOption | ||
| ): Omit<D, K> { | ||
| const folded = fold(obj) | ||
|
|
||
| const targetKeys = new Set( | ||
| Object.keys(folded).filter((k) => keys.some((key) => includesKey(key, k, opt))) | ||
| ) | ||
|
|
||
| const fixedKeyMap = Object.fromEntries( | ||
| Object.entries(folded).filter(([k]) => targetKeys.has(k)) | ||
| ) as Folded<Dictionary> | ||
|
|
||
| return unfold(fixedKeyMap, opt) as Dictionary | ||
| } |
There was a problem hiding this comment.
pick関数の実装は、指定されたキーを持つオブジェクトを正しく生成するように見えます。しかし、パフォーマンスの観点から、行41でkeys.some()を使用している部分が問題となります。このコードは、各キーに対して全てのターゲットキーを走査し、O(n^2)の時間複雑度を持ちます。これを改善するために、keysをSetに変換し、その存在チェックをO(1)の操作にすることを検討してみてください。
- const targetKeys = new Set(
- Object.keys(folded).filter((k) => keys.some((key) => includesKey(key, k, opt)))
- )
+ const keySet = new Set(keys);
+ const targetKeys = new Set(
+ Object.keys(folded).filter((k) => keySet.has(k))
+ )There was a problem hiding this comment.
includesKey は文字列の一致比較ではないため、 keySet.has とは判定結果が異なります。
There was a problem hiding this comment.
@mew-ton ご指摘いただきありがとうございます。includesKey関数が単純な文字列の一致比較ではないという点は見落としていました。そのため、提案した修正は適切ではありませんでした。
しかし、パフォーマンスの観点からは依然として改善の余地があります。keys.some()の使用は時間複雑度をO(n^2)にするため、この部分を最適化することを検討してみてください。
具体的には、includesKey関数の結果をキャッシュすることで、同じキーに対する重複した計算を避けることができます。以下に示すように、Mapを使用してキーとそれに対応するincludesKeyの結果を保存し、再利用することができます。
+ const includesKeyCache = new Map();
const targetKeys = new Set(
- Object.keys(folded).filter((k) => keys.some((key) => includesKey(key, k, opt)))
+ Object.keys(folded).filter((k) => {
+ if (!includesKeyCache.has(k)) {
+ includesKeyCache.set(k, keys.some((key) => includesKey(key, k, opt)));
+ }
+ return includesKeyCache.get(k);
+ })
)この変更により、各キーに対するincludesKeyの計算が最大で1回しか行われないため、パフォーマンスが向上する可能性があります。ただし、キャッシュの使用はメモリ使用量を増加させる可能性があるため、そのトレードオフを考慮に入れてください。
| import { fold } from './fold' | ||
| import { unfold } from './unfold' | ||
| import { includesKey } from './utils' | ||
| import type { Dictionary, DeepKeyOf, OmitOption, Omit, Folded } from './type' | ||
|
|
||
| /** | ||
| * Returns an object with the specified keys removed from the object. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * const obj = { | ||
| * a: 1, | ||
| * b: { | ||
| * c: 2, | ||
| * d: [3, 4] | ||
| * } | ||
| * } | ||
| * | ||
| * const omitted = omit(obj, ['a', 'b.c']) | ||
| * // omitted is | ||
| * // { | ||
| * // b: { | ||
| * // d: [3, 4] | ||
| * // } | ||
| * // } | ||
| * ``` | ||
| * | ||
| * @param obj | ||
| * @param keys | ||
| * @param opt | ||
| */ | ||
| export function omit<D extends Dictionary, K extends DeepKeyOf<D>>( | ||
| obj: D, | ||
| keys: K[], | ||
| opt?: OmitOption | ||
| ): Omit<D, K> { | ||
| const folded = fold(obj) | ||
|
|
||
| const targetKeys = new Set( | ||
| Object.keys(folded).filter((k) => !keys.some((key) => includesKey(key, k, opt))) | ||
| ) | ||
|
|
||
| const fixedKeyMap = Object.fromEntries( | ||
| Object.entries(folded).filter(([k]) => targetKeys.has(k)) | ||
| ) as Folded<Dictionary> | ||
|
|
||
| return unfold(fixedKeyMap, opt) as Dictionary | ||
| } |
There was a problem hiding this comment.
omit関数の実装は、指定されたキーをオブジェクトから削除するという目的に適しています。この関数は、まずオブジェクトをフラットな表現に変換し(fold関数)、次に指定したキーをフィルタリングし、最後に元の構造に戻します(unfold関数)。これらのステップは効率的であり、コードの可読性も高いです。
ただし、一部のパフォーマンス改善が可能です。特に、keys.some((key) => includesKey(key, k, opt))の部分では、各キーに対してincludesKey関数が呼び出されています。これは、大きなキー配列に対しては非効率的になる可能性があります。代わりに、キーをSetに変換し、その存在をチェックすることで、パフォーマンスを向上させることができます。
以下に修正案を示します：
export function omit<D extends Dictionary, K extends DeepKeyOf<D>>(
obj: D,
keys: K[],
opt?: OmitOption
): Omit<D, K> {
const folded = fold(obj)
+ const keySet = new Set(keys);
- const targetKeys = new Set(
- Object.keys(folded).filter((k) => !keys.some((key) => includesKey(key, k, opt)))
- )
+ const targetKeys = new Set(
+ Object.keys(folded).filter((k) => !keySet.has(k))
+ )
const fixedKeyMap = Object.fromEntries(
Object.entries(folded).filter(([k]) => targetKeys.has(k))
) as Folded<Dictionary>
return unfold(fixedKeyMap, opt) as Dictionary
}
Summary by CodeRabbit
リリースノート:
pick関数を追加しました。指定したキーを持つ新しいオブジェクトを生成します。omit関数を追加しました。指定したキーを除去した新しいオブジェクトを生成します。unfold,twist,pick,omit関数にpruneArrayオプションを追加しました。配列要素の削除を制御できます。omitとpick関数のテストスイートを追加しました。unfold関数のdefaultUnfoldOptionの使用を更新しました。twist関数のシグネチャを更新し、ジェネリック型パラメータMを追加しました。