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

[클린코드 5기 김경현] 로또 미션 STEP 2 #245

Merged
merged 8 commits into from Sep 5, 2023

Conversation

frankuserconnect
Copy link

안녕하세요 인성님,

로또 미션 참가자 김경현입니다.

최근에 이사를 하면서.. 여러 문제가 생겨 미션에 원하는 만큼 집중을 하지 못했네요 ㅜㅜ
기다리시고 계실텐데.. 늦게 연락드려 죄송합니다.

남은 주말 잘 보내시길 바라겠습니다. 🙏🏻

feedback from 미션1

  • "return 구문으로 if-else의 스코프를 줄일 수 있을 것 같습니다!"
    -> 조건에 맞으면, 다음 실행 못하게 return 해주는 방향으로 접근했습니다. 생각하신 의도가 맞는지 궁금합니다.
if (restartOrExitLotto.toLowerCase() !== "y") {
  return rl.close();
} else {
  initializeDataStorage();
  return startLottoGame();
}
  • "메세지를 다르게 가져가거나, 조건문을 합쳐도 괜찮을 것 같네요!"
    -> 조건문을 합치는 방향으로 정했습니다.
export const validatePositiveNumber = num => {
  if (!isValidNumber(num) || isNaN(num) || num <= 0) {
    throw new Error(ERROR_MESSAGE[VALID_NUMBER_REQUIRED]);
  }

  // if (num <= 0) {
  //   throw Error(ERROR_MESSAGE[VALID_NUMBER_REQUIRED]);
  // }
};
  • "_ 접두어는 어떤 의미일까요?"

    다른 함수들과 헷갈리지 않게, 파일안에서만 고유하게 쓸 수 있는 함수 컨벤션을 만들어봤습니다.

  • "앱 전반에 switch-case 구문이 많이 보이는데, 가독성을 헤치는 주 원인 중 하나로 꼽는 문법입니다. 만약 좀 더 좋게 바꾸려면 어떻게 해야 할까요?"
    -> if/else 보다, switch-case이 가독성이 좋을 것 같아 접근을 하였으나, 객체로 묶어서 관리하는편이 나을 것 같다고 판단하여 수정했습니다. + 파일을 깔끔하게 쓰기 위해, jsdoc 파일을 분리하였습니다.

    /** @typedef {RankPriceFunctions} */
    const RANK_PRICE_STATS = {
      firstWinner: count =>
        `6개 일치 (${LOTTO_PRICE.FIRST_WINNER.toLocaleString()}원) - ${count}개`,
      secondWinner: count =>
        `5개 일치, 보너스 볼 일치 (${LOTTO_PRICE.SECOND_WINNER.toLocaleString()}원) - ${count}개`,
      thirdWinner: count =>
        `5개 일치 (${LOTTO_PRICE.THIRD_WINNER.toLocaleString()}원) - ${count}개`,
      fourthWinner: count =>
        `4개 일치 (${LOTTO_PRICE.FOURTH_WINNER.toLocaleString()}원) - ${count}개`,
      fifthWinner: count =>
        `3개 일치 (${LOTTO_PRICE.FIFTH_WINNER.toLocaleString()}원) - ${count}개`,
    };

"난해했던 점"

  • "어떤 의도로 이렇게 배치했는지"
    자동차 경주 미션에서 클래스 + MVC패턴을 사용하면서, 도메인/UI 분리에 도전하였습니다.
    이번 미션에서는 함수형을 사용하면서 도메인/UI 분리를 하며 클래스형과의 차이점을 느껴보고 싶었습니다. 설계 능력이 아직 많이 부족해서 그런지, 드라마틱한 차이점을 느끼지는 못했지만...
    함수형을 사용하면서, 순수 함수를 정의할 때 상대적으로 유연함을 느꼈고, 의존성을 줄이는데 편리함을 느꼈습니다. 또한, 프로그램 동작 예측을 하는데 상대적으로 쉽게 느껴졌습니다. 함수형 코드를 제대로 쓰고 있는지 확신이 들지 않아 인성님께 조언을 구하고 싶습니다. 🙏🏻

  • LottoStore가 "연산"만 담당한다면 store가 맞는지"
    관심사 분리를 하면서 store의 역할이 상당히 애매하다고 느꼈습니다. 역할 분리는 해주고 싶은데, 로또 종이 계산 연산만 필요하다면 지금 당장 뺴야되는건지에 대하여 고민이 컸습니다. 추후에 앱에 미션을 추가하면서 확장된다면, 잔돈을 관리하는 부분과 buyer, company와의 소통 역할도 추가해보고 싶습니다.

feedback for 미션2

/**
 * @typedef {Object} RankPriceStatsFunctions
 * @property {(count: number) => string} firstWinner
 * @property {(count: number) => string} secondWinner
 * @property {(count: number) => string} thirdWinner
 * @property {(count: number) => string} fourthWinner
 * @property {(count: number) => string} fifthWinner
 */

module.exports = {};

/** @type {import('../../../jsdoc_comment.js').RankPriceStatsFunctions}  */
const RANK_PRICE_STATS = {
  firstWinner: count =>
    `6개 일치 (${LOTTO_PRICE.FIRST_WINNER.toLocaleString()}원) - ${count}개`,
  secondWinner: count =>
    `5개 일치, 보너스 볼 일치 (${LOTTO_PRICE.SECOND_WINNER.toLocaleString()}원) - ${count}개`,
  thirdWinner: count =>
    `5개 일치 (${LOTTO_PRICE.THIRD_WINNER.toLocaleString()}원) - ${count}개`,
  fourthWinner: count =>
    `4개 일치 (${LOTTO_PRICE.FOURTH_WINNER.toLocaleString()}원) - ${count}개`,
  fifthWinner: count =>
    `3개 일치 (${LOTTO_PRICE.FIFTH_WINNER.toLocaleString()}원) - ${count}개`,
};
  • 테스트 코드를 어떻게 추가작업을 하면 좋을지 고민을 하고 있습니다. 도전 미션을 던져 주시면 감사하겠습니다. 말씀 해주신 대로 BDD vs TDD에 대해 검색을 해보면서, TDD를 항상 먼저 작성하는 것이 옳을까라는 의문을 던지면서 구글링을 해봤습니다. 현업에서 도메인이 변경이 될 수도 있고, 기능이 변경될 수 있는데 순수함수만 테스트를 해보는데 있어서 괜찮은지 궁금합니다.

  • 에러 핸들링을 처리 미션을 하기 위해 핸들러 파일을 따로 배치하여 도메인, UI를 건드리면서 코드를 작성해봤는데.. 자신감이 떨어졌습니다 ㅜㅜ 효율적인 방법이 있다면 제시해주시면 감사하겠습니다. 🙏🏻

Copy link
Member

@InSeong-So InSeong-So left a comment

Choose a reason for hiding this comment

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

안녕하세요, 경현님!
코드 전반에 피드백이 잘 녹아 있네요. if-else 구문에 대해서만 코멘트를 남겼는데...
나머지는 도메인 모델과 설계가 잘 되어 있어서 크게 코멘트하기 보다,
PR에 적어주신 내용으로 답변하면 어떨까 싶었어요.

조건문 합치기

  • 조건문을 만약 다른 사람이 본다면 어떨지 생각해보면 좋겠습니다. 현재 수정한 바와 같이, 한 번에 이해가 되는 열거형 조건문이 아니라면 이 매직 스트링(0, 혹은 그 외)은 어떤 의도인지 잘 모르기 때문에 요구사항을 다시 체크해야 하거든요.
  • 조건문 합치기, 조건문 캡슐화... 무엇이 되었든 가독성이 좋아지면 된답니다. 조건문 한정 직관성 = 가독성이거든요. 소괄호 안에서 일어나는 true/false 판단문은 누가 읽어도 이해하기 쉬워야 한답니다.

_ 접두어

  • 자바스크립트는 # 접두어, 타입스크립트는 private 를 적용하면 될 것 같네요.
  • 언더바는 왜 사용하는지 알아두면 좋을 것 같습니다. ex) lodash

switch-case 구문

  • lookup table이 흔하게 사용되는 명칭이고, 객체로 핸들링하는 것이 코드도 깔끔하고 이해하기 쉽거든요. 더 나아가서 Map, Set을 활용해도 좋답니다.

난해했던 점: 함수형 프로그래밍

  • 함수형 프로그래밍이 유연하다...? 유연한 것은 오히려 객체 지향 프로그래밍입니다. 함수형 프로그래밍은 순수 함수로써의 규칙을 지키기 위해 오롯이 bottom-up 형식의 함수 개발이 주체라서, 기능 단위 테스트도 정말 많이 진행해봐야 하거든요.
  • 단순히 "함수"로 정의한다고 순수 함수가 아닙니다. 가장 간단한 원칙인 동일한 입력에 대해 동일한 출력을 코드로 구현해서 앱의 핵심 기능까지 만들어야 하거든요.
  • 액션-계산-데이터 관점에서의 함수형 프로그래밍을 좀 더 찾아보면 좋겠네요!

store

  • store가 컨트롤러처럼 쓰이는 게 문제라고 생각됩니다. 그렇다면 그냥 controller로 이름 짓고, 저장하고 있던 데이터를 "분리"하거나 모델에 "위임"하면 될 것 같아요.
  • store를 다른 프레임워크처럼 사용하려고 한 시도는 정말 좋았어요! 다만 연산이 주체가 되면 안되고, 연산의 하위 수단으로써 사용되는 것이 베스트입니다. 그 경계는 도메인 모델에 어떤 영향을 미치는지 잘 추스려보면 될 것 같습니다.
  • 어떻게 빼지...? 이건 여기서 사용하고 얘는 여기서 사용하는데... 라고 느껴지면, 의존도가 높진 않을까 고민해보면 좋겠습니다.

jsDoc

  • 저는 함수에만 사용한답니다. 단순히 객체는 키-값만 유지하면 되지 않을까요?
  • 우리가 주석을 통해 추론을 하는 이유는 "어떤 값인지 몰라서"이기 때문입니다. 객체의 키는 에디터 레벨에서도 추론이 되므로 이러한 행위는 다소 불필요할 수 있을 것 같아요.

테스트코드

  • 순수 함수라는 단어가 이렇게 튀어나와서 좀 당황스럽지만... 함수형 프로그래밍에서 TDD는 순수 함수의 철학에 의거하지, 앱의 요구사항에 의거하는 것이 아닙니다.
  • 동일한 입력 = 동일한 출력이라는 명제를 갖고 시작하는 순수 함수는, 앞의 전제가 무조건 참이기 때문에 순수 함수 + 순수 함수 또는 순수 함수 * 순수 함수라도 참이라는 결과를 갖기 때문이에요.
  • TDD가 옳은가? 라면 당연히 상황에 따라 다를 수 밖에 없으나, TDD가 좋은가? 라고 한다면 좋을 수 밖에 없습니다. 테스트 기반으로 앱을 만들었기 때문에, 해당 테스트가 통과하면 "앱이 잘 동작한다"라는 전제를 얻기 때문이에요.
  • 현업에서 사용하긴 쉽지 않으나, 그렇다고 나쁜 방법론은 아니기 때문에 적재적소에 필요한 기능/함수에 대해 잘 적용하면 좋겠습니다.

에러 핸들링

  • 현재 에러 핸들링은... 사실 함수 단위로 try-catch를 적용해둔 것 뿐이라고 생각합니다.
  • react를 고려해보면 앱 최상위에서 ErrorBoundary를 썼을 때 대부분의 클라이언트 에러를 핸들링할 수 있죠. 그 이유는 무엇일까요?
  • 에러의 발생을 어떻게 제어할지 고민해보면 생각보다 답이 쉽게 나옵니다. try-catch 와 throw를 잘 활용해보면 전역 핸들링이 가능해질 수 있죠.
  • 도메인 단위로 에러 핸들링이라는 접근은 경현님을 더 많이 괴롭히지 않을까 싶습니다.

고생하셨습니다!
코드 정말 잘 작성해주셨고, 생각이나 관념에 대한 PR 코멘트만 잘 읽어봐도 좋을 것 같아요.
이만 머지하도록 할게요! 🥹

Comment on lines +83 to +89
if (restartOrExitLotto.toLowerCase() !== "y") {
return rl.close();
} else {
console.clear();
initializeDataStorage();
return startLottoGame();
}
Copy link
Member

Choose a reason for hiding this comment

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

스코프를 줄인다는 것은 아래와 같은 맥락이랍니다!
{ } 스코프는 많이 있으면 가독성을 해치거든요.

Suggested change
if (restartOrExitLotto.toLowerCase() !== "y") {
return rl.close();
} else {
console.clear();
initializeDataStorage();
return startLottoGame();
}
if (restartOrExitLotto.toLowerCase() !== "y") {
return rl.close();
}
console.clear();
initializeDataStorage();
startLottoGame();

@InSeong-So InSeong-So merged commit 909c20d into next-step:frankuserconnect Sep 5, 2023
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.

None yet

2 participants