[1단계 - 콘솔 기반 로또 게임] 윤생(이윤성) 미션 제출합니다. #173

2yunseong · 2023-02-16T08:18:26Z

안녕하세요~ 포코! 반갑습니다!! 윤생입니다. :D
콘솔 기반 로또 게임 제출합니다.
TDD라는 개념도 처음 접하고, 실제로 해보려고 하니까 많이 어렵네요!! ㅠㅠ 생각보다 테스트를 많이 신경쓰지 못한것 같아 아쉽습니다 ㅠ
문제를 작은 단위로 정의하는게 아직도 어려운 것 같아요!! 기능 요구사항을 작성할 때도, 실제 TDD에서 테스트를 어디까지 나누어 접근해야할지 고민이 많았습니다.
궁금한 점은 피드백 받으면서, 점진적으로 채워나가도록 할께요!!

궁금한 점

유효성 검사를 지금은 여러 클래스에서 별다른 규칙없이 하고 있는데, 유효성 검증 로직은 누가 들고 있어야 할까요 ? 그 부분이 궁금합니다. 각 클래스가 생성될때나 설정될 때 들고 있으면 좋을까요? 아니면 입력을 받자마자 유효성을 검증하는게 좋을까요?

Co-authored-by: 2yunseong<dbsdltjd3701@naver.com>

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

pocojang · 2023-02-17T08:46:26Z

.prettierrc

