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

[Bug]: 괄호 뒤 조사를 처리하지 못하는 문제 #32

Closed
natz92 opened this issue Apr 12, 2024 · 8 comments
Closed

[Bug]: 괄호 뒤 조사를 처리하지 못하는 문제 #32

natz92 opened this issue Apr 12, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@natz92
Copy link

natz92 commented Apr 12, 2024

Bug description

과일(딸기)이 좋다.

위처럼 괄호 뒤 조사의 경우 괄호 앞에 단어를 기준으로 조사를 붙이는게 관행이나, 이를 제대로 인식하지 못합니다.

참고: https://www.korean.go.kr/front/onlineQna/onlineQnaView.do?mn_id=216&qna_seq=252613&pageIndex=1

Expected behavior

No response

To Reproduce

import { josa } from "es-hangul";

const word1 = "비고(총수량)";
const word2 = "총수량(비고)";
const word3 = "()";

console.log(josa(word1, "이/가") + " 비어 있습니다.");
console.log(josa(word2, "이/가") + " 비어 있습니다.");
console.log(josa(word3, "이/가") + " 비어 있습니다.");

const word4 = "대지의 여신(이하 가이아)";
const word5 = "가이아(이하 대지의 여신)";
const word6 = "()";

console.log(josa(word4, "을/를") + " 숭배하라");
console.log(josa(word5, "을/를") + " 숭배하라");
console.log(josa(word6, "을/를") + " 숭배하라");
# 출력값
비고(총수량)가 비어 있습니다.
총수량(비고)가 비어 있습니다.
()가 비어 있습니다.
대지의 여신(이하 가이아)를 숭배하라
가이아(이하 대지의 여신)를 숭배하라
()를 숭배하라

Possible Solution

')' 을 기준으로 조사를 처리하여 발생하는 것으로 보여 괄호 앞 단어를 찾아 그에 맞는 조사를 붙여줘야 할것으로 보입니다.

etc.

No response

@natz92 natz92 added the bug Something isn't working label Apr 12, 2024
@okinawaa
Copy link
Member

okinawaa commented Apr 12, 2024

버그 발견 감사합니다!
충분히 있을법한 유스케이스라고 생각해요.

josa함수 내에서, 아래와 비슷한 인터페이스로 구현해보면 좋을 것 같은데 어떠신가요!?
함수명은 책임에 맡게 적절히 수정이 필요해보이긴 합니다!

export function josa(word: string, josa: JosaOption): string {
  if (word.length === 0) {
    return word;
  }

  const hasBracket = checkHasBracket(word);
  const baseWord = hasBracket ? extractBaseWordByBracket(word) : word

  return word + josaPicker(baseWord, josa);
}

@jw-r
Copy link
Contributor

jw-r commented Apr 12, 2024

word가 (), 대지(땅)의 여신이 될 케이스를 고려해 다음과 같은 인터페이스로 구현해봐도 좋겠네요!

export function josa(word: string, josa: JosaOption): string {
  const endsWithBracket = checkEndsWithBracket(word);
  const baseWord = endsWithBracket ? removeLastBracket(word) : word;

  if (baseWord.length === 0) {
    return word;
  }

  return word + josaPicker(baseWord, josa);
}

안녕하세요, @natz92

매우 의미있는 버그를 발견해주신 것 같아요! 이 이슈에 대해 해결 계획이 있으신지 궁금합니다.
혹시 아직 구체적인 계획이 없으시다면, 제가 이 이슈를 해결해봐도 괜찮을지 여쭙고 싶습니다!

감사합니다!

@natz92
Copy link
Author

natz92 commented Apr 12, 2024

@okinawaa @jw-r 좋은 해결 방안들을 생각해 주셔서 감사합니다! 제가 직접 기여할 계획은 없어서 마음껏 좋은 방법을 적용시켜 주셔도 됩니다.

@jw-r
Copy link
Contributor

jw-r commented Apr 12, 2024

@natz92 감사합니다!

제가 이 문제를 해결해보겠습니다.

@raon0211
Copy link
Collaborator

개인적인 의견이지만, 이런 경우는 약간의 엣지케이스에 해당하는데, josa 함수가 여기에 대응하도록 하면 구현이 복잡해지고 유지보수가 힘들어질 수 있을 것 같다는 생각이 있어요.

(유명한 소프트웨어 설계 원칙 가운데 Worse is better 라고 하는 원칙이 있는데, 실용적인 수준으로 유스케이스를 커버하고, 구현의 단순함을 인터페이스의 복잡함보다 우선순위 높게 가져가는 것을 추천합니다.)

josa 함수를 수정하기보다는, 애플리케이션에서 josa.pick() 함수를 이용하여 대응해보는 것에 대해서는 어떻게 보시나요?

import { josa } from 'es-hangul';

const word = `대지의 여신(이하 가이아)`;

const result = `${word}${josa.pick(removeBrackets(word), '이/가')`;

function removeBrackets(str: string) {
  /* 문자열에서 괄호를 제거 */
}

@evan-moon
Copy link
Member

evan-moon commented Apr 13, 2024

저는 #34 PR에 코멘트를 남겼는데, 이제보니 이슈가 따로 있었군요!

저도 @raon0211 님과 비슷한 의견인데요.

개인적으로 josa 함수는 명사와의 문법 관계를 다루는 것에만 집중하고, 괄호와 같은 특별한 맥락에 대한 처리는 함수 바깥에서 별도로 처리해주는 것이 이 함수의 관심사에 더 적합하지 않을까...하는 생각이 들었습니다!

const text = '샴푸(보습)';
const josaForText = josa(text.replace(/(.*\)/, ''); // '가';
console.log(`${text}${josaForText}`); // '샴푸(보습)가'

#34 (review)

@evan-moon
Copy link
Member

해당 이슈는 #34 가 Close되어 의사결정을 마무리하려고 하는데, 혹시 처음 이슈레이징해주신 @natz92 님 의견은 어떠신가요?? 🙏

@natz92
Copy link
Author

natz92 commented Apr 13, 2024

@evan-moon 내용 잘 확인했습니다. 종료해주셔도 좋습니다. 다들 제 의견에 참여 해주셔서 감사합니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants