[1단계 - 콘솔 기반 로또 게임] 센트(김영우) 미션 제출합니다. #174

kyw0716 · 2023-02-16T08:20:43Z

안녕하세요 포코!! 이번에 제로와 함께 콘솔 기반 로또 게임을 구현한 센트 입니다😀😀

제가 구현한 모델들은 다음과 같이 추상화했습니다!

1️⃣ Lotto 클래스

랜덤한 로또 번호를 입력 받아 해당 값을 필드에 저장해두고, 하나의 로또에 대한 연산을 수행하는 클래스

📝필드

#numbers: 로또 번호
#rank: 로또 등수

🛠️ 메서드

getNumbers(): 로또 번호 반환 기능
getRank(): 로또 등수 반환 기능
caculateRank(): 로또 등수 계산 기능 (등수를 계산하여 rank 필드 값을 채워줌)

2️⃣ Lottos 클래스

구입할 금액이 입력되면 금액에 맞는 개수의 로또를 생성하여 그 로또들을 필드에 저장해두고, 모든 로또들이 필요한 연산을 수행하는 클래스

📝필드

#lottos: 생성된 로또 인스턴스들을 담는 배열
#ranks: 1등부터 5등까지 각 등수에 해당하는 로또의 개수를 담는 배열

🛠️ 메서드

getLottos(): 로또 인스턴스들이 담겨 있는 배열 반환 기능
getAllRanks(): 각 등수에 해당하는 로또들의 개수를 담은 배열 반환 기능
getProfitRate(): 수익률을 반환하는 기능
caculateAllRanks(): 각 로또들의 등수를 계산하여 #ranks필드를 채우는 기능

3️⃣ WinningNumbers 클래스

📝필드

#numbers: 당첨 번호

🛠️ 메서드

getNumbers(): 당첨 번호 반환 기능

4️⃣ BonusNumber 클래스

📝필드

#number: 보너스 번호

🛠️ 메서드

getNumber(): 보너스 번호 반환 기능

⁉️ 궁금한점

1️⃣ 랜덤한 요소에 대한 테스트를 어떻게 진행하는 것이 좋을지 궁금합니다!

이번 미션을 진행하면서 Lottos 클래스를 만들어 구입 금액이 입력되면 랜덤숫자를 만들어 Lotto 인스턴스를 생성해 저장하는 방식으로 구현을 했었습니다. 이때 TDD를 적용해보고자 했는데 Lottos클래스를 테스트 하는데 많은 어려움이 있었습니다. 혹시 이러한 경우에 어떤 방식을 사용하여 테스트를 진행하는 것이 좋을지 궁금합니다!!

2️⃣ 입력값을 저장하기 위해서만 사용하는 클래스를 사용했는데 이러한 방식이 맞는지 궁금합니다!

setter를 사용해 객체 내부의 값을 변경하는 일이 없도록 하기 위해서 입력값을 저장하고자 WinningNumbers, BonusNumber클래스를 사용했었습니다. 이때 저 두 클래스의 존재 의미가 오직 입력값을 저장하기 위해서고, 다른 기능은 수행하지 않기 때문에 혹시 이러한 경우에 사용하는 다른 방식이 있을까 궁금합니다!!!

pocojang

안녕하세요
@kyw0716 👋
반갑습니다.

저는 리뷰어 포코라고 합니다!

소소한 리뷰

eslint 강제 주석이 너무 많습니다.
린트가 동작하고 있나요? 일관성 없이 작성된 포맷팅이 계속 발견됩니다.
불필요하고 과한 분리들이 반복됩니다
함수 혹은 메서드가 적절히 분리되지 않아 여러 일을 반복합니다.
예외 처리 방법이.. 애매한 것 같아요. 더 좋은 방법을 고민해보세요!

Q&A

Q1. 랜덤한 요소에 대한 테스트를 어떻게 진행하는 것이 좋을지 궁금합니다!

스택오버플로우에 랜덤 테스트에 대해 검색해보시면 정말 훌륭한 의견들을 볼 수 있답니다.
한번 참고해보세요. 정말 재미있답니다.

Q2. 입력값을 저장하기 위해서만 사용하는 클래스를 사용했는데 이러한 방식이 맞는지 궁금합니다!

의도에 동의가 되는 사용 방식은 아닌 것 같습니다.
저도 설명을 들어서 이제서야 이해했고 코드를 다시 봤지만.. 그런 의도대로 작성된 코드가 맞나? 의문이 드네요 😢

몇가지 코멘트 리뷰도 남겼으니 참고해보세요!

pocojang · 2023-02-17T13:36:05Z