@@ -1,3 +1,11 @@
 {


기본 설정들과 얼마나 겹치는지 확인해보세요~

pocojang · 2023-02-17T08:49:27Z

__tests__/Lotto.test.js

+    const lotto = new Lotto([1, 2, 3, 4, 5, 6]);
+    const bonusBall = 10;
+
+    expect(lotto.hasBonus(bonusBall)).toBe(false);


toBeFalsy() 👍

pocojang · 2023-02-17T08:50:20Z

__tests__/validation.test.js

+  });
+
+  test.each(['y', 'n'])('재시작 입력에서 y와 n를 받으면 true를 반환한다.', (input) => {
+    expect(isValidRestartCommand(input)).toBe(true);


toBeTruthy() 👍

pocojang · 2023-02-17T08:53:46Z

docs/REQUIREMENTS.md

+- ~~변수 선언시 var를 사용하지 않는다. let, const를 사용한다.~~
+- 전역 변수를 만들지 않는다.
+- 축약하지 않는다.
+- 하드 코딩된 값 대신에 의미 있는 상수를 활용한다.
+- 동등 연산자는 === 로만 사용한다.
+- 함수(또는 메서드)의 길이가 10라인을 넘어가지 않도록 구현한다.
+- 함수(또는 메서드)가 한 가지 일만 하도록 만든다.
+- ~~함수(또는 메서드)의 들여쓰기 depth는 2단계까지만 허용한다.~~
+  - ~~예를 들어 while문 안에 if문이 있으면 depth는 2단계 이다.~~


취소선은 왜 있는걸까요?

이번 미션의 프로그래밍 요구사항이 depth는 1단계까지만 허용한다고 해서, 이 부분은 취소선으로 표기했습니다!
이전 미션의 프로그래밍 요구사항은 기본으로 포함한다고도 명시되어서, 확인차 복사해 가져왔습니다.

pocojang · 2023-02-17T08:54:49Z

src/App.js

+  static RESTART_COMMAND = 'y';
+  static EXIT_COMMAND = 'n';


음.. 위치가 여기가 맞을까요?

2yunseong · 2023-02-17T11:10:04Z

src/step1-index.js

@@ -1,4 +1,9 @@
+import App from './App';


혹시 어떤 부분 말씀하시는지 알 수 있을까요?

아닙니다! 해결되었답니다 :)
제가 요구사항을 착각했어요!

pocojang · 2023-02-17T09:07:53Z

src/view/console/InputView.js

+  },
+
+  async readWinNumbers() {
+    // to-do: 중간에 띄어쓰기 된 입력값 허용하고 있는데 허용하지 않을지


// TODO:!

pocojang · 2023-02-17T09:08:36Z

src/view/console/InputView.js

+    const command = await Console.read(InputView.RESTART_QUERY);
+    if (!isValidRestartCommand(command)) {
+      return new Error(this.INVALID_COMMAND_ERROR);
+    }
+    return command;


차라리 try 또한 같이 쓰는게 낫지 않을까요?

pocojang · 2023-02-17T09:08:58Z

src/view/console/OutputView.js

@@ -0,0 +1,66 @@
+import Console from './Console';
+
+const OutputView = {


OutputView에 로또 관련 로직이 너무나 많군요..

현재 OutputView에서는 데이터를 받아 출력형식에 관련된 메세지를 만들어주는 (20000 -> 20,000원) 등의 로직만 가지고 있습니다. 원래 의도도 그런것이 였는데, 포코의 생각은 어떠신가요?

원래 의도가 그렇다면 괜찮습니다~
다만 View 다운 View는 아닌 것 같아요!
그저 파일만 분리된 느낌이랄까요?

pocojang · 2023-02-17T09:09:20Z

src/view/console/InputView.js

+  BUY_MONEY_QUERY: '구입금액을 입력해 주세요.',
+  WIN_NUMBERS_QUERY: '당첨 번호를 입력해 주세요.',
+  BONUS_QUERY: '보너스 번호를 입력해 주세요.',
+  RESTART_QUERY: '다시 시작 하겠습니까? (y/n)',


InputView에 로또 관련 로직이 너무나 많군요..

혹시 로또에 관련된 입력을 받는 부분을 말씀하신 건가요?

pocojang

안녕하세요
@2yunseong 👋
반갑습니다

저는 리뷰어 포코라고합니다 :) 잘 부탁드려요~

피드백은 댓글로 확인 부탁드리고요!

리뷰

클래스

클래스를 언제? 왜? 사용해야하는지
static 키워드를 언제? 왜? 사용해야하는지
내부 멤버 변수(필드)는 언제 필요한지..

이외에 다양한 부분을 고민해보셨으면 좋겠습니다.

개행

우테코 5기분들 대부분 그런건데.. 코드보기 너무 힘듭니다.
코드 라인을 다닥다닥 아무런 개행도 없어서 정말 유지보수나 확장성에 어려움이 예상되는데 오히려 편하실까요?

Q&A

Q. 유효성 검사를 지금은 여러 클래스에서 별다른 규칙없이 하고 있는데, 유효성 검증 로직은 누가 들고 있어야 할까요 ? 그 부분이 궁금합니다. 각 클래스가 생성될때나 설정될 때 들고 있으면 좋을까요? 아니면 입력을 받자마자 유효성을 검증하는게 좋을까요?

유효성 검사를 왜 해야하고 유효성 검사로 무엇을 해야하는지부터 잘 파악해야할 것 같아요.
말씀하신 테스트 얘기도 마찬가지인데요.
학습에 몰입하는 것은 좋지만 학습에 매몰되지말고 안전한 앱을 만들기 위한 방향으로 고민해보시는게 어떠실까요? 이럴때는 그림을 그리면서 연습해보시는 것도 좋아요

코드 수정 후 다시 리뷰 요청해주세요!

- toBe => toBeTruthy, toBeFalsy

2yunseong

포코 피드백 감사합니다! 

특히 학습에 매몰되지말고 안전한 앱을 만들기 위한 방향으로 고민해보라는 포코의 커멘트를 읽고는 머리를 망치로 한대 맞은 기분이였습니다! 저 자신을 돌아보니 학습에 매몰되어 왜 이렇게 하는지 의문은 별로 품지 않은 채 가까운 목표에만 집중했던것 같습니다. 감사합니다.  

말씀 주셨던 WIN_PRIZE_MONEY에 대해서는 더 많은 고민이 필요해 일단은 커멘트 주신 부분 반영한 것과 커멘트에서 궁금한 부분 남겨 리뷰요청 드립니다!!  

 감사합니다! 즐거운 주말 되세요!! 

2yunseong · 2023-02-17T10:53:40Z

docs/REQUIREMENTS.md

+- ~~변수 선언시 var를 사용하지 않는다. let, const를 사용한다.~~
+- 전역 변수를 만들지 않는다.
+- 축약하지 않는다.
+- 하드 코딩된 값 대신에 의미 있는 상수를 활용한다.
+- 동등 연산자는 === 로만 사용한다.
+- 함수(또는 메서드)의 길이가 10라인을 넘어가지 않도록 구현한다.
+- 함수(또는 메서드)가 한 가지 일만 하도록 만든다.
+- ~~함수(또는 메서드)의 들여쓰기 depth는 2단계까지만 허용한다.~~
+  - ~~예를 들어 while문 안에 if문이 있으면 depth는 2단계 이다.~~


이번 미션의 프로그래밍 요구사항이 depth는 1단계까지만 허용한다고 해서, 이 부분은 취소선으로 표기했습니다!
이전 미션의 프로그래밍 요구사항은 기본으로 포함한다고도 명시되어서, 확인차 복사해 가져왔습니다.

2yunseong · 2023-02-17T10:57:02Z

src/view/console/Console.js

+import * as readline from 'node:readline/promises';
+import { stdin as input, stdout as output } from 'node:process';


넵! 이번부터는 미션 유틸 사용을 금지하고 있습니다!

2yunseong · 2023-02-17T11:00:34Z

src/App.js

+import InputView from './view/console/InputView';
+
+class App {
+  // 고민해볼 것 : 해당 커맨드를 웹 에서는 어떻게 사용할지?, 위치가 컨트롤러로 가야할 지?


네 알겠습니다!!

2yunseong · 2023-02-17T11:04:27Z

src/LottoController.js

+      this.#lottoMachine = new LottoMachine(money);
+    } catch (error) {
+      OutputView.printErrorMsg(error.message);
+      await this.readBuyMoney();


입력을 받으면 다음 로직을 진행하고, 만약 에러가 발생하면 에러메세지를 보여주고 사용자에게 다시 콘솔 입력을 받는 걸 의도했습니다!

2yunseong · 2023-02-17T11:05:28Z

src/LottoController.js

+      this.#lottoMachine.generateWinningLotto(winNumbers);
+    } catch (error) {
+      OutputView.printErrorMsg(error.message);
+      await this.readWinNumbers();


절차적으로 생각하여 이렇게 구현하였습니다! 혹시 다른 방법이 있을까요?

2yunseong · 2023-02-18T08:20:41Z

src/view/console/OutputView.js

@@ -0,0 +1,66 @@
+import Console from './Console';
+
+const OutputView = {


현재 OutputView에서는 데이터를 받아 출력형식에 관련된 메세지를 만들어주는 (20000 -> 20,000원) 등의 로직만 가지고 있습니다. 원래 의도도 그런것이 였는데, 포코의 생각은 어떠신가요?

2yunseong · 2023-02-18T08:32:21Z

src/view/console/InputView.js

+  BUY_MONEY_QUERY: '구입금액을 입력해 주세요.',
+  WIN_NUMBERS_QUERY: '당첨 번호를 입력해 주세요.',
+  BONUS_QUERY: '보너스 번호를 입력해 주세요.',
+  RESTART_QUERY: '다시 시작 하겠습니까? (y/n)',


혹시 로또에 관련된 입력을 받는 부분을 말씀하신 건가요?

2yunseong · 2023-02-18T08:33:23Z

src/domain/LottoMachine.js

+
+class LottoMachine {
+  static LOTTO_COST = 1000;
+  static WIN_PRIZE_MONEY = { 0: 0, 1: 2000000000, 2: 30000000, 3: 1500000, 4: 50000, 5: 5000 };


이 부분에 고민해보았는데 한번 더 고민해보도록 하겠습니다.. ㅠㅠ 일단 떠오르는 건 프로퍼티명을 매직넘버로 바꾸는 거네요!

2yunseong · 2023-02-18T09:02:04Z

src/domain/Lotto.js

+    const matchNumbers = this.#numbers.filter((number) => {
+      return other.#numbers.includes(number);
+    });


에러가 발생합니다 ㅠ 아마도 private 설정때문에 그런 것 같습니다! 에러가 발생하는 이유도 한번 찾아볼께요! 좋은 제안 감사합니다!

var matchNumbers = _classPrivateFieldGet(this, _numbers).filter(_classPrivateFieldGet(other, _numbers).includes); ^ TypeError: Cannot convert undefined or null to object at includes (<anonymous>) at Array.filter (<anonymous>) at Lotto.countMatch (/Users/iyunseong/GitHub repo/javascript-lotto-1/dist/step1-bundle.js:370:64) at WinningLotto.getWinRank (/Users/iyunseong/GitHub repo/javascript-lotto-1/dist/step1-bundle.js:669:30) at /Users/iyunseong/GitHub repo/javascript-lotto-1/dist/step1-bundle.js:503:60 at Array.map (<anonymous>) ... 생략 ```

2yunseong · 2023-02-18T09:04:16Z

src/App.js

+  #lottoController;
+
+  constructor() {
+    this.#lottoController = new LottoController();


넵! 찾아보니 상속과 조합 대해 고민해볼 내용이라고 생각이 듭니다! (상속보단 조합을 사용하자)
감사합니다 :D

pocojang

@2yunseong
좋은 코드를 작성하기 위해 고민도 많이 하시고 리뷰 요청도 상당히 빠르시네요 👍
배열 메서드도 상당히 잘 활용하시는 것 같네요 👏

간단 리뷰

에러 처리를 어디에 위임할지 고민해보시면 좋겠네요
static 이 과하게 많이 사용되는 것 같습니다
클래스 내부에 불필요한 필드가 많은 것 같은데 어떠세요?

이만 머지할테니 다음 스텝에서 뵙도록 할게요!

pocojang · 2023-02-19T12:00:32Z

src/step1-index.js

@@ -1,4 +1,9 @@
+import App from './App';


아닙니다! 해결되었답니다 :)
제가 요구사항을 착각했어요!

pocojang · 2023-02-19T12:01:33Z

src/domain/LottoRank.js

@@ -0,0 +1,27 @@
+const LottoRank = {


제 의견은 중요하지 않을 것 같습니다 :)
@2yunseong 의 의도가 더 중요하고용
이런 저런 시도해보면서 고민해보시면 되지요!

pocojang · 2023-02-19T12:02:12Z

src/domain/LottoMachine.js

+
+class LottoMachine {
+  static LOTTO_COST = 1000;
+  static WIN_PRIZE_MONEY = { 0: 0, 1: 2000000000, 2: 30000000, 3: 1500000, 4: 50000, 5: 5000 };


매직 넘버 말고도 나중에는 다양한 시도를 해보실 수 있을거에요~

pocojang · 2023-02-19T12:02:29Z

src/domain/Lotto.js

+    const matchNumbers = this.#numbers.filter((number) => {
+      return other.#numbers.includes(number);
+    });


엇 이런.. 안되는군요 😢

pocojang · 2023-02-19T12:03:57Z

src/domain/Lotto.js

+    return numbers.every(Lotto.isValidLottoNumber);
+  }
+
+  #numbers = [];


그렇군요!
개인 차도 있을 것 같은데 저는 생성자의 위치가 상위에 있어야한다고 생각하는 편이에요.
그래야 빠르게 코드도 파악할 수 있으니까요!
우리가 책을 읽을때 왼쪽에서 오른쪽 위에서 아래로 읽듯이 생성자와 친해지기 위해 위에서 본다면 당연히 생성자도 윗 부분에 있어야하지 않을까 생각합니다 후후..

일단 더 고민해보시면서 스스로 기준을 만들어보시지요!

pocojang · 2023-02-19T12:05:40Z

__tests__/validation.test.js

+import { isPositiveInteger, isValidRestartCommand } from '../src/validation';
+
+describe('유효성 검증 테스트입니다.', () => {
+  test.each([1.11, null, undefined, 'string', {}])(


음 사용자가 정말로 다룰 만한 값으로 테스트 해보는건 어때요?

pocojang · 2023-02-19T12:17:29Z

src/view/console/OutputView.js

+  generateProfitRateMessage(profitRate) {
+    return `총 수익률은 ${profitRate.toFixed(2)}%입니다.`;
+  },
+  // to-do : 메서드명 나중에 변경하기


https://marketplace.visualstudio.com/search?term=todo&target=VSCode&category=All%20categories&sortBy=Relevance

TODO 주석을 보통 어떻게 만드는지 확인해보세요 :)

pocojang · 2023-02-19T12:20:00Z

src/view/console/OutputView.js

+    const rankLength = 5;
+    const results = Array.from({ length: rankLength }, (_, i) => {
+      return `${this.WIN_CONDITION[rankLength - i]} ${this.generateWinPrizeMoneyMessage(
+        winPrizeMoney,
+        rankLength - i,
+      )} - ${this.generateWinCountMessage(winCount, rankLength - i)}`;
+    });
+    results.forEach((result) => Console.print(result));


물론 어마어마한 연산은 아니지만 불필요하게 루프를 더 돌리는 것 같아 뭔가 찝찝한 느낌이 들긴 하네요 ㅎㅎ

pocojang · 2023-02-19T12:20:14Z

src/view/console/OutputView.js

+  },
+
+  generateWinPrizeMoneyMessage(winPrizeMoney, rank) {
+    return `(${winPrizeMoney[rank].toLocaleString('ko-KR')}원)`;


적절한 API 활용 좋습니다 👍

pocojang · 2023-02-19T12:25:13Z

src/view/console/OutputView.js

@@ -0,0 +1,66 @@
+import Console from './Console';
+
+const OutputView = {


원래 의도가 그렇다면 괜찮습니다~
다만 View 다운 View는 아닌 것 같아요!
그저 파일만 분리된 느낌이랄까요?

cruelladevil and others added 30 commits February 14, 2023 18:43

docs: 요구사항 목록 작성

286ba48

Co-authored-by: 2yunseong<dbsdltjd3701@naver.com>

feat: 콘솔 인터페이스 구현

e8edb1a

Co-authored-by: 2yunseong<dbsdltjd3701@naver.com>

feat: 입/출력 UI 로직 작성

003f199

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

test: Lotto 클래스 테스트 추가

77eaa13

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: Lotto 클래스 구현

8951221

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

test: validation 유틸 함수 테스트 추가

771bc53

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: validation 유틸 함수 구현

385b61b

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: RandomGenerator 구현

7b7b2a5

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: WinningLotto 클래스 구현

fed7cd4

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: LottoMachine 클래스 구현

e183e9f

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: LottoController 클래스 구현

7e7c120

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: App 구현

83f4d4f

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: validation 추가

d38e64f

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

test: 메서드 변경에 따른 테스트 수정

2e08722

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

feat: 도메인 로직 변경에 따른 OutputView 수정

987b906

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

docs: 완료한 기능 체크

1ea68ab

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

chore: eslint 및 prettier 설정

a0b74d5

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: validation 메서드 추출

6ec9738

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 에러 메세지 상수화

b76fa5a

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 매직넘버 제거

2fec3b5

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 당첨 통계 계산 책임을 LottoMachine에게 위임

bdb3558

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 재사용을 위해 static 메서드로 변경

f934e7b

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 등수 판별 로직 추출

9d7ce91

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: OutputView 리터럴 상수화

201b756

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 커맨드 상수화

50e0b47

Co-authored-by: 2yunseong <dbsdltjd3701@naver.com>

refactor: 에러 메세지 상수화

f8a26cf

refactor: 에러 출력 부분 메서드로 분리

860ae54

refactor: 사용하지 않는 변수 제거

43bc87b

refactor: 리터럴 변수 상수화

e0fc0e2

fix: 잘못된 파라미터 사용 변경

e15c175

pocojang self-requested a review February 16, 2023 09:20

pocojang self-assigned this Feb 16, 2023

pocojang reviewed Feb 17, 2023

View reviewed changes

pocojang requested changes Feb 17, 2023

View reviewed changes

2yunseong added 9 commits February 17, 2023 19:51

test: jest에서 제공하는 메서드 이용

a9a9f88

- toBe => toBeTruthy, toBeFalsy

chore: 주석 변경

9991a5b

refactor: random Generator 범용성을 위한 로또만을 위한 로직 분리

4086f15

refactor: 범용적으로 쓰이는 상수 static에서 분리

b3a3147

refactor: 매직넘버 상수화

027c11b

refactor: 범용적으로 쓰이는 상수 static에서 분리

0d90f66

refactor: 메서드 축약 및 일부 로직 메서드로 분리

e26a21b

refactor: InputView 에러 핸들링 변경

fd155ab

refactor: 필요없는 로직 제거

f24b8cb

2yunseong commented Feb 18, 2023

View reviewed changes

pocojang approved these changes Feb 19, 2023

View reviewed changes

pocojang merged commit 0980cd3 into woowacourse:2yunseong Feb 19, 2023

		@@ -0,0 +1,66 @@
		import Console from './Console';

		const OutputView = {

		import * as readline from 'node:readline/promises';
		import { stdin as input, stdout as output } from 'node:process';

[1단계 - 콘솔 기반 로또 게임] 윤생(이윤성) 미션 제출합니다. #173

[1단계 - 콘솔 기반 로또 게임] 윤생(이윤성) 미션 제출합니다. #173

Conversation

2yunseong commented Feb 16, 2023 • edited Loading

궁금한 점

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pocojang left a comment

Choose a reason for hiding this comment

리뷰

클래스

개행

Q&A

2yunseong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pocojang left a comment

Choose a reason for hiding this comment

간단 리뷰

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong commented Feb 16, 2023 •

edited

Loading