@evan-moon 님 @BO-LIKE-CHICKEN 님 의견 모두 동의합니다! 좋은 코맨트들 남겨주셔서 너무 많이 배웠어요!
#42가 머지되어서, 이 이슈는 닫아주셔도 좋을 것 같아요!

from es-hangul.

좋은 의견과, 해결책까지 제시해주셔서 감사해요!

1. 엣지 케이스를 대응하기 위해, 유지보수 난이도 상승(관련 코멘트)
2. 함수의 관심사를 명확하게 분리하기

코맨트와 비슷한 의견인데요, 함수의 목적에 맞게 chosungIncludes는 문자에, 초성이 포함되는지 까지의 역할을 담당하고,
더 나은 유저 경험을 위해 띄어쓰기 까지 고려하는 경우는 해당 함수를 사용하는 서비스 개발자에게 맡기는것은 어떠신가용?

예시

'빌즈 강남'을 'ㅂㅈ ㄱㄴ'으로 검색했을 때 일치하는 결과가 반환되길 원한다면 아래와 같이 사용해도 좋을 것 같아요!

const word = "빌즈 강남";
const chosung = "ㅂㅈ ㄱㄴ";

const trimmedWord = word.replace(/\s/g, '');
const trimmedChosung = chosung.replace(/\s/g, '');

const 유저가_입력한_초성이_단어에_맞는가 = chosungIncludes(trimmedWord, trimmedChosung);

함수 내부에서 처리하지 않기로 한다면, DOCS에 추가 꿀팁 같은것을 남겨도 좋을 것 같아요.

예시

🍯 공백을 포함한 단어도 처리하고 싶다면, 다음과 같이 사용해보세요!

from es-hangul.

좋은 의견과, 해결책까지 제시해주셔서 감사해요!

1. 엣지 케이스를 대응하기 위해, 유지보수 난이도 상승(관련 코멘트)
2. 함수의 관심사를 명확하게 분리하기

코맨트와 비슷한 의견인데요, 함수의 목적에 맞게 chosungIncludes는 문자에, 초성이 포함되는지 까지의 역할을 담당하고, 더 나은 유저 경험을 위해 띄어쓰기 까지 고려하는 경우는 해당 함수를 사용하는 서비스 개발자에게 맡기는것은 어떠신가용?

예시

'빌즈 강남'을 'ㅂㅈ ㄱㄴ'으로 검색했을 때 일치하는 결과가 반환되길 원한다면 아래와 같이 사용해도 좋을 것 같아요!

const word = "빌즈 강남";
const chosung = "ㅂㅈ ㄱㄴ";

const trimmedWord = word.replace(/\s/g, '');
const trimmedChosung = chosung.replace(/\s/g, '');

const 유저가_입력한_초성이_단어에_맞는가 = chosungIncludes(trimmedWord, trimmedChosung);

함수 내부에서 처리하지 않기로 한다면, DOCS에 추가 꿀팁 같은것을 남겨도 좋을 것 같아요.

예시

🍯 공백을 포함한 단어도 처리하고 싶다면, 다음과 같이 사용해보세요!

@okinawaa @BO-LIKE-CHICKEN

저는 공백까지 처리하는 것이 함수의 관심사에서 크게 벗어나는 것은 아닌 것 같다고 생각합니다!
#32 에서 논의했던 특수한 문법인 괄호와는 다르게, 띄어쓰기라는 요소는 한글과 떼어내기 어려운 부분이라고 생각해서요.

가령 두 분이 작성해주신 코멘트나 제가 작성하고 있는 이 글만 봐도 공백이 없는 문자열을 찾아볼 수 없다는 것을 알 수 있슴다.

한글이라는 관심사의 특수성

chosungIncludes와 유사한 동작을 가진 String#includes는 공백 여부까지 일치하지 않으면 false를 반환하기는 하는데, 이건 이 친구가 추상 레벨이 한글보다 낮은 "문자열"에 대한 관심사를 다루고 있기 때문이라고 생각해요.

한글을 다루는 녀석이라면 한글 문법에서 필수적인 띄어쓰기까지도 관심사에 포함해야하는 것이 아닌가 하는 생각이 들었습니다.

DX에 대한 Concern

띄어쓰기가 한글에서 떼어내기 힘든 요소라고 가정했을때, 결국 chosungIncludes가 공백을 처리할 수 없다면 한글 문장을 가져와 chosungIncludes를 사용할 때마다 거의 매번 String#replace(/\s/, '') 처리를 해줘야 한다는 것을 의미합니다.