.eslintrc.json

@@ -1,11 +1,44 @@
 {


OMG... 😱
프론트엔드 개발자라면 다양한 확장자를 주의해서 다뤄야합니다.
HTML.. CSS... JS 모두 주석 사용 방법도 다른데요. JSON은 설정 파일이기때문에 더 주의해야합니다.

JSON은 누가? 왜? 무엇을 대체하기 위해 만들어졌을까요?

JSON에서는 주석이 가능할까요?

현재 이 JSON을 파싱해보면 잘 동작할까요?

JSON에 주석이 지원되지 않는다는 것을 모르고 그동안 사용했던 것 같네요 😭😭
수정한 커밋: bda4f74

추가로 주신 질문들을 보고 학습 한 내용입니다!

Q: JSON은 누가? 왜? 무엇을 대체하기 위해 만들어졌을까요?
A: XML로 정보를 주고 받을 때 헤더와 태그 등 여러 요소들로 인해 가독성이 떨어지고, 쓸데 없는 용량을 잡아먹는다는 단점을 갖고 있어 이를 대체하기 위해 더글라스 크록포드에 의해 만들어졌습니다!

Q: JSON에서는 주석이 가능할까요?
A: JSON에는 주석을 지원하지 않아 설정파일에 주로 사용됩니다! (이걸 몰랐네요 ㅠㅠㅠ)

Q: 현재 이 JSON을 파싱해보면 잘 동작할까요?
A:

const json = '{"hi": true //주석}'; const obj = JSON.parse(json); console.log(obj);

주석이 있을 때 파싱이 이루어질 수 있는지 판별해보고자 간단한 예시 코드를 실행시켜 보았더니 오류가 발생하는 것을 확인할 수 있었습니다😭

오 완벽합니다!!
JSON에게 어쩌면 주석조차도 데이터가 아닐까요? 후후

pocojang · 2023-02-17T13:36:47Z

.vscode/settings.json

@@ -0,0 +1,15 @@
+{


여기도..ㅠㅠ주석이..

pocojang · 2023-02-17T13:39:17Z

__tests__/Lotto.test.js

+
+    const profit = calculateProfit(lotto.getRank());
+
+    expect(profit).toBe(expectedProfit);


expectedProfit이 아니라 다른 네이밍으로 가야 기대하는 테스트 결과물과 비교하지 않을까요?
expect()에 이미 의미가 포함되어 있으니까요

계산 되어져 나온 결과라는 의미를 담고자 네이밍을 expected => caculated로 변경했습니다!

수정한 커밋: 2636e6b

pocojang · 2023-02-17T13:39:59Z

__tests__/Lottos.test.js

+    const bonusNumber = 7;
+
+    lottos.calculateAllRanks(winningNumbers, bonusNumber);
+    //console.log(lottos.#calculateProfitRate(), lottos.getAllRanks());


console.log를 다 지웠다고 생각했는데 남아있는게 있었네요😭😭 바로 수정했습니다!

수정한 커밋: 53a430a

pocojang · 2023-02-17T13:41:14Z

docs/REQUIREMENTS.md

@@ -0,0 +1,64 @@
+# 기능 목록


체크박스가 없는 것을 보면.. 다 했다고 볼 수 있는 것일까요?!

체크박스 추가했습니다! 진행상황을 중간중간 체크하기 위해서 체크박스를 쓰는 것이 좋을 것 같네요!! 감사합니다😀😀

수정된 커밋: c308849

pocojang · 2023-02-17T13:52:35Z

src/domain/model/Lottos.js

+  }
+
+  #calculateProfit() {
+    const initialValue = 0;


음 따로 빼낼 이유가 있을까요?

따로 빼지 않는 방식으로 수정 완료 했습니다!

수정한 커밋: 01c60f5

pocojang · 2023-02-17T13:52:58Z

src/domain/model/Lottos.js

+    const initialValue = 0;
+
+    return this.#ranks.reduce((profit, rankCount, index) => {
+      return profit + rankCount * profitByRank[index];


연산 순서를 명시하는게 좋지 않을까요?

수정 완료했습니다!

수정한 커밋: d24cb62

pocojang · 2023-02-17T13:54:21Z

src/domain/model/Lotto.js

+  }
+
+  #setRankTwoOrThree(bonusNumber) {
+    this.#rank = this.#numbers.includes(bonusNumber) ? 2 : 3;


열심히 상수를 만드시던데 2, 3을 너무 날로 작성하시는군요! 휴먼 에러가 발생되지 않을까요?

