feat: 숫자로 된 금액을 국립국어원 규칙의 한글 읽기로 변환합니다. #125
Conversation
🦋 Changeset detectedLatest commit: a803e90 The changes in this PR will be included in the next version bump. 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -0,0 +1,24 @@ | |||
| --- | |||
| title: amountsToHangul | |||
There was a problem hiding this comment.
아래 이유에 따라서, amountToHangul 가 더 적합할 것 같은데, 함수 이름을 amountsToHangul라고 지으신 이유가 궁금해요!
amountToHangul: 단수 형태로, 하나의 특정 금액을 한국어 읽기로 변환하는 함수라는 의미를 전달합니다.
amountsToHangul: 복수 형태로, 여러 금액을 한국어 읽기로 변환하는 함수라는 의미를 전달합니다.
src/amountsToHangul.ts
Outdated
| } | ||
| result.push(HANGUL_NUMBERS[parseInt(str[str.length - 1])]); | ||
| return result.join(''); | ||
| } |
There was a problem hiding this comment.
src/amountsToHangul.ts
Outdated
| export function amountsToHangul(str: string) { | ||
| str = str.replace(/\..*$/, '') // 소수점 지원 안함 | ||
| .replace(/[^\d]+/g, ''); // , 표기 등 오류내지 않음 | ||
| if(str.length > 80) { |
There was a problem hiding this comment.
80이 magic number가 아닌, 상수로 선언해서 80이라는 숫자가 의미하는 역할에 대해서 표기해주는 것은 어떤가요?
src/amountsToHangul.ts
Outdated
| str = str.replace(/\..*$/, '') // 소수점 지원 안함 | ||
| .replace(/[^\d]+/g, ''); // , 표기 등 오류내지 않음 | ||
| if(str.length > 80) { | ||
| return undefined; |
There was a problem hiding this comment.
80이상의 단어를 지원하지 않는다면, 이유를 함께 에러를 던져주는것은 어떤가요?
amountesToHangul에서 undefined를 반환한다면, 왜 undefined를 반환하는지 의문이 들 것 같아요.
There was a problem hiding this comment.
상수화 하고, throw new Error 로 했습니다.
src/amountsToHangul.spec.ts
Outdated
| expect(amountsToHangul('15,201,100')).toEqual('일천오백이십만천백'); | ||
| expect(amountsToHangul('120,030원')).toEqual('일십이만삼십'); // 숫자 외 문자 무시 | ||
| expect(amountsToHangul('392.24')).toEqual('삼백구십이'); // 소수점 무시 | ||
| }); |
There was a problem hiding this comment.
expect(amountsToHangul('100000000')).toEqual('일억'); 위와 같은 경우, 일억 으로 기대하였는데 일억만 이라는 반환값이 와서 당황했어요.
일억만이 공식적으로 맞는 표현인지 궁금합니당!
There was a problem hiding this comment.
0 만 잔뜩 넣어보는 테스트를 안해봤네요;;; 수정했습니다.
src/amountsToHangul.ts
Outdated
| export const HANGUL_CARDINAL = ['', '십', '백', '천']; | ||
|
|
||
| // https://ko.dict.naver.com/#/correct/korean/info?seq=602 | ||
| // https://github.com/crucifyer/koreanCardinalOrdinal |
There was a problem hiding this comment.
이 주석이 하는 역할이 궁금해요!
현재 여기에 정의된 함수와 같은 역할을 하는것으로 보이는데, 유저에게 동일한 기능을하는데 많은 정보를 줄 필요가 있나 싶어서요!
There was a problem hiding this comment.
원본 프로젝트이니 혹시 문제 있을 때 피드백을 받을 수 있을 것 같아 표기했습니다.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #125 +/- ##
=======================================
Coverage ? 99.56%
=======================================
Files ? 14
Lines ? 230
Branches ? 51
=======================================
Hits ? 229
Misses ? 1
Partials ? 0 |
|
1.3.6 버전에서 반영되었어요. 정말 감사합니다!! |
Overview
숫자로 된 금액을 국립국어원 규칙의 한글 읽기로 변환합니다.
PR Checklist