feat: 숫자를 날짜를 나타내는 순우리말로 바꿔주는 함수 중 days를 추가 #228

BO-LIKE-CHICKEN · 2024-08-07T15:42:14Z

Overview

#217 에서 논의한 함수를 추가합니다.
당시에는 date라는 함수명을 제안드렸으나, 조사 과정에서 비단 날(day) 뿐만 아니라 달(month)을 뜻하는 순 우리말이 있다는 사실을 알게되었습니다.

하여, date 파일의 days라는 이름으로 작성하였습니다.

함께 논의되고 싶은 사항은 다음과 같습니다:

15일의 경우 열닷새와 보름으로 사용되는데 우선 열닷새로 구현했습니다. 하지만, 보름이 조금 더 많이 사용될 케이스인 것 같아서 고민입니다.
마찬가지로 보름이 아니더라도 특정 날을 지칭하는 순 우리말들이 있습니다. (21일의 경우 세이레) 이런 경우는 극히 마이너해서 배제했지만, 이후에 어떻게 관리되는 편이 좋을까요?

PR Checklist

I read and included theses actions below

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

changeset-bot · 2024-08-07T15:42:17Z

🦋 Changeset detected

Latest commit: 0922f46

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

vercel · 2024-08-07T15:42:19Z

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	Aug 9, 2024 4:05pm

codecov-commenter · 2024-08-07T15:46:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.83%. Comparing base (08adfc9) to head (87b0b41).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #228   +/-   ##
=======================================
  Coverage   99.82%   99.83%           
=======================================
  Files          29       31    +2     
  Lines         584      600   +16     
  Branches      141      145    +4     
=======================================
+ Hits          583      599   +16     
  Misses          1        1

okinawaa

우선 너무 잘 구현해주셔서 감사합니다.

기본값은 열닷새, 스무하루처럼 유저에게 제공하되, 아래처럼 옵션을 받아서 자주쓰는(대안어) 를 제공해주도록 하는 것은 어떨까요~?

day(15) // 열닷새
day(15, {
  useAlternateExpression: true
}) // 보름

docs/src/pages/docs/api/date.en.mdx

okinawaa · 2024-08-09T05:44:38Z

src/constants.ts