상수를 만드는 이유 중에 하드코딩된 값의 역할을 명확히 하기 위함도 있다는 것을 이 부분에서 생각하지 못했던거 같습니다. 하드코딩된 값 모두 상수처리하여 수정 완료했습니다!!

수정한 커밋: 7485474

pocojang · 2023-02-17T13:55:41Z

src/controller/LottoGameController.js

+    winningNumbers: undefined,
+    bonusNumber: undefined,


음 숫자를 다루는데 초기 값을 undefined로 사용할때 문제는 없으신가요?

WinningNumbers와 BonusNumber 클래스를 컨트롤러에서 사용하기 때문에 생성된 로또들에 대한 정보까지 합하면 인스턴스 변수를 3개 이상 사용하지 말라는 요구사항을 지키지 못해 그걸 통합하고자 위와 같은 코드가 탄생하게 됐습니다...😭😭

저렇게 하는 방식보단 WinningLotto로 한꺼번에 두 값들을 가지고 있게 리팩터링 하면 어떨까 싶어 수정해봤습니다!!

수정한 커밋: 22e668a

pocojang · 2023-02-17T14:01:40Z

src/domain/model/Lotto.js

+
+  #setRank(correctNumberCount, bonusNumber) {
+    if (correctNumberCount === correctCountPerRank.SECOND_RANK)
+      return this.#setRankTwoOrThree(bonusNumber);


음 굳이 분리할 이유가 있을까요?

depth를 1까지만 허용한다는 요구사항과 함수의 길이를 10으로 제한한다는 요구사항을 충족하려다 최대한 생각한 방식이 저런 방식이었습니다😭😭

before: 하나의 랜덤 값을 생성하고, 빈 배열에 중복되지 않게 그 값을 채워넣고 생성하는 것을 반복하여 랜덤 로또 번호를 생성 after: 최소 ~ 최대까지의 숫자를 전부 포함하는 배열을 생성하여 랜덤하게 섞은 후 로또 번호의 개수만큼 잘라서 반환하는 방식으로 로또 번호 생성

pocojang

@kyw0716

코드를 상당 부분 개선하셨네요!
아래의 항목만 점검해보시면 좋겠습니다!

사용자의 애플리케이션 사용 흐름을 고려한 TDD
불필요한 추상화 기준
에러 처리에 대한 고민
네이밍 동사 위치 재점검
과한 코드 라인 축소 및 불안한 포매팅 점검

물론 이외에도 아쉬움이 많이 들 수 있겠지만 차근차근 2단계까지 함께 진행해보아요
그럼 2단계에서 뵙겠습니다!

pocojang · 2023-02-20T09:03:57Z

.eslintrc.json

@@ -1,11 +1,44 @@
 {


오 완벽합니다!!
JSON에게 어쩌면 주석조차도 데이터가 아닐까요? 후후

pocojang · 2023-02-20T09:04:25Z

.eslintrc.json

    "es6": true,
    "node": true
  },
  "parserOptions": {
    "ecmaVersion": "latest"
  },
-  "extends": ["eslint:recommended", "plugin:prettier/recommended"]
+  "extends": ["airbnb", "plugin:prettier/recommended"],
+  "rules": {


5기는 ESLint 경쟁이 한참인가보군요 ㅋㅋ

pocojang · 2023-02-20T09:05:29Z

.vscode/settings.json

+    "source.fixAll.eslint": true
+  },
+  "eslint.alwaysShowStatus": true,
+  "prettier.disableLanguages": ["js"],


아니 이것은... 👀

pocojang · 2023-02-20T09:05:56Z

.vscode/settings.json

+  },
+  "eslint.alwaysShowStatus": true,
+  "prettier.disableLanguages": ["js"],
+  "files.autoSave": "onFocusChange"


"[javascript]": { "editor.formatOnSave": true },

위의 설정이랑 겹치지 않을까요?

pocojang · 2023-02-20T09:06:36Z

__tests__/Lottos.test.js

