-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(utils): chunk 유틸 함수 구현 #184
Merged
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8aa958e
feat: 정수, 자연수 타입 추가
Collection50 d815021
feat: chunk 유틸 함수 구현
Collection50 bf4b603
fix: chunk의 2번째 인수의 제네릭 타입 추가
Collection50 0599eb9
feat: 범자연수 타입인 WholeNumber 추가
Collection50 c48fcf0
fix: array.length, size가 0인 경우, size >= array.length인 경우의 early retur…
Collection50 229b129
test: 테스트 코드를 한글에서 영어로 변경
Collection50 338406f
fix: size가 NaN인지 확인하는 early return 구문 추가
Collection50 4fc45df
fix: size가 NaN인 경우 빈 배열을 반환하도록 변경
Collection50 471f39c
chore: Create loud-zebras-shave.md
ssi02014 b48d5c8
docs: chunk 유틸 함수의 문서 작성
Collection50 e61b7c0
fix: 불필요한 NaN 타입 제거
Collection50 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
"@modern-kit/types": minor | ||
"@modern-kit/utils": minor | ||
--- | ||
|
||
feat(utils): chunk 유틸 함수 추가 및 Interger, NaturalNumber WholeNumber 유틸 타입 추가 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export type Integer<T extends number> = `${T}` extends | ||
| `${string}.${string}` | ||
| 'NaN' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NaN이 정확한 타입 추론이 안된다면 제거해도 될 것 같기도 하네요 ! |
||
? never | ||
: T; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { Integer } from 'Integer'; | ||
|
||
export type NaturalNumber<T extends number> = Integer<T> extends never | ||
? never | ||
: `${T}` extends `-${string}` | '0' | ||
? never | ||
: T; | ||
Comment on lines
+3
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Integer, NaturalNumber 모두 멋진 타입이네요! |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { Integer } from 'Integer'; | ||
|
||
export type WholeNumber<T extends number> = Integer<T> extends never | ||
? never | ||
: `${T}` extends `-${string}` | ||
? never | ||
: T; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import { chunk } from '../chunk'; | ||
|
||
describe('chunk', () => { | ||
it('splits the array according to the second parameter', () => { | ||
const arr1 = [1, 2, 3, 4, 5, 6]; | ||
expect(chunk(arr1, 1)).toEqual([[1], [2], [3], [4], [5], [6]]); | ||
expect(chunk(arr1, 2)).toEqual([ | ||
[1, 2], | ||
[3, 4], | ||
[5, 6], | ||
]); | ||
|
||
const arr2 = [1, 2, 3, 4, 5]; | ||
expect(chunk(arr2, 3)).toEqual([ | ||
[1, 2, 3], | ||
[4, 5], | ||
]); | ||
}); | ||
|
||
it('returns an array of each element when the second parameter is 1', () => { | ||
const arr = [1, 2, 3, 4, 5, 6]; | ||
expect(chunk(arr, 1)).toEqual([[1], [2], [3], [4], [5], [6]]); | ||
}); | ||
|
||
it('returns an empty array when given an empty array', () => { | ||
const arr = [] as []; | ||
expect(chunk(arr, 3)).toEqual([]); | ||
}); | ||
|
||
it('returns the array as is if the chunk size is greater than the length of the array', () => { | ||
const arr = [1, 2]; | ||
expect(chunk(arr, 3)).toEqual([arr]); | ||
}); | ||
|
||
it('returns an empty array if the second parameter is 0', () => { | ||
const arr = [1, 2]; | ||
expect(chunk(arr, 0 as number)).toEqual([]); | ||
}); | ||
|
||
it('returns an empty array if the second parameter is NaN', () => { | ||
const arr = [1, 2]; | ||
expect(chunk(arr, NaN)).toEqual([]); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { NaturalNumber } from '@modern-kit/types'; | ||
|
||
export const chunk = <T, U extends number>( | ||
array: T[], | ||
size: NaturalNumber<U> | ||
): T[][] => { | ||
if (array.length === 0 || Number.isNaN(size) || size === 0) { | ||
return []; | ||
} | ||
|
||
if (size >= array.length) { | ||
return [array]; | ||
} | ||
|
||
return array.reduce((result, _, index) => { | ||
if (index % size === 0) { | ||
result.push(array.slice(index, index + size)); | ||
} | ||
return result; | ||
}, [] as T[][]); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
export * from './countOccurrencesInArray'; | ||
export * from './excludeElements'; | ||
export * from './chunk'; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
먼저 좋은 작업 감사합니다! 👍
한가지 궁금한점은 Integer에서 체크하고자하는 'NaN'이 제대로 체킹되시는지 궁금합니다!
저도 기존에 NaN을 타입레벨에서 정확하게 잡아내는게 불가능한 것으로 알고있었고, 실제로 TS Playground에서 실행해보니 에러 체킹이 안되는 것으로 보여서요! (제가 잘못 시도했거나 모르는 부분이 있는거라면 알려주시면 감사드리겠습니다! 😄 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위의 체킹이 제대로 이루어지지 않는 것이 맞다면, 함수 내에서 NaN에 대한 방어코드를 작성해두는 것이 좋을듯하여 의견드려봅니다! 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
완전 여담으로 위의 발상이 시작된 지점은 NatualNumber타입에 Infinity와 -Infinity가 존재하지 않아 생각했었습니다!
NaN, Inifinity, -Infinity 모두 number 타입이나 타입레벨에서 정확하게 찾아내는건 불가능한것으로 알고있어서요! 😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 확인하지 못한 부분이 있었네요 !
코드를 리뷰해주셔서 감사합니다 🙌
혹시
NaN
을 아래와 같이 사용하는 것은 어떻게 생각하시나요?Number.isNaN
은 인수를Number
타입으로 변환하지 않습니다.size
를U
타입으로 선언해두어서 괜찮을 것 같기도 합니다 !)array
를 반환하는것There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 사실을 알게되어 기쁩니다 !☺️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Collection50 1, 2번 의견 주신 부분에 동의합니다! 간단하게 생각했던 의견이었는데 더 좋은 방향으로 제시해주셔서 감사합니다! 👍👍👍
NaN, Infinity, -Infinity 에 대한 부분은 저도 현재는 이렇게 알고있으나 잘못 알고있는 부분일수도 있습니다 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Collection50 @Sangminnn 제가 지금 외부라 확인이 좀 늦었습니다! 위 의견들 모두 확인했고 너무 좋았는데요!
마지막으로
NaN
의 경우[array]
를 반환하는 의도가 있을까요?NaN이 넘어온다는게 적절한 케이스는 아니고
예외 케이스
라고 생각이 드는데요! 예외 케이스라는 의미에서빈 배열([])
을 반환하는게 낫지 않을까요?[array]
를 반환한다는 것은size가 array.length와 동일하거나 그 이상
이라는 뭔가 특정 케이스가 존재합니다.size가 0
(예외 케이스라고 생각한 이유는 size가 0이라면 사실 chunk를 사용 할 이유가 없기 때문)과 같이 뭔가예외 케이스
라 생각이들어서요! 그런 의미에서 빈 배열이 낫지않나 의견드려봅니다추가로 테스트 커버리지를 위해 NaN의 경우 테스트 코드가 추가되면 좋을 것 같습니다 !!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분의 경우 최초에는 작성자분이 의도하신 바가 있는 것으로 생각했었습니다만 말씀주신 것을 들어보면 정리가 필요할 수 있겠네요!
다시 생각해보면 처리 과정에서 특정 케이스를 어떻게 분류하고 처리하는지에 대해 차이가 있는것처럼 보여서 그런것같습니다.
현재는 NaN이라는 케이스와 0이 들어오는 케이스를 다르게 처리하고있으나 NaN의 케이스와 0이 들어오는 케이스 모두를 의도가 아닌 예상치 못하게 들어오는 케이스로 분류하게 된다면, 둘을 동일하게 처리하는게 더 자연스럽겠네요! 😄
저도 위에 남겨주신 의견 이외에는 작성해주신 부분은 다 너무 좋습니다! 고생많으셨습니다! 👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Collection50 @Sangminnn
맞습니다 저는
array.length 0
,size 0
과NaN
을 모두예외 케이스
로 묶고 동일하게 처리하면 어떨까 생각했습니다.위 3가지 케이스는 모두 chunk를 사용 할 적절한 케이스가 아니기 때문입니다..!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다 !!
변경된 사항 적용했습니다. (4fc45df)