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]: removeLastHangulCharacter 함수에서 disassembledGroups의 길이가 4개인 경우 에러가 발생합니다. #123

Closed
jgjgill opened this issue Jun 18, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@jgjgill
Copy link
Contributor

jgjgill commented Jun 18, 2024

Bug description

removeLastHangulCharacter: 인자로 주어진 한글 문자열에서 가장 마지막 문자 하나를 제거하여 반환

현재 removeLastHangulCharacter에서 withoutLastCharacter를 구할 때 map 함수 내부에서 first, second, last로 구성되어 있어요. 하지만 last에서 생각대로 동작하지 못하는 부분을 발견했습니다.

disassembleHangulToGroups 함수의 테스트 예시로 을 넣었을 경우 ['ㄱ', 'ㅏ', 'ㅂ', 'ㅅ']을 기대하고 있습니다.

하지만 실제 combineHangulCharacter로 다시 결합시킬 때는 을 받고 싶으면 'ㄱ', 'ㅏ', 'ㅂㅅ'을 입력받기를 기대하고 있습니다.

그래서 다음과 같이 테스트를 진행했을 때 에러가 발생해요.

  • 테스트값: 안녕하세요 값이
  • 예상 값: 안녕하세요 값ㅇ
  • 실제 값: 안녕하세요 갑ㅇ
스크린샷 2024-06-19 오전 1 08 22

콘솔로 확인해보면 last에서 이 생략되고 있습니다.
스크린샷 2024-06-19 오전 1 22 10

스크린샷 2024-06-19 오전 1 22 32

정리하면 종성으로 겹받침이 오면 withoutLastCharacter 변수를 구할 때 map 함수에서 생략되는 부분이 발생해요.

Expected behavior

No response

To Reproduce

종성에 겹받침이 오는 문자가 마지막 문자가 아닌 경우 에러가 발생합니다.

ex) 안녕하세요 많이, 안녕하세요 값이

Possible Solution

나머지 매개변수를 활용해서 마지막은 join으로 합쳐서 last를 구성하는 것을 생각했어요.

const withoutLastCharacter = disassembledGroups
  .filter(v => v !== lastCharacter)
  .map(([first, middle, ...rest]) => {
    const last = rest.join('');

    if (middle != null) {
      return combineHangulCharacter(first, middle, last);
    }

    return first;
  });

안녕하세요 많이로 테스트 진행할 경우

  • 예상 값: 안녕하세요 많ㅇ
  • 실제 값: 안녕하세요 많ㅇ
스크린샷 2024-06-19 오전 1 36 36

etc.

라이브러리를 보면서 많이 배우고 있습니다..! 감사합니다! 🙇‍♂️

@jgjgill jgjgill added the bug Something isn't working label Jun 18, 2024
@crucifyer
Copy link
Contributor

요 pr 로 해결 되겠네요 :)
91377c5

@jgjgill
Copy link
Contributor Author

jgjgill commented Jun 20, 2024

@crucifyer 안녕하세요! 해당 부분 이슈나 PR이 없어서 작업이 진행된 줄 몰랐네요..! 😅
참고해주신 커밋 확인했습니다!

해당 파일에서 문제가 되는 withoutLastCharacter을 없애면 될 것 같네요..!
여기서 withoutLastCharacter이라는 변수명은 유지하면 어떨까요? 변수명을 통해 더 이해하기 쉬울 것 같아요.
그리고 나머지 부분은 따로 변경할 필요없이 기존 코드와 유사하게 구성해도 괜찮을 것 같은데 어떻게 생각하시나요?

제안하는 코드

export function removeLastHangulCharacter(words: string) {
  const disassembledGroups = disassembleHangulToGroups(words);
  const lastCharacter = disassembledGroups[disassembledGroups.length - 1];

  if (lastCharacter == null) {
    return '';
  }

  const withoutLastCharacter = words.substring(0, words.length - 1);
  const [[first, middle, last]] = excludeLastElement(lastCharacter);
  const result = middle != null ? combineHangulCharacter(first, middle, last) : first;

  return [withoutLastCharacter, result].join('');
}

우선 해당 이슈는 올려주신 PR이 머지되거나 따로 메인테이너분께서 안내해주시기 전까지는 열어두겠습니다..!
감사합니다..! 🙇‍♂️

@jgjgill
Copy link
Contributor Author

jgjgill commented Jun 30, 2024

해당 PR에서 개선되어서 close 합니다..!

@jgjgill jgjgill closed this as completed Jun 30, 2024
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

2 participants