게다가 클라이언트에서 다루는 값은 대부분 어플리케이션 외부 세계인 API 응답, 유저의 입력 등과 같은 곳에서 생성되는 경우가 많으니, 이러한 예외처리를 건너뛸 수 있는 상황도 드물 것 같아요.

이렇게 어떤 함수를 사용하기 전에 매번 예외처리를 해줘야 한다면 생산성 측면에서 DX가 떨어지지는 않을까 하는 우려가 됩니다.

결론

그래서 저는 아래와 같이 chosungIncludes가 두 인자의 공백을 무시하고 결과를 반환해주는 것이 맞지 않을까 생각했슴다 🙇

chosungIncludes('프론트엔드 개발자', 'ㅍㄹㅌㅇㄷ ㄱㅂㅈ') // true
chosungIncludes('프론트엔드 개발자', 'ㅍㄹㅌㅇㄷㄱㅂㅈ') // true
chosungIncludes('프론트엔드개발자', 'ㅍㄹㅌㅇㄷ ㄱㅂㅈ') // true
chosungIncludes('프론트엔드개발자', 'ㅍㄹㅌㅇㄷㄱㅂㅈ') // true
chosungIncludes('프론트엔드 개발자', ' ') // 공백만 있으면 false

from es-hangul.

es-hangul에서는 utils를 제외한 함수들에 TSDoc을 사용하지 않는 것으로 보이는데요. 이는 각각의 기능에 대한 명세를 chosungIncludes.md 같은 파일에서 관리하고자는 의도일까요?

네! Docs에 접근하는게 크게 어렵진 않을 것 같아서, Docs에 라이브러리 맥락에 대한 정보를 최대한 담으려고 하고 있어요! TSDoc도 사용하면 좋겠지만, TSDoc과, Docs 두벌을 관리하기에는 점점 라이브러리가 커감에 따라, 두 문서간 어긋나는 부분도 생기고 SSOT를 지키기 어려울 것 같다고 생각해요. 당연히 TSDoc를 사용하면 개발자 경험은 좋을 것이라는 의견에는 동의합니다! 나중에 한번 더 이 부분 리비짓 해보시죠!

from es-hangul.

좋은 의견과, 해결책까지 제시해주셔서 감사해요!

1. 엣지 케이스를 대응하기 위해, 유지보수 난이도 상승(관련 코멘트)
2. 함수의 관심사를 명확하게 분리하기

코맨트와 비슷한 의견인데요, 함수의 목적에 맞게 chosungIncludes는 문자에, 초성이 포함되는지 까지의 역할을 담당하고, 더 나은 유저 경험을 위해 띄어쓰기 까지 고려하는 경우는 해당 함수를 사용하는 서비스 개발자에게 맡기는것은 어떠신가용?

예시

'빌즈 강남'을 'ㅂㅈ ㄱㄴ'으로 검색했을 때 일치하는 결과가 반환되길 원한다면 아래와 같이 사용해도 좋을 것 같아요!

const word = "빌즈 강남";
const chosung = "ㅂㅈ ㄱㄴ";

const trimmedWord = word.replace(/\s/g, '');
const trimmedChosung = chosung.replace(/\s/g, '');

const 유저가_입력한_초성이_단어에_맞는가 = chosungIncludes(trimmedWord, trimmedChosung);

함수 내부에서 처리하지 않기로 한다면, DOCS에 추가 꿀팁 같은것을 남겨도 좋을 것 같아요.

예시

🍯 공백을 포함한 단어도 처리하고 싶다면, 다음과 같이 사용해보세요!

먼저, 좋은 의견 감사드립니다! 첨부해 주신 코멘트 또한 매우 유용했습니다.
이슈를 제기하기 전에 이 부분이 마이너한 케이스인지에 대해 많은 고민을 했습니다. 🤔
앞서 언급하신대로 이 부분을 버그로 판단하지 않았던 이유는 아래 코드 때문입니다.

기존코드

const initialConsonantsY = getFirstConsonants(y).replace(/\s/g, '');

isOnlyInitialConsonant 함수는 인자로 받는 문자열에 공백이 포함되면 반드시 false를 반환합니다. 따라서 initialConsonantsY에서 공백을 제거하는 것은 불필요합니다.

따라서, 더 나은 사용자 경험을 위해 띄어쓰기까지 고려하는 것을 서비스 개발자에게 맡긴다면, 다음과 같이 개선할 수 있을 것 같습니다.

개선하기