+/*eslint-disable */
+const Lottos = require("../src/domain/model/Lottos");
+
+describe("Lottos 테스트", () => {


아주 깔끔한 테스트군요 ㅎㅎ
GWT 까지 좋아요

pocojang · 2023-02-20T09:14:45Z

src/domain/model/Lottos.js

+    const lottos = [];
+
+    while (lottos.length < lottoCount) {
+      lottos.push(randomNumberGenerator.generateLottoNumbers());
+    }
+
+    this.#lottos = lottos.map((lottoNumber) => new Lotto(lottoNumber));


루프를 한번에 할 수 있지 않을까요?
불필요한 연산같기도 하고 오히려 좋은 가독성은 좋기도하고 애매하네요

pocojang · 2023-02-20T09:15:42Z

src/domain/model/Lottos.js

+      lotto.calculateRank(winningNumbers, bonusNumber);
+
+      this.#calculateRank(lotto.getRank());


처음 협업하는 입장에서 아래와 같은 코드가 하나의 함수에 포함되어있다면 맥락 파악이 잘 될까요?
lotto.calculateRank() & this.#calculateRank()

pocojang · 2023-02-20T09:16:10Z

src/domain/model/Lottos.js

+  }
+
+  #calculateRank(lottoRank) {
+    if (lottoRank === undefined) return;


에러를 던질 필요는 없은걸까요?

pocojang · 2023-02-20T09:16:52Z

src/domain/model/Lottos.js

+    const profitRate = this.#calculateProfitRate();
+    return profitRate;


바로 반환해도 될 것 같고.. 애초에 불필요한 추상화로 느껴지기도 하네요!

pocojang · 2023-02-20T09:33:22Z

src/utils/exception.js

+const { errorMessage } = require('../constants/constants');
+const validator = require('../domain/validation/validator');
+
+const exception = {


사실 에러처리라는게 정말 어려운 부분이기도 합니다.
에러에 대한 알림을 받는 대상자가 누구인가 또한 에러를 던지고 처리하는 레이어는 어디인가를 고민해보시면 좋아요.
하지만 음.. 이런 활용 방법이 효용성이 있다라는 공감대는 들지 않는 편이네요

일단은 언어에서 기본적으로 제공해주는 try~catch를 적극 활용하셨으면 좋겠습니다.

에러를 주로 받는 레이어는 어디인가

에러를 주로 던지는 레이어는 어디인가

에러를 받아야하는 대상자는 누구인가 (사용자, 동료 개발자 혹은 나 자신)

또한 TDZ 로 인한 레퍼런스 에러 수신에 대해서도 고민해보시면 좋아요

inyeong-kang added 30 commits February 14, 2023 16:28

docs: 기능 목록 작성

aa2cb08

chore: package.json 설치

87e601a

chore: eslint 세팅

2f05918

docs: 기능(예외 상황 등) 목록 구체화

b70385c

feat(constants): 구입 금액 단위 및 공백 처리 정규식 관련 상수 분리

985fade

chore: prettier 관련 세팅

c3a01fe

docs: To-do list 작성

07ef6a9

feat(constants): 로또 게임 관련 상수 분리

310cf75

feat(controller): 구입 금액에 따른 로또 개수 반환 기능 구현

ed5ad95

feat(validator): 구입 금액에 대한 유효성 검사 구현

0a204c2

docs: 기능 목록 수정

50c5ae5

feat(domain/model): 로또 번호 한줄(Lotto)을 당첨번호 및 보너스번호와 비교하여 등수 계산 기능 구현

bd636c9

feat(constants): 로또 게임 수익 관련 상수 분리

3ca1dc8

feat(constants): 등수에 따라 수익을 보여주는 배열 상수 분리

943f7ed

feat(domain/model): 등수에 따라 수익 계산하는 기능 구현

92a0198

refactor(domain/model): Lotto 모델의 profit 필드 삭제

2af4b53

feat(constants): 로또 번호 범위(1-45) 관련 상수 분리

853d6ad

feat(domain/model, utils): 등수로 게임 수익 계산

798048e

feat(utils): 무작위의 로또 번호 생성 기능 구현

9b01e6e

feat(constants): 로또 번호 개수에 대한 상수 분리

57db5bd

test(domain/model): 로또 번호가 일치하는 것이 없는 경우에 대한 테스트 케이스 추가

b295031

feat(domain/model): 구입된 Lotto 인스턴스들로 등수 계산하는 기능 구현

ab08662

chore(eslint): endOfLine 설정 변경

cc3904f

chore: 테스트 코드 내에서 eslint 10줄 메서드 제한 해제

a42c6b5

feat(domain/model): 로또들(Lottos)로 수익 및 수익률 계산 기능 구현

a1b8292

refactor(validator): eslint에 따라 불필요한 삼항 연산자 제거

02a2e32

chore: 불필요한 모듈 import 코드 제거

ed281f4

feat(domain/model): 당첨 번호 인스턴스 생성

4439fda

feat(validation): 당첨 번호 유효성 검사

d8abddd

feat(domain/model): 보너스 번호 인스턴스 생성

2f8bbe2

inyeong-kang added 7 commits February 16, 2023 16:22

feat(controller, domain): 당첨 번호 입력 기능 구현

42d6916

feat(controller, domain): 보너스 번호 입력 기능 구현

4cad90b

fix(constants): 게임 총 수익률 출력 메시지 상수 수정

4dece78

feat(controller, view): 게임 실행 결과(당첨 통계 및 총 수익률) 출력 기능 구현

7b07aad

fix(constants): 게임 실행 결과 관련 상수 메시지 수정

979d32e

fix(__test__): 테스트 함수명 등 오타 수정

7cacc14

feat: 게임 실행 기능 추가

61e66da

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

pocojang self-assigned this Feb 16, 2023

pocojang requested changes Feb 17, 2023

View reviewed changes

kyw0716 added 17 commits February 20, 2023 12:31

fix: JSON 파일에 있는 주석 삭제

bda4f74

style(Lotto.test): 테스트 코드 예상되는 결과 값 네이밍 변경 expected => caculated

2636e6b

fix: 쓰지 않는 console.log 삭제

53a430a

docs: 기능 목록 체크 박스 추가

c308849

style: inputView 콜백 함수 네이밍 변경

109b261

style: RandomNumberGenerator내 숫자 중복 체크 메서드명 변경

daf00cb

refactor: RandomNumberGenerator내 번호 정렬 함수 분리하지 않는 방식으로 변경

1fe7289

style: 파일명 변경 RandomNumberGenerator => randomNumberGenerator

701bd06

style: validator 하위 메서드들에 *-valid 키워드 제거

8ac62d7

refactor: Lottos의 caculateProfitRate 메서드 속 무의미한 값 할당 삭제

0157f5e

refactor: Lottos의 caculateProfit 속 무의미한 값 할당 삭제

01c60f5

style: double quote => single quote

3247eab

refactor: Lottos 수익 계산 메서드 연산 순서 명시

d24cb62

refactor: Lotto내 순위 지정 메서드 속 순위 값 상수처리

7485474

test: WinningLotto 클래스 테스트 코드 추가

a885dda

refactor: WinningNumbers 클래스, BonusNumber 클래스 => WinningLotto 클래스로 통합

22e668a

pocojang approved these changes Feb 20, 2023

View reviewed changes

pocojang merged commit af02134 into woowacourse:kyw0716 Feb 20, 2023

kyw0716 deleted the kyw0716 branch February 23, 2023 06:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 콘솔 기반 로또 게임] 센트(김영우) 미션 제출합니다. #174

[1단계 - 콘솔 기반 로또 게임] 센트(김영우) 미션 제출합니다. #174

kyw0716 commented Feb 16, 2023 •

edited

Loading

pocojang left a comment •

edited

Loading

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 17, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang Feb 17, 2023

kyw0716 Feb 20, 2023

pocojang left a comment

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023

pocojang Feb 20, 2023


		const profit = calculateProfit(lotto.getRank());

		expect(profit).toBe(expectedProfit);

		lotto.calculateRank(winningNumbers, bonusNumber);

		this.#calculateRank(lotto.getRank());

		const profitRate = this.#calculateProfitRate();
		return profitRate;

[1단계 - 콘솔 기반 로또 게임] 센트(김영우) 미션 제출합니다. #174

[1단계 - 콘솔 기반 로또 게임] 센트(김영우) 미션 제출합니다. #174

Conversation

kyw0716 commented Feb 16, 2023 • edited Loading

안녕하세요 포코!! 이번에 제로와 함께 콘솔 기반 로또 게임을 구현한 센트 입니다😀😀

1️⃣ Lotto 클래스

📝필드

🛠️ 메서드

2️⃣ Lottos 클래스

📝필드

🛠️ 메서드

3️⃣ WinningNumbers 클래스

📝필드

🛠️ 메서드

4️⃣ BonusNumber 클래스

📝필드

🛠️ 메서드

⁉️ 궁금한점

1️⃣ 랜덤한 요소에 대한 테스트를 어떻게 진행하는 것이 좋을지 궁금합니다!

2️⃣ 입력값을 저장하기 위해서만 사용하는 클래스를 사용했는데 이러한 방식이 맞는지 궁금합니다!

pocojang left a comment • edited Loading

Choose a reason for hiding this comment

소소한 리뷰

Q&A

Q1. 랜덤한 요소에 대한 테스트를 어떻게 진행하는 것이 좋을지 궁금합니다!

Q2. 입력값을 저장하기 위해서만 사용하는 클래스를 사용했는데 이러한 방식이 맞는지 궁금합니다!

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

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

kyw0716 commented Feb 16, 2023 •

edited

Loading

pocojang left a comment •

edited

Loading