[Bug]: amountToHangul 함수명이 괜찮은가요? #214
Comments
|
Related with : #204 (comment) |
|
@Collection50 논의에 큰 도움을 주실 것 같아서 멘션드립니다 🙇 |
|
조심스럽게 논의에 참여해봅니다 🙂 의견먼저, 저의 생각은 다음과 같아요. #204 에서 다뤄졌던 함수의 설명에 적합한 이름은 일관된 이름 짓기 규칙을 참고했을때 말그대로 수를 뜻하는 import hangul from 'hangul' // zod와 같은 방식으로도 제공
hangul.josa();
hangul.susa();
hangul.number()
hangul.num()
hangul.su(); 또한 해당 함수의 구현에도 개선이 이루어지면 좋을 것 같습니다. 이유그렇게 생각하는 이유는 다음과 같아요. 근거와 함수의 구현 내용이 적절치 못하다왜냐하면 문서에서 근거로 삼은 이 링크에서는 "한글 맞춤법 제 44항"을 근거로 삼고있지만, 아래와 같은 형태에서 금액이 모두 한글로 이루어진 경우만 지원하고 있어요. 그리고 "한글 맞춤법 제 44항"에서는 숫자를 한글로 표기하는 경우 '만' 단위로 띄어서 쓰지만 해당 함수에서는 붙여서 반환해 주고 있어요. 하여,
라는 함수가 되기 위해서는 다음과 같이 바뀌어야 해요. 개선안amountToHangul('15,201,100'); // '천오백이십만 천백'
amountToHangul('120,030원'); // '십이만 삼십'
amountToHangul('12345.6789'); // '만이천삼백사십오점육칠팔구'
amountToHangul(15_201_100); // '천오백이십만 천백''
amountToHangul('15,201,100',options); // '1200만 1100'
amountToHangul('120,030원',options); // '12만 30'
amountToHangul('12345.6789',options); // 이 부분은 더 탐색이 필요합니다
amountToHangul(15_201_100,options); // '1520만 1100'이렇게 바꾸면 근거의 규칙을 따를뿐만 아니라 책임을 줄여 보다 많은 곳에서 이 함수를 사용할 수 있을 것이라고 예상되어요 그리고 다음의 경우는 es-hangul에서 다룰지, 사용하는 곳에서 충분히 대응 가능한 부분인지를 논의하여 필요하다면
참고저도 숫자를 통화로 표현해야하는 경험이 더러있었어요. 당시에는 numToKorean이라는 라이브러리를 사용했는데요. 앞으로도 수를 다루는 함수들이 추가되거나 이 함수에 options로 추가될 경우가 많아보여서 참고 할 함수들을 남겨보아요.
|
|
number, num, su 로 함수이름을 지으면 간결해서 좋긴하지만, from(숫자), to(한글) 로 변한다는 것을 잘 나타낼 수 있을까요? 저는 amountToHangul이 인자와, 결과값을 함수명으로부터 유추할 수 있다고 생각하�고 amount는 굳이 필요없을 것 같아서. numberToHangul와 같은 함수명이면 좋을 것 같다고 생각합니다. 하지만, 위에서 말씀해주신대로 44항을 제대로 반영하고 있지도 못하고, 전체를 한글로 반환해야할 수도 있고 숫자는 남기고 단위만 한글로 반환해야할수도 있고 하기때문에 options로 여러 요구사항을 반영하는것에 찬성합니당 |
|
이게 susa나, date(#217) 와 같은 함수들과 혼동이 없었으면 합니다 왜냐하면 숫자를 한글로 바꾼다는것은 너무나 넓은 의미의 함수라고 생각했어요! const result = numberToHangul(1234)
1. expect(result).toBe('일이삼사')
2. expect(result).toBe('천이백삼십사')
// 1,2 무엇도 틀리거나 맞지 않음 |
먼저,
라는 생각이 들 것 같아서 새로운 함수명 보다는 아래의 다소 실험적인 제안을 드려보려 합니다 🙂 Case 1: 커링 방식으로 numberToHangul만 export 하며 내부적으로 분리된 함수를 호출함수의 첫 번째 호출에서 주요 설정을 분리하고 두 번째 호출에서 실제 데이터를 처리함으로써 로직이 명확하게 분리됩니다. 이를 통해, 함수의 목적과 설정을 명확히 드러낼 수 있습니다. 이 방식은 함수명이 변경되지 않더라도 의도를 분명하게 전달할 수 있습니다. const result = numberToHangul('KRW')(12_340_000);
expect(result).toBe('일금 일천이백삼십사만원정')Case 2: numberToHangul은 내부적으로만 사용하고 외부에는 래핑한 함수만을 export다만, 생각보다 많은 개발자에게 커링은 다소 생소하거나 복잡하게 느껴질 수 있습니다. 직관적이고 사용하기 쉬운 API를 만드는 것이 중요하다면 아래의 방식도 좋을 것 같습니다. export const numberToKRW = () =>{
// 내부에서 numberToHangul에 의존
}
const result = numberToKRW(12_340_000);
expect(result).toBe('일금 일천이백삼십사만원정');하지만, 각 함수가 독립적으로 무척이나 유익한 issue 생성과 의견주심에 감사드립니다. 🙌 |
|
두 분 말씀에 덧붙여 제 생각 남겨드립니다 !!
타 라이브러리에서 제공하는 기능이라면 #176 에서 나누었던 것처럼 동일한 기능이라면, 현재 이슈의 함수명 고민도 타 라이브러리와 다른 이름을 짓는 고민이 아닌가 싶었습니다. 두 분의 의견이 궁금합니다 !! |
타 라이브러리에서 제공해주는지의 여부는 es-hangul이 새로운 기능을 추가할 때 영향을 주는 잣대는 아닌 것 같아요! |
좋습니다 !! 구현 방식은 @BO-LIKE-CHICKEN 님이 작성해주신 커링도 좋고 |
|
@Collection50 저도 |
|
혹시 numberToHangul에서 제공할 함수들을 구현해봐도 될까요? 어떤 방식으로 API를 제공할지는 논의가 더 필요해 보이지만, es-hangul에 추가된다면 유용하게 사용될 기능들이 많아보여서요! ex) formatToKRW, formatToKoreanNumber |
|
구현하기에 앞서, 인터페이스나 함수의 역할과 책임에 대해 먼저 정리하고 나서 구현하면 좋을 것 같아요! |
|
@suhdonghwi @minsoo-web 님께서 별도 채널에서 소중한 의견들 주셔서 제 생각도 정리해봤습니다. num-to-korean 라이브러리에서 대응하는 정도의 옵션들이라면 비즈니스 로직의 95% 정도는 처리할 수 있다고 생각해요! 결국 함수가 해야하는 역할은 숫자를 한글로 변환해주는 것이고 내부적으로 약간 옵션에 따른 분기가 추가되기때문에 여러개의 함수보다는 하나의 함수면서 옵션으로 결과값의 형태를 정의하는 모습이면 좋겠다고 생각했어요. number(123456) // 일이삼사오육 구현할 필요 없다고 생각해요. 너무 간단하고, 니즈가 크게 없어보입니다.
number(123456) // 십이만삼천사백오십육 => 기본 기능
number(123456, "숫자한글믹스") // 10만 3,456
formatToKRW와 같은 기능을 하는 옵션
formatToKoreanNumber 와 같은 기능을 하는 옵션위 정도면 대부분의 숫자를 한글로 변환시키려는 케이스는 대응이 가능해보여요 이렇게 사용하는 쪽의 편의성을 챙길 수 있도록 제공해주고 추후에 더 다양한 비즈니스 요구사항이 있다면 @suhdonghwi 님이 제안해주신 다음과 같은 방법이 있을 것 같아요. numberToHangul(15201100) =>
[
{
multiple: 1520,
multipleHangul: '일천오백이십'
multipleHangulLingual: '천오백이십'
unit: 10000,
unitHangul: '만',
},
{
multiple: 1100,
multipleHangul: '일천백',
multipleHangulLingual: '천백',
unit: 1,
unitHangul: ''
}
]
|
|
더 잘 tree-shaking 될 수 있도록 각각의 옵션을 분리하는 것도 좋을 것 같아요 import { numberToKorean, numberToKoreanMixed, numberToKoreanSpaced } from 'es-hangul/format'; numberToKorean(12345678) === '일천이백삼십사만오천육백칠십팔' Referenced by @raon0211 |
TL;DR: es-hangul이 트리 쉐이킹(Tree Shaking) 을 중점으로 둔다면, 각각의 옵션을 개별적으로 분리하는 것이 더 적합하다고 생각합니다. 이렇게 하면 유지보수도 용이하고, 각 함수가 명확한 책임을 가지며 코드 가독성도 좋아집니다. 대부분의 애플리케이션에서는 복잡한 금융 도메인처럼 여러 옵션이 필요하지 않기 때문에, 필요한 함수만 다만, 파일이 많아질 수 있다는 단점이 있어서, 파일 구조에 대한 명확한 전략이 필요할 것 같아요. 또한, 사용자가 원하는 기능을 찾기 어렵지 않도록 API 문서나 디렉터리 구조를 명확하게 제공하는 것이 중요해 보입니다. 
저는 es-hangul의 가장 큰 지향점이 더 나은 트리 쉐이킹(Tree Shaking) 이라면, 각각의 옵션을 분리하는 것이 더 적합하다는 의견에 동의합니다. 트리 쉐이킹을 제외하더라도 아래의 이유로 이 방식이 더 좋다고 생각해요:
단점은 파일 수가 많아질 수 있다는 점인데, 이를 해결하려면 파일 구조를 어떻게 구성할지에 대한 명확한 전략을 정하고 가면 좋을 것 같아요. 예를 들어, 숫자 관련 함수는 이러한 구조의 문제점은 유저가 원하는 기능을 찾기 어려워질 수 있다는 점입니다. 복잡한 한글이라는 유니코드의 문제점을 해결하고자 es-hangul을 찾았지만 비슷한 기능을 하는 함수들이 문서에서 파편화되어 있다면 만족스럽지 못할 것 같아요. 예를 들어, '3,086만 4,627'와 같은 숫자 포맷을 바꾸고 싶다면, 사용자들은 최악의 경우에
|
|
폴더구조는 src밑에 함수가 1뎁스로 쭉 나오면 좋을 것 같고, |
|
안녕하세요. 폴더 구조를 #242 에서 개선해주신 후 여전히 숫자를 한글로 포매팅하는 함수가 진행되지 않고 있는 것 같습니다. 추가적인 논의가 이뤄지고 있는 상황이 아니라면 이슈라이징 후 진행해봐도 괜찮을까요? |
|
네네 답변 늦어 죄송합니다 진행해주시면 감사하겠습니다!! 🙇 |
Bug description
amountToHangul이라는 함수 이름이 함수 자신이 하는 역할과 책임을 잘 나타내고 있나요?Expected behavior
No response
To Reproduce
No response
Possible Solution
No response
etc.
No response
The text was updated successfully, but these errors were encountered: