[2단계 - 행운의 로또 미션] 빌리(정환희) 미션 제출합니다. by hwanheejung · Pull Request #378 · woowacourse/javascript-lotto

hwanheejung · 2025-02-26T15:35:47Z

학습 목표

이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.

UI와 도메인 영역을 분리할 수 있는 설계를 고민해보고, 목적에 맞게 객체와 함수를 활용
TDD 방식으로 개발하며, 단위 테스트 기반으로 점진적인 리팩터링

제출 전 체크 리스트

기능 요구 사항을 모두 구현했고, 정상적으로 동작하는지 확인했나요?
기본적인 프로그래밍 요구 사항을 준수하고 있는지 확인했나요?
테스트 코드는 모두 정상적으로 실행되나요?
(해당하는 경우) 배포한 데모 페이지에 정상적으로 접근할 수 있나요?
- 배포한 링크 기입: https://hwanheejung.github.io/javascript-lotto/

리뷰 요청 & 논의하고 싶은 내용

1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점

1. 💡 상태(state) 변경과 UI 업데이트를 어떻게 연결할 것인가?

이번 미션에서 가장 고민했던 부분은 setState()를 호출할 때, UI 전체를 다시 렌더링하지 않고 변경된 부분만 업데이트하는 방법이었습니다.

초기 구현에서는 setState()를 호출하면 this.target.innerHTML = this.template()가 실행되며 전체 UI가 다시 렌더링되었습니다.

그러나 변경된 state에 의존하는 동작만 다시 렌더링되어야 하는데, 불필요한 렌더링이 발생하는 문제가 있었습니다.

🛠 해결 과정

아이디어: state와 UI 업데이트 동작을 미리 연결하자!

상태 변화 감지 (watchState 도입)

React에서는 가상 DOM을 사용해서 변경된 부분을 diffing 알고리즘으로 비교하고, 변경된 부분만 효율적으로 업데이트합니다.

대신 저는 watchState()를 통해 특정 state가 변경됐을 때 실행할 동작을 미리 연결하는 방식으로 해결했습니다.

  setup() {
    this.state = {
      price: 0,
      lottoBundle: [],
      winningNumbers: Array(Lotto.SIZE).fill(""),
      bonusNumber: "",
      opened: false,
    };

    this.watchState("price", this.renderLottoUI.bind(this));
    this.watchState("opened", this.toggleModal.bind(this));
  }

price가 변경되면 renderLottoUI() 실행
opened가 변경되면 toggleModal() 실행

내부 동작 (stateToUIMap 도입)

  watchState(stateKey, callback) {
    this.#stateToUIMap[stateKey] = callback;
  }

특정 state가 변경될 때 실행해야 하는 UI 업데이트 로직을 #stateToUIMap에 저장
setState()가 실행되면, 변경된 키(changedKeys)만 찾아서 해당하는 UI 업데이트 실행

  componentDidUpdate(changedKeys) {
    changedKeys.forEach((key) => {
      if (this.#stateToUIMap[key]) {
        this.#stateToUIMap[key]();
      }
    });
  }

이 방식 덕분에 UI 변경이 좀 더 명확하게 연결되고, 불필요한 렌더링을 줄일 수 있었습니다.

2) 이번 리뷰를 통해 논의하고 싶은 부분

현재 state 기반 UI 업데이트 로직에서 흐름이 명확한지, 불필요한 동작은 없는지 여쭙고 싶습니다.

✅ 리뷰어 체크 포인트

1단계

TDD를 활용해 기능을 구현하는 과정에서 적절한 테스트 우선 접근 방식을 적용했는가? 단위 테스트의 커버리지는 충분한가?
도메인과 UI의 관심사를 분리하여 적절한 모듈화가 이루어졌는가? 하나의 객체나 모듈이 너무 많은 책임을 가지고 있지는 않은가?
객체의 프로퍼티를 직접 조작하기보다 메시지를 던지고 있는가?
불필요한 클래스를 사용하지 않고, 함수를 적극적으로 활용하여 JavaScript다운 방식으로 로직을 구현했는가?

2단계

도메인 로직에 불필요한 영향을 주지 않고 UI 변경에 대응했는가?
DOM 조작과 이벤트 활용을 JavaScript의 개념에 맞게 이해하고 적절하게 적용했는가?
웹 표준을 준수하는 마크업을 활용하며, 스타일 작성에 일관성이 있는가?

JunilHwang

안녕하세요 빌리! 리뷰가 너무 늦어졌네요 ㅠㅠ 죄송합니다.
개인적인 사정이 있다보니... 송구스럽네요.

여튼 미션 잘 진행해주신 것 같아요!
설계에 대한 부분은 큰 이견은 없습니다 ㅎㅎ 다만 디테일한 부분에 대해 리뷰를 몇 가지 남겼으니 확인 부탁드려요!

리액트와 최대한 비슷하게 구현하려고 노력했는데, 더 개선해볼 부분이 있는지 궁금합니다.

개선이라기보단, 모든 부분을 class component로 표현하지 않아도 무방하답니다!
그냥 문자열 템플릿으로 표현해도 되는 부분이 많아보여요.

state 변경과 UI 업데이트를 더 선언적으로 연결할 방법이 있는지 함께 논의해보고 싶습니다.

이건 빌리가 상상하는 모습을 보여주시면 좋겠어요! 어떤 모습을 상상하고 있나요?

JunilHwang · 2025-02-28T16:05:44Z

src/constants/rules.js

+export const Lotto = {
+  SIZE: 6,
+};


Lotto에 대한 상수는 Lotto Class 내부에 정의되어있으면 어떨까요?

class Lotto { static SIZE = 6 }

이렇게요!

JunilHwang · 2025-02-28T16:05:54Z

src/constants/rules.js

+export const LottoShop = {
+  PRICE_UNIT: 1_000,
+  MIN_PURCHASE_AMOUNT: 1_000,
+  MAX_PURCHASE_AMOUNT: 100_000,
+};

-export const MIN_PURCHASE_AMOUNT = 1_000;
-export const MAX_PURCHASE_AMOUNT = 100_000;
+export const LottoNumber = {
+  MIN: 1,
+  MAX: 45,
+};


이 친구들도 사실 상수가 아니라 도메인 클래스로 정의되어야할 것 같아요 ㅎㅎ

엇 그러면

class LottoShop { static PRICE_UNIT = 1_000; static MIN_PURCHASE_AMOUNT = 1_000; static MAX_PURCHASE_AMOUNT = 100_000; }

이렇게 만들어야 하나요?? 새로운 도메인 class가 도입됨에 따라서 역할을 다시 분배해야 하는지 궁금합니다!

JunilHwang · 2025-02-28T16:08:36Z

src/step2-index.js

+document.addEventListener("DOMContentLoaded", () => {
+  new App(document.querySelector("#app"));
+});


DOMContentLoaded 내부에서 로직을 실행하게 되면 문제가 발생할 수 있어요 ㅎㅎ

코드를 로딩하는 시점이 이미 DOMContentLoaded 이벤트가 발생한 이후일 경우, 아예 이 구간으로 진입을 못한답니다.

어차피 module 방식으로 불러오고 있어서, 그냥 이 이벤트는 사용하지 않는게 더 안전할 것 같아요.

JunilHwang · 2025-02-28T16:10:05Z

src/ui/components/core/Component.js

+
+    this.setup();
+    this.initialRender();
+  }


멤버변수로 쓰이는 것들은 클래스 필드에 정의해주세요!
그리고 다 pubilc으로 정의되어 있는데, 접근 제한자도 지정해주시면 좋을 것 같습니다.

JunilHwang · 2025-02-28T16:10:30Z

src/ui/components/core/Component.js

+  componentDidMount() {}
+  componentWillUpdate() {}
+  componentDidUpdate(changedKeys) {}
+  componentWillUnmount() {}


componentWillUnmount 는 아예 안 쓰이고 있네요..!

JunilHwang · 2025-02-28T16:23:14Z

src/ui/components/game/LottoGame.js

+      bonusNumber: "",
+      isModalOpen: false,
+    };
+  }


클래스와 상수를 조화롭게 사용할 수 있답니다.

class LottoGame extends Component { static TITLE = "🎱 내 번호 당첨 확인 🎱";

이렇게 사용할 수 있어요.

LottoGame.TITLE

JunilHwang · 2025-02-28T16:23:40Z

src/ui/components/game/LottoGame.js

+
+  setIsModalOpen(openModal) {
+    this.setState({ isModalOpen: openModal });
+  }


isModalOpen -> opened

이 표현이 더 직관적이지 않을까요!?

JunilHwang · 2025-02-28T16:25:48Z

src/ui/components/modal/Modal.js

+
+  close() {
+    this.props.onClose();
+  }


이 부분이 조금 어색하게 느껴져요.

close란 "닫는다" 라는 행위인데
실제로는 onClose를 실행해주는게 전부네요 ㅎㅎ

이걸 close 라고 부를 수 있는걸까요?

이벤트는 "어떤 일이 발생했을 때 알려주는 것" 이라고 생각해요.
지금은 "이 일을 해줘!" 같은 느낌으로 쓰이고 있습니다.

JunilHwang · 2025-02-28T16:28:18Z

src/ui/components/modal/ResultModal.js

+    super.setup();
+    this.events = {
+      ...this.events,
+      "click@.modal__restart": this.handleClickRestart,


이벤트에서 정의하는 selector를 상수로 만들어서 템플릿에서도 사용해주면 어떨까요!?

JunilHwang · 2025-02-28T16:28:39Z

src/ui/components/modal/ResultModal.js

+
+  handleClickRestart() {
+    this.props.onRestart();
+  }


이 부분도 앞선 리뷰와 동일합니다.

hwanheejung · 2025-03-03T05:27:33Z

@JunilHwang 안녕하세요, 해먼드! 😊
이번에도 꼼꼼한 리뷰 정말 감사드려요! state 변경에 따른 UI 업데이트 로직을 좀더 선언적으로 하기 위한 방법을 고민해서 watchState를 도입해 좀더 선언적으로 업데이트하는 방식으로 수정했습니다. 그래서 PR 메시지도 다시 작성했는데 확인 한번 부탁드립니다!

코멘트 주신 부분도 반영하여 코드 변경 사항이 살짝 생겼는데, 다시 한 번 확인해주시면 감사하겠습니다! 🙌
혹시 더 보완할 점이 있다면 편하게 말씀해주세요. 😊

따뜻한 연휴 되시길 바라요!
감사합니다!

JunilHwang

안녕하세요 빌리!
watchState에 대한 아이디어는 무척 좋네요 ㅎㅎ
여기에 DX(개발자의 경험)을 더 고려해서, 수동으로 값을 watching 하는게 아니라 자동으로 watching 하는 방법도 고민해보시면 좋을 것 같아요!

이 외에 이벤트 네이밍에 대한 피드백을 반복적으로 남겼는데요
이벤트에 대해 고민해보시면 좋겠어요.
이벤트는 특정 행위가 발생했을 때 일어나는 일이지, 행위를 실행해주는건 아니랍니다.

onOpen -> open이 되었을 때 발생하는 이벤트

현재 빌리가 쓰는 모습 -> onOpen 으로 아예 open 시켜버림

onSubmit -> submit이 발생했을 때 일어나는 일
onClick -> 클릭이 발생했을 때 일어나는 일

FE에서는 컴포넌트간의 소통을 할 때 주로 이벤트를 통해서 하곤 합니다. 그런데 이벤트에 대해 제대로된 기준을 세워놓지 않으면 다른 사람들이 작성한 코드를 가져다 사용할 때 굉장히 힘들어요 ㅠㅠ

빌리가 만든 컴포넌트를 다른 사람이 가져다 쓴다고 했을 때

onOpen을 보고 "알아서 오픈되는 기능이 있구만!?" 이라고 생각할텐데 실제로는 아무 일도 일어나지 않는다고 생각해보세요 ㅎㅎ

네이밍에 대한 기준은 오픈소스를 다양하게 참고해보시면 좋답니다.

혹은 직접 "이벤트"와 "함수"가 어떤 차이인지 고민해보시면 좋겠어요.

로또 미션은 여기서 마무리하도록 하겠습니다.
짧은 기간이었지만 조금이나마 빌리에게 도움이 되었길 바랍니다.
고생하셨습니다!

JunilHwang · 2025-03-04T02:38:43Z

src/domain/WinningLotto.js

  evaluate(lotto) {
    const matchCount = lotto.matchCount(this.#winningLotto.getNumbers());
-    const isBonusMatched = lotto.hasBonus(this.#bonusNumber);
+    const isBonusMatched = lotto.has(this.#bonusNumber);


bonusMatched 처럼 표현해도 무방하지 않을까요!?

JunilHwang · 2025-03-04T02:40:12Z

src/ui/components/core/Component.js

+  /** 상태 변경 감시 */
+  watchState(stateKey, callback) {
+    this.#stateToUIMap[stateKey] = callback;
+  }


변경되는 상태를 자동으로 감시하는 방법은 없을까요 ㅎㅎ?
지금은 수동으로 watch를 해야 하는데, 자동으로 하는 방법도 고민해보시면 좋을 것 같아요!

JunilHwang · 2025-03-04T02:43:05Z

src/ui/components/game/LottoForm.js

+    this.validation = useUIValidation();
+    this.events = {
+      [`submit@${LottoForm.SELECTOR.FORM}`]: this.submit.bind(this),
+      [`input@${LottoForm.SELECTOR.PRICE}`]: this.activateButton.bind(this),


bind를 아예 안 하는 방법은 없을까요!?

JunilHwang · 2025-03-04T02:44:02Z

src/ui/components/game/LottoGame.js

+      lottoBundle: this.state.lottoBundle,
+    });
+    new WinningNumbersForm(document.querySelector("#winning-numbers-form"), {
+      onModalOpen: () => this.setOpened(true),


onModalOpen이 의미하는 것은 "모달이 오픈되었을 때 실행한다"인데, 지금은 "모달 오픈"함수를 직접적으로 실행하네요 ㅎㅎ
이게 이벤트로서의 기능을 하는게 맞을까요?

JunilHwang · 2025-03-04T02:44:55Z

src/ui/components/game/LottoGame.js

+    const modalRoot = document.querySelector("#modal-root");
+    if (this.state.opened) {
+      new ResultModal(modalRoot, {
+        onClose: () => this.setState({ opened: false }),


앞선 내용과 동일합니다.

JunilHwang · 2025-03-04T02:44:59Z

src/ui/components/game/LottoGame.js

+    if (this.state.opened) {
+      new ResultModal(modalRoot, {
+        onClose: () => this.setState({ opened: false }),
+        onRestart: () => this.resetGame(),


마찬가지입니다.

JunilHwang · 2025-03-04T02:45:36Z

src/ui/components/Header.js

+
+  return `
+    <header id="header" class="text-title">
+      <h1>${TITLE}</h1>


"🎱 행운의 로또"를 여기에 직접 써도 되지 않을까 싶네요..!

JunilHwang · 2025-03-04T02:46:44Z

src/ui/components/game/LottoForm.js

+  };
+
+  setup() {
+    this.validation = useUIValidation();


setup을 할 때 마다 useUIValidation을 통해 함수를 만들어내고 있어요.
불필요한 연산이라고 생각합니다.. ㅠ

hwanheejung added 22 commits February 24, 2025 21:34

refactor: 리뷰 반영

6c09396

feat: Component class 구현

22d7060

style: 디자인시스템 설정

3d1d231

feat: Header component

957f346

feat: Footer component 구현 및 레이아웃 잡기

f96559f

feat: 로또 구매 후 출력

20c58c3

feat: 당첨 번호 입력 폼

1cbf754

feat: 당첨 번호 state에 저장

376766d

feat: open modal

21cbf79

feat: 결과 계산 및 출력

1a36701

refactor: mounted를 renderChildren으로 네이밍 변경

81e7288

refactor: 당첨 번호 폼에서 불필요한 리렌더링 삭제

a11c857

feat: useUIValidation으로 재활용, price 유효성 검사

c760b26

feat: 버튼 비활성화 및 리렌더링 제거

4579b12

feat: 게임 재시작

1a16f4f

refactor: renderChildren 대신 componentDidMount에서 수행, 불필요한 코드 정리

9f0be88

refactor: 폴더 구조 개편

51b9f45

refactor: step1 startLottoGame console 폴더로 이동

0157731

refactor: 작은 함수로 분리

4d7651d

docs: README 업데이트

c79b960

chore: homepage url 수정

cd0679e

refactor: console validation hook 파일 분리

4070cfd

JunilHwang self-requested a review February 28, 2025 16:02

JunilHwang assigned JunilHwang and hwanheejung Feb 28, 2025

JunilHwang requested changes Mar 1, 2025

View reviewed changes

hwanheejung added 4 commits March 2, 2025 22:33

refactor: class field 명확하게, 네이밍 수정

628d6b4

refactor: Header, Footer는 class일 필요가 없으므로 함수로 변경

6dea74a

refactor: useUIValidation 한 번만 생성되도록

74856e2

refactor: lottoService 폴더를 app에서 services로 변경

9979001

hwanheejung added 6 commits March 3, 2025 00:32

refactor: 이벤트 바인딩 밖에서 하도록 변경

d517595

refactor: event selector 상수 분리

86f6c94

refactor: 메시지 상수 정리

54c62e3

refactor: 상태 변경 watchState로 처리

6ade4b2

refactor: Lotto 상수 도메인으로 이동, selector 상수 수정

7a7f8f2

refactor: 불필요한 코드 정리, docs update

3813057

JunilHwang approved these changes Mar 4, 2025

View reviewed changes

JunilHwang merged commit 7e2d90b into woowacourse:hwanheejung Mar 4, 2025

Conversation

hwanheejung commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

학습 목표

제출 전 체크 리스트

리뷰 요청 & 논의하고 싶은 내용

1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점

1. 💡 상태(state) 변경과 UI 업데이트를 어떻게 연결할 것인가?

2) 이번 리뷰를 통해 논의하고 싶은 부분

1단계

2단계

Uh oh!

JunilHwang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hwanheejung Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hwanheejung commented Mar 3, 2025

Uh oh!

JunilHwang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hwanheejung commented Feb 26, 2025 •

edited

Loading

hwanheejung Mar 3, 2025 •

edited

Loading