fix: numberToHangulMixed 및 numberToHangul에 모든 number 처리 #323
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@gs18004 님 안녕하세요! 먼저 예외 처리를 추가해주셔서 정말 감사드립니다. 현재 작업이 매우 훌륭해 보이는데요, 혹시 괜찮으시다면 numberToHangulMixed 문서에도 조금 더 자세한 설명을 덧붙여주실 수 있을까요? 그러면 처음 사용하시는 분들도 테스트 코드를 보지 않고 바로 이해하실 수 있을 것 같아요. numberToHangulMixed 안내에는 단순히 “한글의 숫자 단위”라고만 되어 있는데, 이 표현이 조금 모호하게 느껴질 수 있을 것 같아요. 해당 문장도 함께 다듬어주시면 정말 감사하겠습니다! 제가 생각해본 예시는 아래와 같습니다. 그대로 사용하셔도 좋고, 자유롭게 변경해주셔도 좋습니다.
마지막으로, 기여해주셔서 다시 한 번 감사드립니다! 궁금하신 점 있으시면 언제든 말씀해 주세요. |
감사합니다 동규님! 문서도 수정해서 올려 보겠습니다. |
|
gs18004
changed the title
|
@okinawaa 말씀하신 부분 반영해놓았습니다. 혹시 더 바꿀 부분이 있을까요? |
|
@gs18004 제가 주말까지 리뷰하도록 할게요 너무 감사합니다. |
감사합니다! 좋은 주말 되세요 :) |
src/numberToHangul/numberToHangul.ts
Outdated
| } | ||
|
|
||
| return koreanParts.join(''); | ||
| return isNegative ? (options?.spacing ? '마이너스 ' + result : '마이너스' + result) : result; |
There was a problem hiding this comment.
마지막 return 문에서 삼항 연산자가 중첩되어 있어서 가독성이 다소 좋지 않을 수 있을 것 같아요.
중첩된 삼항 연산자는 한눈에 읽기 어려울 수 있기 때문에, 명확한 if 분기로 나누면 더 직관적으로 이해할 수 있을 것 같습니다.
예를 들어, 아래처럼 작성하면 가독성이 좋아질 것 같아요.
if (isNegative) {
result = options?.spacing ? `마이너스 ${result}` : `마이너스${result}`;
}
return result;이렇게 하면 조건에 따라 result를 수정하고, 마지막에 return을 단순화할 수 있어서 코드를 읽는 부담이 줄어들 것 같아요! 😊
어떻게 생각하시나요?
There was a problem hiding this comment.
좋은 것 같습니다! 아래 말씀주신 내용까지 모두 반영하여 새로 commit했습니다😄
src/numberToHangul/numberToHangul.ts
Outdated
| if (input === 0) { | ||
| return '영'; | ||
| } | ||
|
|
||
| const isNegative = input < 0; | ||
| if (isNegative) { | ||
| input = Math.abs(input); |
There was a problem hiding this comment.
파라미터(input)를 직접 변경하는 방식보다는, 새로운 변수를 만들어 사용하는 것이 더 명확하고 예측 가능한 코드가 될 것 같아요.
파라미터를 변경하면 함수 내부에서 원래 값이 유지되지 않아 코드 흐름을 따라가기 어려울 수 있고, 예기치 않은 부작용이 발생할 가능성이 있습니다.
예를 들어, 아래와 같이 absoluteInput을 사용하면 원본을 유지하면서 가독성을 높일 수 있을 것 같습니다.
const isNegative = input < 0;
const absoluteInput = Math.abs(input);
Overview
numberToHangulMixed및numberToHangul가 양의 정수 뿐만 아니라 소수, 음수, 무한대 또한 처리할 수 있게 수정하였습니다.PR Checklist