Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 숫자를 순 우리말 서수사로 변환하는 함수 추가 #307

Merged
merged 9 commits into from
Jan 6, 2025

Conversation

wet6123
Copy link
Contributor

@wet6123 wet6123 commented Dec 14, 2024

Overview

숫자를 순 우리말 서수사로 변환하는 함수 seosusa를 추가합니다.
1-99 범위의 정수에 대해서만 동작합니다.

서수사 위키, 표준어 규정 6항 을 바탕으로 구현되었습니다.

resolved #303

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.

Copy link

changeset-bot bot commented Dec 14, 2024

🦋 Changeset detected

Latest commit: 4737624

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

Copy link

vercel bot commented Dec 14, 2024

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 Jan 6, 2025 5:56am

Copy link
Member

@okinawaa okinawaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

유저분들이 이 좋은 함수를 잘 사용하실 수 있도록 문서작성도 부탁드릴게요!

src/seosusa/seosusa.ts Outdated Show resolved Hide resolved
Co-authored-by: 박찬혁 <pgg6713@gmail.com>
Comment on lines 32 to 34
if (!isValidNumber(num)) {
throw new Error('유효하지 않은 입력입니다. 1부터 99까지의 정수만 지원합니다.');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요 부분 좀 고민을 해봤는데요
@minsoo-web 님께서 너무나 좋은 의견 주셔서 작성해봅니다!

0~99 의 숫자가 들어오지 않았다는것을 유저에게 조금 더 경험 좋게 전달 할 방법이 있을까요?
주로 seosusa에는 동적으로 값이 주입될텐데. 개발서버에서는 값이 적게 들어와서 에러가 없이
라이브에 코드가 배포되어서 갑자기 100 이상의 값이 들어온다해도 에러가 발생하는건 조금 가혹하다고 생각이 들었어요.

물론 문서에 충분히 명시가 되어있지만, 문서를 읽지 않고 API만 사용하는 개발자분들에게도 조금 더 좋은 경험을 드리고 싶다는 생각도 했어요!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요. 서수사가 99까지만 지원해주는것으로 위키낱말사전 에는 명시가 되어있는데요.

@minsoo-web 님 말씀대로 100이상에 에러를 던지는것은 개발자경험에서 크게 좋지는 않은 것 같아서요.

100이상 숫자에 대해서는 서수사 고유어가 없으므로, 서수사 한자어를 개발자에게 전달해주는것은 어떠신가요?
image

Copy link
Contributor Author

@wet6123 wet6123 Dec 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okinawaa 님 좋은 의견 감사합니다!
최근 일정이 많아 응답이 늦어졌네요. 기다리시게 해서 죄송합니다.

제가 생각해본 방법은 아래와 같은데요.

1. 서수사 한자어 사용

첫 번째 방법은 앞서 이야기 해주신 것처럼 서수사 한자어를 사용하는 것입니다.
이 방법을 선택하는 경우 “째” 대신 “번째”를 사용하는게 더 자연스러운 것같다고 생각하는데 어떤가요?

e.g. 백일째, 백이째, 백삼째 -> 백일 번째, 백이 번째, 백삼 번째

장점

  1. 직관적이고 단순하다.
  2. es-hangul의 기존 함수를 재사용 할 수 있다.

단점

  1. 출력 형식이 제한적이다.

2. 함수 핸들러 사용

두 번째 방법은 주어진 숫자가 범위를 벗어난 경우 사용자가 함수 핸들러를 이용하여 출력을 지정할 수 있도록 하는 것입니다.
입력된 숫자가 범위를 벗어나는 경우 에러 문구를 출력하거나, 한자어로 변환해 출력하는 등의 방법을 자유롭게 사용할 수 있습니다.

function seosusa(num, outOfRangeHandler = (x) => "유효한 범위를 벗어난 숫자입니다") {
	if (!(num >= 1 && num <= 99)) {
		return outOfRangeHandler(num);
	}

장점

  1. 유연한 처리가 가능하다.
  2. 사용자가 더 많은 제어권을 가진다.

단점

  1. 사용이 상대적으로 복잡하다.

저는 한자어를 사용하는 첫 번째 번째 방법이 더 좋다고 생각하는데, 저희가 미처 생각하지 못한 불편함이 있진 않을까 걱정되네요.
의견 남겨주시면 참고해서 이번주까지 완성해보도록 하겠습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

첫번째 방법이 더 사용하기 간결하여 좋은 것 같아요~!
문서에만 99이하일때와, 99초과일때 고유어 / 한자어의 차이가 있다고 잘 명시가 되면 될 것 같습니다~

2번 방법에서 보여주신 예시는 99초과일때 사용자가 서비스 코드에서 충분히 구현 가능할 것 같아서 사용자의 책임으로 여겨도 좋을 것 같네요!

const serverNumer = useServerNumber();

if(serverNumber > 99){
  // 별도 처리
  return;
}

return seosusa(serverNumber);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okinawaa
좋습니다~! 첫번째 방법으로 구현하고 문서와 주석까지 업데이트해보겠습니다.

Copy link
Member

@okinawaa okinawaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

긴 작업 정말 감사합니다!

@okinawaa okinawaa merged commit 5244525 into toss:main Jan 6, 2025
1 of 9 checks passed
@github-actions github-actions bot mentioned this pull request Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: 순서를 나타내는 서수사를 지원하면 어떨까요?
2 participants