+
+export const DATE_DAYS_MAP = {
+  1: '하루',
+  2: '이틀',
+  3: '사흘',
+  4: '나흘',
+  5: '닷새',
+  6: '엿새',
+  7: '이레',
+  8: '여드레',
+  9: '아흐레',
+  10: '열',
+  20: '스무',
+} as const;
+
+export const DATE_DAYS_ONLY_TENS_MAP = {
+  10: '열흘',
+  20: '스무날',
+  30: '서른날',


요것도 date에 종속된 상수이므로 standardizePronunciation.constants.ts 처럼
day.constants.ts로 분리되면 좋을 것 같아요!

저에게도 보기 더 좋아보여요.

as-is

canbe.ts를 참고해서 폴더구조나 컨벤션을 가져갔어요.

to-be

제안 주신 방식대로 refactor: date의 확장을 고려해 폴더구조를 변경에 개선해 보았어요.

검토부탁드려요 🙏🏻

okinawaa · 2024-08-09T05:45:05Z

docs/src/pages/docs/api/date.ko.mdx

+```ts index.ts
+import { canBeJongseong } from 'es-hangul';
+
+console.log(canBeJongseong('ㄱ'));
+```


import { days } from 'es-hangul'; console.log(days(3)); console.log(days(4));

찾아주셔서 감사해요!

docs: 잘못된 예제 수정에서 수정되었습니다

okinawaa

잘못 어프롭눌러서 수정합니당!! 감사해요

BO-LIKE-CHICKEN · 2024-08-09T14:36:45Z

우선 너무 잘 구현해주셔서 감사합니다.

기본값은 열닷새, 스무하루처럼 유저에게 제공하되, 아래처럼 옵션을 받아서 자주쓰는(대안어) 를 제공해주도록 하는 것은 어떨까요~?
day(15) // 열닷새
day(15, {
  useAlternateExpression: true
}) // 보름

아직 반영 전 이기하지만 이런 인터페이스는 어떨까요?

day(15) // 열닷새

day(15, {
 alternateExpressionDays: [15]
}) // 보름

추후 보름이 아닌 덜 알려진 날짜(세이레)의 지원도 고려했을 때 alternateExpressionDays를 직접 배열로 넣어주는 방식은 어떨까요?
저는 열닷새의 경우에는 보름으로 치환하고 싶지만 21일은 세이레로 치환되는 것을 원치 않을 경우를 고려했어요.

alternateExpressionDays는 조금 길어보이긴 하네요 🙃

okinawaa · 2024-08-09T14:52:57Z

boolean값만 받아서 내부적으로 분기하는게 사용하는쪽에서도, 구현측면에서도 깔끔하다고 생각이 들어요.
아래 코드가, 이 days함수에 집중해서 논의하고 있는 저희에게는 이해가 쉬워도, 아래 코드를 스쳐지나가며 보는 개발자는 배열로 15를 주는것이 어떤 옵션인지 잘 이해하지 못할 것 같다고 생각했습니다.

day(15, {
 alternateExpressionDays: [15]
})

boolean으로 옵션을 받아도 말씀해주신 요구사항은 충분히 반영가능해보여요!

const serverDays = number

day(serverDays, {
  useAlternateExpression: serverDays === 15 
})

막상 코드를 적어보니, boolean, array 둘다 사용하는 쪽에선 verbose한 느낌이 나네요 ㅠㅠㅋㅋ

okinawaa · 2024-08-09T14:48:00Z

src/date/date.spec.ts

+describe('date', () => {
+  describe('days', () => {
+    const validNumbers = [
+      { num: 1, word: '하루' },
+      { num: 2, word: '이틀' },
+      { num: 3, word: '사흘' },
+      { num: 4, word: '나흘' },
+      { num: 5, word: '닷새' },
+      { num: 6, word: '엿새' },
+      { num: 7, word: '이레' },
+      { num: 8, word: '여드레' },
+      { num: 9, word: '아흐레' },
+      { num: 10, word: '열흘' },
+      { num: 11, word: '열하루' },
+      { num: 20, word: '스무날' },
+      { num: 21, word: '스무하루' },
+      { num: 30, word: '서른날' },
+    ];
+
+    const invalidNumbers = [0, -1, 31, 1.1, -1.1, Infinity, -Infinity, NaN];
+
+    validNumbers.forEach(({ num, word }) => {
+      it(`${num} - 순 우리말 날짜 ${word}로 바꿔 반환해야 한다.`, () => {
+        expect(days(num)).toBe(word);
+      });
+    });
+
+    invalidNumbers.forEach(num => {
+      it(`유효하지 않은 숫자 ${num}에 대해 오류를 발생시켜야 한다.`, () => {
+        expect(() => days(num)).toThrow('지원하지 않는 숫자입니다.');
+      });
+    });
+  });
+});


👍👍👍👍👍👍👍👍

BO-LIKE-CHICKEN · 2024-08-09T15:03:23Z

boolean값만 받아서 내부적으로 분기하는게 사용하는쪽에서도, 구현측면에서도 깔끔하다고 생각이 들어요. 아래 코드가, 이 days함수에 집중해서 논의하고 있는 저희에게는 이해가 쉬워도, 아래 코드를 스쳐지나가며 보는 개발자는 배열로 15를 주는것이 어떤 옵션인지 잘 이해하지 못할 것 같다고 생각했습니다.
day(15, {
 alternateExpressionDays: [15]
})
boolean으로 옵션을 받아도 말씀해주신 요구사항은 충분히 반영가능해보여요!
const serverDays = number

day(serverDays, {
  useAlternateExpression: serverDays === 15 
})
막상 코드를 적어보니, boolean, array 둘다 사용하는 쪽에선 verbose한 느낌이 나네요 ㅠㅠㅋㅋ

고민이 많이 되네요 boolean으로 처리하는 것이 훨씬 깔끔하다는 것은 공감하고, 대체어를 사용하고 싶은 부분을 boolean으로 처리하는 방식도 납득되어요.

하지만, 말씀해주신것처럼 둘 다 간결하다는 느낌은 들지 않네요.

우선 es-hangul에서는 지금의 형태로 제공하고 제가 가장 염려하는 days(15) => "보름"의 경우에는 days를 사용하는 곳에서 처리하도록 할까요?

이후에 더 많은 함수들이 생기고, 확장되는 기능들에 대한 규칙이 정립되면 그때 추가해도 괜찮을 것 같다고 생각이 들었어요 🙂

(나중에는 es-hangul의 함수들을 기반으로 하는 더 많은 확장기능응 제공하는 es-hangul+ 같은게 있어도 재미있을듯하네요 😁)

const restDays = 15;

restDays === 15 ? "보름" : days(restDays)

okinawaa · 2024-08-09T16:03:19Z

우선 es-hangul에서는 지금의 형태로 제공하고 제가 가장 염려하는 days(15) => "보름"의 경우에는 days를 사용하는 곳에서 처리하도록 할까요?
이후에 더 많은 함수들이 생기고, 확장되는 기능들에 대한 규칙이 정립되면 그때 추가해도 괜찮을 것 같다고 생각이 들었어요 🙂

와우 너무 좋은 의견입니다! 동의해요~!

okinawaa

이슈레이징 및 직접 기능 구현까지 항상 너무 감사드려요!!

BO-LIKE-CHICKEN · 2024-08-09T22:46:03Z

이슈레이징 및 직접 기능 구현까지 항상 너무 감사드려요!!

섬세한 의견 교류 덕분에 많이 배우고 너무 즐겁습니다.

저도 감사해요! 😊

BO-LIKE-CHICKEN requested review from okinawaa and raon0211 as code owners August 7, 2024 15:42

vercel bot deployed to Preview August 7, 2024 15:43 View deployment

okinawaa approved these changes Aug 9, 2024

View reviewed changes

okinawaa requested changes Aug 9, 2024

View reviewed changes

BO-LIKE-CHICKEN added 8 commits August 9, 2024 23:14

feat: days 구현

d42cc8f

test: days에 대한 테스트 작성

172c8fc

feat: 테스트를 통과하도록 미구현부 구현

244c615

feat: index 에서 export 하도록 수정

6e8cd9e

chore: 코드 정리

5491d78

chore: 불필요한 주석 제거

5d72e89

docs: date mdx 파일 추가

ca4391b

test: 테스트를 date로 감싸고 invaildNumber에서 101보다 가까운 31로 수정

2531d0a

BO-LIKE-CHICKEN force-pushed the feature/days branch from 8aa1273 to 2531d0a Compare August 9, 2024 14:14

vercel bot deployed to Preview August 9, 2024 14:16 View deployment

docs: 잘못된 예제 수정

d84d271

vercel bot deployed to Preview August 9, 2024 14:18 View deployment

refactor: date의 확장을 고려해 폴더구조를 변경

87b0b41

vercel bot deployed to Preview August 9, 2024 14:28 View deployment

BO-LIKE-CHICKEN requested a review from okinawaa August 9, 2024 14:37

okinawaa reviewed Aug 9, 2024

View reviewed changes

Create spicy-teachers-admire.md

0922f46

okinawaa approved these changes Aug 9, 2024

View reviewed changes

okinawaa merged commit 0633b9f into toss:main Aug 9, 2024
9 of 10 checks passed

vercel bot deployed to Preview August 9, 2024 16:05 View deployment

github-actions bot mentioned this pull request Aug 9, 2024

chore: version packages #232

Merged

BO-LIKE-CHICKEN deleted the feature/days branch August 9, 2024 23:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 숫자를 날짜를 나타내는 순우리말로 바꿔주는 함수 중 days를 추가 #228

feat: 숫자를 날짜를 나타내는 순우리말로 바꿔주는 함수 중 days를 추가 #228

BO-LIKE-CHICKEN commented Aug 7, 2024

changeset-bot bot commented Aug 7, 2024 •

edited

Loading

vercel bot commented Aug 7, 2024 •

edited

Loading

codecov-commenter commented Aug 7, 2024 •

edited

Loading

okinawaa left a comment

okinawaa Aug 9, 2024

BO-LIKE-CHICKEN Aug 9, 2024 •

edited

Loading

okinawaa Aug 9, 2024

BO-LIKE-CHICKEN Aug 9, 2024

okinawaa left a comment

BO-LIKE-CHICKEN commented Aug 9, 2024 •

edited

Loading

okinawaa commented Aug 9, 2024

okinawaa Aug 9, 2024

BO-LIKE-CHICKEN commented Aug 9, 2024 •

edited

Loading

okinawaa commented Aug 9, 2024 •

edited

Loading

okinawaa left a comment

BO-LIKE-CHICKEN commented Aug 9, 2024

feat: 숫자를 날짜를 나타내는 순우리말로 바꿔주는 함수 중 days를 추가 #228

feat: 숫자를 날짜를 나타내는 순우리말로 바꿔주는 함수 중 days를 추가 #228

Conversation

BO-LIKE-CHICKEN commented Aug 7, 2024

Overview

함께 논의되고 싶은 사항은 다음과 같습니다:

PR Checklist

changeset-bot bot commented Aug 7, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Aug 7, 2024 • edited Loading

codecov-commenter commented Aug 7, 2024 • edited Loading

Codecov Report

okinawaa left a comment

Choose a reason for hiding this comment

okinawaa Aug 9, 2024

Choose a reason for hiding this comment

BO-LIKE-CHICKEN Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

as-is

to-be

okinawaa Aug 9, 2024

Choose a reason for hiding this comment

BO-LIKE-CHICKEN Aug 9, 2024

Choose a reason for hiding this comment

okinawaa left a comment

Choose a reason for hiding this comment

BO-LIKE-CHICKEN commented Aug 9, 2024 • edited Loading

okinawaa commented Aug 9, 2024

okinawaa Aug 9, 2024

Choose a reason for hiding this comment

BO-LIKE-CHICKEN commented Aug 9, 2024 • edited Loading

okinawaa commented Aug 9, 2024 • edited Loading

okinawaa left a comment

Choose a reason for hiding this comment

BO-LIKE-CHICKEN commented Aug 9, 2024

changeset-bot bot commented Aug 7, 2024 •

edited

Loading

vercel bot commented Aug 7, 2024 •

edited

Loading

codecov-commenter commented Aug 7, 2024 •

edited

Loading

BO-LIKE-CHICKEN Aug 9, 2024 •

edited

Loading

BO-LIKE-CHICKEN commented Aug 9, 2024 •

edited

Loading

BO-LIKE-CHICKEN commented Aug 9, 2024 •

edited

Loading

okinawaa commented Aug 9, 2024 •

edited

Loading