chore: numberToHangul 성능을 개선하였습니다.

[iamhungry1030]

Overview

1. 숫자가 0 이상일 경우에만 unshift문을 수행하도록 수정하였습니다.
2. 여러번의 정규표현식 replace을 그룹캡쳐를 활용하도록 수정하였습니다.

PR Checklist

• I read and included theses actions below

1. I have read the Contributing Guide
2. I have written documents and tests, if needed.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
es-hangul	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 16, 2024 6:54am

[iamhungry1030]

벤치마크 없이 직관으로만 수정한 코드라 조금 아쉽긴 하네요..

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.53%. Comparing base (0a60a65) to head (15ed196).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #283      +/-   ##
==========================================
- Coverage   99.53%   99.53%   -0.01%     
==========================================
  Files          38       38              
  Lines         642      640       -2     
  Branches      156      157       +1     
==========================================
- Hits          639      637       -2     
  Misses          3        3              

[okinawaa]

확실히 그룹캡쳐로 개선해주신건 개선이 있네요.
1번 수정사항에 대해서 조금 더 살펴볼게요!!

아래와 같이 2번 사항에 대한 개선은 살펴봤어요!

function method1(input) {
  return input.replace(/일천/, '천').replace(/일백/, '백').replace(/일십/, '십') || '';
}

function method2(input) {
  return input.replace(/일(천|백|십)/g, '$1') || '';
}

const input = '일천일백일십일천일백일십';
const iterations = 100000;

// Measure time for method 1
console.time('Method 1');
for (let i = 0; i < iterations; i++) {
  method1(input);
}
console.timeEnd('Method 1');

// Measure time for method 2
console.time('Method 2');
for (let i = 0; i < iterations; i++) {
  method2(input);
}
console.timeEnd('Method 2');

[okinawaa]

currentPart가 음수인 경우는 없고,
0인 경우는 0이 네번 연속인 경우일뿐일텐데.

이는 극히 드문 케이스이며 (10000, 200000, 300000... )
이를 처리하려고 Number 를 씌우고 양수 분기를 태우는것이 코드의 성능 개선보다는 코드의 복잡도를 더 올린다고 생각하는데 어떻게 생각하시나요?!

[iamhungry1030]

맞습니다 ㅎㅎ
제가 es-hangul은 한글을 편하게 다룰 수 있는 저수준의 모듈로 정의하였고, 애초에 성능을 개선할 점이 있을까? 라는 관점으로 바라보았어요.

때문에 내부 구현은 가독성을 조금 포기하여도 된다고 생각하였어요.

다만 numberToHangul의 경우 숫자를 한글로 바꾸는 함수이기 때문에 일반적인 use case에서는 input의 size가 어느정도 제한되어있고, 성능차이는 미비할 것으로 보여요.

다음부턴 이런 개선 사항들은 이슈로 먼저 올려봐야겠네요 !

감사합니다 🙇‍♂️