1. 불필요한 공백 제거 로직을 제외한다

const initialConsonantsY = getFirstConsonants(y);

2. 문서화의 역할을 겸하는 테스트코드를 추가한다

it('should return false when "ㅍㅌㅌㅇㄷ ㄱㅂㅈ" is entered for searching "프론트엔드 개발자" as the function does not support spaces in the input', () => {
    expect(chosungIncludes('프론트엔드 개발자', 'ㅍㄹㅌㅇㄷ ㄱㅂㅈ')).toBe(false);
  });

3. chosungIncludes.md에 공백이 포함된 문자열에서 사용하는 방법을 예시로 추가한다.

Tips

공백을 포함한 단어도 처리하고 싶다면, 다음과 같이 사용해보세요!

const word = '프론트엔드 개발자';
const chosung = 'ㅍㄹㅌㅇㄷ ㄱㅂㅈ';

const trimmedWord = word.replace(/\s/g, '');
const trimmedChosung = chosung.replace(/\s/g, '');

const 유저가_입력한_초성이_단어에_맞는가 = chosungIncludes(trimmedWord, trimmedChosung);

덧붙여 es-hangul에서는 utils를 제외한 함수들에 TSDoc을 사용하지 않는 것으로 보이는데요. 이는 각각의 기능에 대한 명세를 chosungIncludes.md 같은 파일에서 관리하고자는 의도일까요? 이 부분이 아니라면 TSDoc을 활용해서 함수를 사용하는 서비스 개발자에게 알려주어도 무척 좋겠다는 생각이 듭니다.

감사합니다!

from es-hangul.

@evan-moon , @okinawaa 님의 소중한 의견을 모두 귀담아 듣고 다음과 같이 제 의견을 정리해보았습니다.

제가 생각하는chosungIncludes의 개선 방향

기존 이슈를 등록할 때의 의도처럼 공백이 포함된 초성 검색 시 두 인자에 포함된 공백을 무시하고 초성의 일치여부를 검색하면 좋을 것 같습니다. 🙂

아무래도 한글이라는 특수성을 고려하며 다양한 유즈케이스를 생각해 본다면 서비스 개발자의 불필요한 반복을 줄일 수 있는 방향으로 가는것이 좋을 것 같다는 생각이 들었습니다. 특히 가장 많이 사용될 것으로 예상되는 검색이라는 영역에서 더 유리하게 작용할 것 같습니다.

chosungIncludes('반포자이', 'ㅂㅍㅈㅇ') // true
chosungIncludes('반포자이', 'ㅂㅍ ㅈㅇ') // true
chosungIncludes('반포 자이', 'ㅂㅍㅈㅇ') // true
chosungIncludes('반포 자이', 'ㅂㅍ ㅈㅇ') // true

다만, isOnlyInitialConsonant함수의 역할을 유지하며 chosungIncludes에 공백과 상관없이 초성이 포함되었는지 여부를 검사해주도록 변경하면 더욱 좋을 것 같습니다.

개선된 코드

import { disassembleHangulToGroups } from './disassemble';
import { canBeChosung, getFirstConsonants } from './utils';

export function chosungIncludes(x: string, y: string) {
  const trimY = y.replace(/\s/g, '');

  if (!isOnlyInitialConsonant(trimY)) {
    return false;
  }

  const initialConsonantsX = getFirstConsonants(x).replace(/\s/g, '');
  const initialConsonantsY = trimY;

  return initialConsonantsX.includes(initialConsonantsY);
}

/*
 * @description 문자열이 한글초성으로만 주어진 경우
 */
function isOnlyInitialConsonant(str: string) {
  return disassembleHangulToGroups(str).every(disassembled => {
    return disassembled.length === 1 && canBeChosung(disassembled[0]);
  });
}

앞으로의 개선점

• 공백을 포함하여 정확히 일치하는지의 검사도 필요한 경우가 있을 것 같습니다.

chosungIncludes('프론트엔드 개발자', 'ㅍㄹㅌㅇㄷ ㄱㅂㅈ') // true
chosungIncludesExact('프론트엔드 개발자', 'ㅍㄹㅌㅇㄷㄱㅂㅈ') // false

비록 마이너한 케이스이지만, 유즈케이스가 많아진다면 #49 와 함께 개선해보면 좋을 것 같습니다.

제가 생각하는chosungIncludes의 개선 방향에 대한 반영은 #42 에 이어서 반영해 두겠습니다.
의견남겨주시면 무척 감사하겠습니다. 🙏🏻

from es-hangul.