Feat(utils): isSubset #309
Conversation
🦋 Changeset detectedLatest commit: 9308730 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 |
|
@beberiche 우선 작업해주셔서 정말 감사합니다 :) 이슈의 코멘트로 남긴 예제처럼 부분 집합 임을 결정하는건 const arr1 = [{ id: 1, name: 'mj'}, { id: 2 }, {id: 3}];
const arr2 = [{ id: 1}, { id: 2, name: 'gromit' }];
isSubset(arr1, arr2, (item) => item.id); // true
또한, 이렇게 될 경우 우리는 참조형일 때 굳이 만약, 깊은 중첩 객체 혹은 배열의 케이스에는 const arr1 = [{ id: 1, info: { address: 'seou' }}, { id: 2, info: { address: 'busan' }}];
const arr2 = [{ id: 2, info: { address: 'busan' }}];
isSubset(arr1, arr2, (item) => JSON.strinify(item));또는 iteratee의 반환값을 특정해서 지정해도 됩니다. ex: 제 의견을 정리하자면, 아래와 같습니다.
|
|
@ssi02014 |
|
@ssi02014 |
There was a problem hiding this comment.
여러 면에서 검토하느라 리뷰를 늦은 밤에 남기는 점 양해부탁드립니다. 🙏
기존에 있던 difference 함수를 재사용해서 구현하는 방향으로 수정되면 좋을 것 같습니다.
저는 이런 방향성을 위해 간단하지만 difference의 성능 개선 작업을 진행하였습니다. (즉, 최신 main 브랜치와 싱크를 맞춰주세요!)
- modern-kit의 difference는 lodash의 같은 기능인 differenceBy보다 약
1.8~2배가량 빠릅니다.
즉, 아래와 같이 변경을 요청드립니다 :)
const isSubset = <T, U>(
superset: T[] | readonly T[],
subset: T[] | readonly T[],
iteratee?: (item: T) => U
) => {
return difference(subset, superset, iteratee).length === 0;
};- 부분집합을 검증하는 의미에서
superset,subset으로 인자의 네이밍 변경 superset,subset의 타입을 다른 함수들과 컨벤션을 맞추기 위해T[] | readonly T[]로 변경difference재사용문서및테스트 코드 변수네이밍 변경
성능 테스트
실제로 제안주신 코드(isSubsetDefault)와 difference를 활용한 isSubset 코드(isSubsetWithDifference)의 성능 차이는 크게 없습니다.
- 성능 테스트 실행 할 때마다 결과가 다릅니다. 따라서 이정도 차이는 크게 고려하지 않아도 된다고 판단됩니다
| import { isSubset } from '.'; | ||
|
|
||
| describe('isSubset', () => { | ||
| it('if the childArray is a subset of the parentArray', () => { | ||
| const parentArray = [1, 2, 3, 4]; | ||
| const childArray1 = [1, 3]; | ||
| const childArray2 = [1, 5]; | ||
|
|
||
| expect(isSubset(parentArray, childArray1)).toBeTruthy(); | ||
| expect(isSubset(parentArray, childArray2)).toBeFalsy(); | ||
| }); | ||
|
|
||
| it('if the type is dfferent between comparison elements', () => { | ||
| const parentArray = ['1', 2, 3, 4]; | ||
| const childArray1 = ['1', 2, 3]; | ||
| const childArray2 = [1, '2', 3]; | ||
|
|
||
| expect(isSubset(parentArray, childArray1)).toBeTruthy(); | ||
| expect(isSubset(parentArray, childArray2)).toBeFalsy(); | ||
| expect(isSubset(parentArray, childArray2, (el) => Number(el))).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('if elements type is array', () => { | ||
| const parentArray = [ | ||
| [0, 1, 2, 3, 4], | ||
| [5, 6, 7, 8, 9], | ||
| ]; | ||
| const childArray = [[0, 1, 7, 4, 9]]; | ||
|
|
||
| expect(isSubset(parentArray, childArray)).toBeFalsy(); | ||
| expect(isSubset(parentArray, childArray, (obj) => obj[2])).toBeTruthy(); // [2,7], [7]; | ||
| expect(isSubset(parentArray, childArray, (obj) => obj[3])).toBeFalsy(); // [3,8], [4] | ||
| }); | ||
|
|
||
| it('if elements type is reference', () => { | ||
| const parentArray = [ | ||
| { | ||
| name: 'Peter', | ||
| age: 13, | ||
| }, | ||
| { | ||
| name: 'Aimee', | ||
| age: 25, | ||
| }, | ||
| ]; | ||
|
|
||
| const childArray1 = [ | ||
| { | ||
| name: 'Aimee', | ||
| age: 25, | ||
| }, | ||
| ]; | ||
|
|
||
| const childArray2 = [ | ||
| { | ||
| name: 'Peter', | ||
| age: 15, | ||
| }, | ||
| ]; | ||
|
|
||
| expect(isSubset(parentArray, childArray1)).toBeFalsy(); | ||
| expect( | ||
| isSubset(parentArray, childArray1, (obj) => JSON.stringify(obj)) | ||
| ).toBeTruthy(); | ||
| expect( | ||
| isSubset(parentArray, childArray2, (obj) => JSON.stringify(obj)) | ||
| ).toBeFalsy(); | ||
| expect(isSubset(parentArray, childArray2, (obj) => obj.name)).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
- 함수 설명을
it()함수에 맞게 변경해주시면 감사드립니다. it과test는 동일한 기능을 하지만 이름이 서로 다르기 때문에 테스트 설명이 다르게 작성되어야 합니다.- 이런
it()에 들어갈 테스트 설명 작성이 어렵다면ChatGPT를 활용하시는 것을 추천드립니다 🤗
import { isSubset } from '.';
describe('isSubset', () => {
it('should correctly determine if the subset arrays are subsets of the superset array', () => {
// 네이밍 변경
const superset = [1, 2, 3, 4];
const subset1 = [1, 3];
const subset2 = [1, 5];
// ...
});
it('should return the correct result if the types are different between comparison elements', () => {
// ...
});
it('should handle elements of type array correctly', () => {
expect(isSubset(superset, subset1, (arr) => arr[2])).toBeTruthy(); // (*) iteratee인자를 arr로 변경해주세요.
// ...
});
it('should handle elements of type reference correctly', () => {
// ...
});
});
|
@ssi02014 |
Overview
Issue: #306
두번째 인자로 주어지는 배열의 모든 요소를 첫번째 인자의 배열이 완전히 포함하는지(부분집합) 에 대한 여부
boolean를 반환합니다.배열 요소의 타입이 참조형인 경우, 깊은 비교를 진행하며,
iteratee함수 인자를 정의하여 비교항목을 설정하는 것이 가능합니다.고민점 1. (배열요소가 참조형인 경우 부분집합의 정의)
예를들어,
[{1:1, 2:2}, {3:3, 4:4}]와[{2:2}]의 경우에 대해{2:2}는{1:1, 2:2}의 부분집합이라고 볼 수 있냐는 것 입니다.결과적으로 저는 부분집합이 아니라고 판단했습니다. 참조형 역시 여러 값이 모인 하나의 집합이라고 볼 수 있으며, 객체 입장으로 바라보았을 때
{2:2},{1:1, 2:2}는 전혀 다른 구조를 가지므로 부분집합 관계라 정의하기 어렵다고 해석했습니다.고민점 2. (시간복잡도)
참조형 요소의 속성과 값이 서로 일치하는지 확인하기 위해서는
깊은 비교가 필요합니다. 다만, 객체 내부의 객체, 또 내부의 객체 등 객체의 깊이가 깊을 수록, 시간복잡도는 한없이 증가한다는 문제가 있습니다.이에 대해 크게 2가지는 해결안을 모색했습니다.
O(N*M*d)현재 구현된
isSubset()함수는 깊이에 상관없이 참조형 요소의 모든 항목을 비교하는 형태로 로직을 구성했습니다.이미 라이브러리 내에
deepEqual()이 구현되어 있기도 했고, 이왕이면 대부분의 케이스를 반영하는 것이 라이브러리 구현에 적합하다고 판단했습니다.단, 두 해결책 모두 장단점이 명확하기 때문에 이 부분은 메인테이너님의 의견도 한번 들어본 후에 다시 방향성을 고민해보려고 합니다.
close #306
PR Checklist
Contributing Guide