[차세진] 로또 미션 Step1 #3

chasj0326 · 2024-07-07T15:21:25Z

미션을 수행하면서 어려웠던 점

1. 도메인 나누기

모든 결과를 추합해서 출력하기 위해서는, 로또를 작동시키는 클래스가 너무 많은 일을 해야 했어요. (금액 받아서 로또 만들기, 구매한 로또 등수매기기, 등수마다 개수세기, 수익률 계산하기 등등) 그래서 어려웠습니다 🥲
따라서 전체적인 동작은 LottoSystem 이라는 모델에서 하되, 로또 결제(LottoPayment)와 당첨로또와의 비교(LottoMatcher)를 도와주는 클래스로 나누었습니다. 말그대로 도와주는 친구들이라서, 순수함수인 static 메소드로 작성했습니다
아래와 같은 구조입니다
- 모델: LottoSystem
- 컨트롤: LottoSystemControl
- 뷰 : LottoSystemView
- 기타 도우미 : LottoPayment, LottoMatcher

(사실 ... 뭔가 대놓고 컨트롤러!뷰! 이름을 붙이기는 조금 싫었지만 ... 마땅히 생각나지 않았습니다 ㅎ.ㅎ)

2. TDD

프로그래밍 요구사항에 나와있는 대로 TDD 로 구현해봤습니다 .. ! 테스트랑 엄청 친하진 않아서 어려웠는데, 뭘 구현해야 하는 지가 명확해지고 (뇌절을 안할 수 있다!) 코드가 실행되는 시나리오에 의존하지 않게 되었습니다.
예를 들면, A 가 실행되어야 B 를 테스트할 수 있으면 A 를 모킹하게 됩니다. 하지만 테스트를 먼저 작성하니 이런 상황을 미리 안만들 수 있는? 것 같아요 ! (그치만 랜덤넘버는.. 모킹해버린 ..)

리뷰 받고 싶은 부분 & 궁금한 점

테스트 코드에서 메소드의 동작여부 보다는 validate 를 올바르게 하는지, 잘못된 값은 에러를 내는지에 대한 테스트가 더 많은 것 같아요. 메소드의 동작 여부와 함께 관리하나요 ? 아니면 유효성 검사 부분은 따로 두기도 하나요 !

클래스(또는 객체)를 사용하는 경우, 프로퍼티를 외부에서 직접 꺼내지 않는다. 객체에 메시지를 보내도록 한다.
getter를 금지하는 것이 아니라 말 그대로 프로퍼티 자체를 그대로 꺼내서 객체 바깥에서 직접 조작하는 등의 작업을 지양하자는 의미입니다. 객체 내부에서 알아서 할 수 있는 일은 객체가 스스로 할 수 있게 맡겨주세요.

요런 요구사항이 있었는데, 잘 지켜졌는지 모르겠습니다 .. ! LottoSystemController에서 LottoSystemViewer에게 LottoSystem 을 통째로 넘기고, 뷰어에서는 LottoSystem 의 프로퍼티를 꺼내서 출력합니다. 이런건 조작은 아니구 참조이기 때문에 무관한 거.. 겠죠 !?
이 외에도 validation, 전체적인 구조 등등ㅇ..등.. 자유롭게 피드백 주세요 ! 🙇🏻‍♀️

suyeon1218

안녕하세요 세진님...! 비록 팀은 아니지만 세진님이 이번엔 어떻게 설계하셨나 싶어 슬쩍 구경하러 왔어요...! TDD 를 기반으로 설계를 하신 걸까요? 아니면 설계를 먼저 한 뒤 TDD 로 로직을 작성하시려고 한 건가요?! 어느 쪽이든 LottoSystem (LottoMatcher, LottoPayment) 로 역할을 분리해준 게 깔끔하단 생각이 들었어요. 그리고 차츰 세진님이 RacingCar 에서 사용했던 코드들을 재사용하시는 걸 보고 자신만의 스타일을 잡아간다는 느낌도 들어요. 저는 아직 방황하는 중이지만... 세진님 코드를 보고 많이 배웁니다 😀

suyeon1218 · 2024-07-09T07:27:04Z

src/constants/lottoType.js

일반 텍스트로 타입을 구분한 것과 Symbol 을 사용해서 구분하는 것의 차이점이 무엇인가요...?!

오오 독립성을 보장해준다?
디테일이 돋보이네요..!

'lottoTicket' === 'lottoTicket' // true Symbol('lottoTicket') === Symbol('lottoTicket') // false

suyeon1218 · 2024-07-09T08:12:20Z

src/validations/lotto.js

+  lottoType: {
+    check: ({ type }) => type === LOTTO_TYPE.WINNING || type === LOTTO_TYPE.TICKET,
+    errorMessage: '로또는 ticket, winning 두 가지 유형이어야 합니다.',
+  },


세진님의 이런 유효성 검사 방식이 깔끔하다고 느끼면서 앞에 구문이 통과하지 못하면 || 연산자의 마지막 에러 메세지를 반환해주는 react-hook-form 이 생각났어요...! 세진님의 방법도 좋은데 이런 방법도 괜찮지 않을까 싶어 적어봅니다...!!

lottoType: ({type}) => type === LOTTO_TYPE.WINNING || type === LOTTO_TYPE.TICKET || 'errorMessage'

suyeon1218 · 2024-07-09T08:37:36Z

src/utils/formatCurrency.js

이런 게 있군요...충격... ㅋㅋㅋㅋㅋ

오오 ko-KR도 인자로 받으면 좋을 것 같네요..!
지금 함수는 formatCurrency와 달리 한국만 지원하는 느낌?

suyeon1218 · 2024-07-09T08:41:24Z

src/domain/createLottoNumbers.js

저도 이 부분을 고민하다가 번호를 랜덤으로 생성해주는 유틸함수를 만들고, LottoRule 이라는 클래스에서 한번 더 감싸서 호출할 수 있도록 했거든요...! 세진님은 요 함수를 LottoSystem 에도 넣지 않고, util 함수에도 넣지 않으신 이유가 있으실까요?!

seeyoujeong

코드 잘읽었어요!
객체 마스터의 길은 대단한듯합니다 ㅋㅋㅋ

테스트 코드에서 메소드의 동작여부 보다는 validate 를 올바르게 하는지...

validate를 통과하면 메서드가 잘 동작한다고 생각이 들어서 괜찮지 않을까요..? 테스트 코드도 정답이 없는듯합니다... ㅎㅎ

요런 요구사항이 있었는데, 잘 지켜졌는지 모르겠습니다 .. !

제가 봤을땐 잘지켜진거같아요!

seeyoujeong · 2024-07-12T01:51:30Z

src/__tests__/lotto.test.js

+        new Lotto({
+          type: LOTTO_TYPE.WINNING,
+          numbers: [1, 2, 3, 4, 5, 45],
+          bonusNumber: 46,
+        }),
+    ).toThrow(lottoValidations.lottoRange.errorMessage);
+
+    expect(
+      () =>
+        new Lotto({
+          type: LOTTO_TYPE.TICKET,
+          numbers: [0, 1, 2, 3, 4, 5],
+        }),


중복을 줄이는 용도로 팩토리 메서드를 만들어도 괜찮을듯합니다!

function createWinningLotto(numbers, bonusNumbers) { ... } function createLotto(numbers) { ... }

seeyoujeong · 2024-07-12T01:52:44Z

src/constants/lottoRankingRule.js

+const LOTTO_RANKING_DATA = [
+  {
+    rank: 1,
+    matchCount: 6,
+    profit: 2000000000,
+  },
+  {
+    rank: 2,
+    matchCount: 5,
+    bonusMatch: true,
+    profit: 30000000,
+  },
+  {
+    rank: 3,
+    matchCount: 5,
+    bonusMatch: false,
+    profit: 1500000,
+  },
+  {
+    rank: 4,
+    matchCount: 4,
+    profit: 50000,
+  },
+  {
+    rank: 5,
+    matchCount: 3,
+    profit: 5000,
+  },
+];


오호! 저도 이런 데이터를 만들어놓고 시작해야겠어요!

seeyoujeong · 2024-07-12T02:00:15Z

src/domain/LottoSystem.js

+  constructor({ rankingRule = LOTTO_RANKING_RULE } = {}) {
+    LottoSystem.#validate({ rankingRule });
+
+    this.#rankingRule = rankingRule.sort((a, b) => b.rank - a.rank);


sortRank 요런 함수로 분리해도 괜찮을지도..?! 아닐지도..

seeyoujeong · 2024-07-12T02:08:00Z

src/domain/LottoSystemView.js

+    this.printer = new ConsolePrinter({
+      paidCount: '%{1}개를 구매했습니다.',
+      lottoTicket: '%{1} | %{2} | %{3} | %{4} | %{5} | %{6}',
+      line: '---------------------------------------------',
+      ranking: '%{1}등 : %{2}개 일치 (%{3}원) - %{4}개',
+      rankingWithBonus: '%{1}등 : %{2}개 일치, 보너스볼 일치 (%{3}원) - %{4}개',
+      profitRatio: '총 수익률은 %{1}% 입니다.',
+      error: '⚠️ %{1}',
+    });


최고! 👍

seeyoujeong · 2024-07-12T02:18:33Z

src/service/ConsoleReader.js

+  read(message, validations = []) {
+    const processInput = async () => {
+      const input = await this.askQuestion(message);
+      const errorMessage = this.findInputError(input, validations);
+
+      if (errorMessage) {
+        return processInput();
+      }
+      return input;
+    };
+    return processInput();
+  }


이러면 read 메서드가 동기인지 비동기인지 헷갈릴거 같아요!

seeyoujeong · 2024-07-12T02:22:53Z

src/validations/lottoPayment.js

+  payAmountUnit1000: {
+    check: ({ payAmount }) => payAmount % 1000 === 0,
+    errorMessage: '로또 결제는 1000원 단위로만 가능합니다.',
+  },


생각해보니 1000원 단위로만 사야겠군요! 제 코드 수정해야겠군요 ㅎㅎ
아. 아니면 수익률 계산할때 로또 개수로 해도 되니깐 1000원 단위로만 안받게 해도 괜찮지 않을까요?

seeyoujeong · 2024-07-12T02:27:11Z

src/validations/lottoSystem.js

+    errorMessage: '로또 랭킹 규칙에 유효하지 않은 값이(rank) 포함되어 있습니다.',
+  },
+  validMatchCounts: {
+    check: ({ rankingRule }) =>
+      rankingRule.every(({ matchCount }) => Number.isInteger(matchCount) && 0 < matchCount && matchCount <= 6),
+    errorMessage: '로또 랭킹 규칙에 유효하지 않은 값이(matchCount) 포함되어 있습니다.',
+  },
+  validBonusMatches: {
+    check: ({ rankingRule }) => rankingRule.every(({ bonusMatch }) => bonusMatch === undefined || typeof bonusMatch === 'boolean'),
+    errorMessage: '로또 랭킹 규칙에 유효하지 않은 값이(bonusMatch) 포함되어 있습니다.',
+  },
+  validProfits: {
+    check: ({ rankingRule }) => rankingRule.every(({ profit }) => Number.isInteger(profit) && profit > 0),
+    errorMessage: '로또 랭킹 규칙에 유효하지 않은 값이(profit) 포함되어 있습니다.',


이 에러메시지 만들어주는 함수가 있어도 괜찮을지도..?

seeyoujeong · 2024-07-12T02:32:01Z

src/service/ConsolePrinter.js

+  printWithTemplate(templateKey, messages) {
+    if (this.template.hasOwnProperty(templateKey)) {
+      console.log(this.format(templateKey, messages));
+    } else {
+      console.log(...messages);
+    }
+  }
+
+  lineBreak() {
+    console.log('');
+  }
+}


console.log 를 print로 대체해도 될듯합니다!

seeyoujeong · 2024-07-12T02:35:46Z

src/domain/LottoPayment.js

+  static #validate(lottoPaymentProps) {
+    validateLottoPayment({
+      target: lottoPaymentProps,
+    });
+  }


바로 validateLottoPayment를 안쓰고 한번더 감싼 이유가 있을까요??

jgjgill

수고 많으셨습니다..! 🙇‍♂️
Matcher와 Payment와 같은 부분 도우미로 정의하신 부분이 기억에 남네요..!

테스트 코드

보통 에러에 대한 체크 테스트도 같이 두는 것 같습니다..!
그래서 테스트만 보고도 올바르게 사용한 케이스, 잘못 사용한 케이스를 확인할 수 있는게 장점으로 느껴집니다.
그래서 제 생각에는 사용자 입장에서 생각하면 좋을 것 같아요. 다른 사람이 봤을 때 해당 함수를 어떻게 하면 쉽게 이해할 수 있을까? 이 테스트 구문은 없는게 오히려 좋지 않을까? 등등...

저같은 경우 사용자가 잘못된 입력을 하는 것을 테스트하기 보다는 로직에 집중해서 테스트를 구성한 것 같습니다. (핑계입니다.. 😅)

이렇게 하면 잘못된 값에 대한 테스트도 조금은 덜어낼 수 있지 않을까 싶네요..!

jgjgill · 2024-07-15T10:51:02Z

src/__tests__/lottoSystem.test.js

+    lottoSystem.setWinningLotto([1, 2, 3, 4, 5, 6], 7);
+    lottoSystem.payLottoTicket(2000);
+
+    expect(lottoSystem.profitAmount).toBe(2000050000);


2_000_050_000 더 보기 편할 것 같아요..!

jgjgill · 2024-07-15T10:53:47Z

src/constants/lottoType.js

오오 독립성을 보장해준다?
디테일이 돋보이네요..!

'lottoTicket' === 'lottoTicket' // true Symbol('lottoTicket') === Symbol('lottoTicket') // false

jgjgill · 2024-07-15T10:55:25Z

src/domain/LottoMatcher.js

@@ -0,0 +1,27 @@
+import { validateLottoMatcher } from '../validations/lottoMatcher.js';
+
+export default class LottoMatcher {


오오 Matcher라는 역할을 구성했군요..!

jgjgill · 2024-07-15T11:03:10Z

src/domain/createLottoNumbers.js

+  while (lottoNumbers.size < 6) {
+    lottoNumbers.add(getRandomNumber(1, 45));
+  }


정말 최악의 경우로 1이 100억 번 나온다면?! 🤪 (딴지 걸기)

jgjgill · 2024-07-15T11:08:54Z

src/utils/cloneDeep.js

오오 이 함수는 믿고 쓸 수 있겠죠..!
여기에 추가해도 괜찮을까요? 🧐

jgjgill · 2024-07-15T11:11:08Z

src/utils/formatCurrency.js

오오 ko-KR도 인자로 받으면 좋을 것 같네요..!
지금 함수는 formatCurrency와 달리 한국만 지원하는 느낌?

jgjgill · 2024-07-15T11:13:05Z

vitest.config.js

+  test: {
+    globals: true,
+  },
+})


JunilHwang · 2024-07-16T10:14:14Z

AI가 해주는 리뷰입니다!

알겠습니다. 포맷팅을 개선하여 각 섹션을 더 명확하게 구분하고 제목을 강조하겠습니다.

코드 분석:

제공된 Pull Request는 로또 게임 구현을 위한 기본적인 구조와 기능을 추가하고 있습니다. 주요 변경사항은 다음과 같습니다:

프로젝트 설정: ESLint와 Prettier 설정 파일 추가
도메인 로직: Lotto, WinLotto, LottoController, LottoRule 클래스 구현
뷰 로직: Input, Output, View 클래스 구현
유틸리티 함수: createValidator, formatCurrency, generateRandomNumber, readline 등 추가
유효성 검사: InputValidator, OutputValidator, LottoValidator 구현
테스트: Lotto와 WinLotto 클래스에 대한 단위 테스트 추가

강점:

관심사 분리: 도메인 로직, 뷰 로직, 유틸리티 함수가 잘 분리되어 있습니다.
테스트 주도 개발(TDD) 접근: Lotto와 WinLotto 클래스에 대한 단위 테스트가 구현되어 있습니다.
유효성 검사: 각 도메인과 입출력에 대한 상세한 유효성 검사 로직이 구현되어 있습니다.
확장성: LottoRule 객체를 통해 게임 규칙을 중앙에서 관리하고 있어 변경에 유연합니다.

개선점:

테스트 커버리지 확대: LottoController, View 등 다른 클래스에 대한 테스트도 추가될 필요가 있습니다.
에러 처리: 현재는 대부분의 에러를 콘솔에 출력하고 있습니다. 더 세밀한 에러 처리 및 사용자 피드백 메커니즘이 필요할 수 있습니다.
설정 관리: LottoRule 객체를 외부 설정 파일로 분리하여 더 쉽게 수정할 수 있도록 할 수 있습니다.
성능 최적화: 대량의 로또 번호 생성 시 성능을 고려한 최적화가 필요할 수 있습니다.

구체적 제안:

LottoController 테스트 추가:

import { describe, test, expect, vi } from 'vitest';
import LottoController from '../domain/LottoController';

describe('LottoController', () => {
  test('getLottos should return correct number of lottos', async () => {
    const mockView = {
      inputMoney: vi.fn().mockResolvedValue(5000),
    };
    const controller = new LottoController({ view: () => mockView, lotto: Lotto, winLotto: WinLotto });
    const lottos = await controller.getLottos();
    expect(lottos).toHaveLength(5);
  });
});

에러 처리 개선:

class LottoError extends Error {
  constructor(message) {
    super(message);
    this.name = 'LottoError';
  }
}

// 에러 발생 시
throw new LottoError('유효하지 않은 로또 번호입니다.');

// 에러 처리
try {
  // 로직
} catch (error) {
  if (error instanceof LottoError) {
    console.error('로또 게임 오류:', error.message);
  } else {
    console.error('알 수 없는 오류:', error);
  }
}

LottoRule을 외부 설정 파일로 분리:

// config.js
export default {
  lottoPrice: 1000,
  limitPrice: 10000000,
  minNumber: 1,
  maxNumber: 45,
  defaultLength: 6,
  winLength: 7,
  // ... 기타 설정
};

// LottoRule.js
import config from './config.js';

const LottoRule = {
  ...config,
  // ... 기타 메서드
};

export default LottoRule;

성능 최적화를 위한 로또 번호 생성 개선:

generateLottoNumbers: () => {
  const numbers = new Set();
  while (numbers.size < LottoRule.defaultLength) {
    numbers.add(Math.floor(Math.random() * LottoRule.maxNumber) + LottoRule.minNumber);
  }
  return Array.from(numbers);
},

학습 포인트:

객체지향 설계: 도메인 객체와 책임 분리에 대해 학습할 수 있습니다.
테스트 주도 개발: 단위 테스트 작성과 TDD 접근법을 실습할 수 있습니다.
유효성 검사: 복잡한 규칙을 가진 도메인의 유효성 검사 방법을 배울 수 있습니다.
설정 관리: 애플리케이션 설정을 중앙에서 관리하는 방법을 학습할 수 있습니다.

추가될 수 있는 요구사항 예시와 대응:

새로운 등수 추가 (예: "6등: 2개 번호 일치 / 1,000원")

대응: LottoRule.js의 winningInfo 객체에 새로운 등수 추가

winningInfo: {
  // ... 기존 등수
  sixth: {
    prize: 1_000,
    checkBonus: false,
    accord: 2,
  },
},

당첨금 변경 (예: "1등 당첨금을 3,000,000,000원으로 인상")

대응: config.js 파일에서 해당 값만 수정

export default {
  // ... 기타 설정
  winningInfo: {
    first: {
      prize: 3_000_000_000,
      // ... 기타 설정
    },
    // ... 기타 등수
  },
};

로또 번호 범위 변경 (예: "로또 번호 범위를 1-45에서 1-50으로 확장")

대응: config.js 파일에서 maxNumber 값 수정
```
export default {
  // ... 기타 설정
  maxNumber: 50,
  // ... 기타 설정
};
```

새로운 로또 게임 유형 추가 (예: "미니 로또(1-30 범위에서 4개 번호 선택)")

대응: 새로운 게임 유형을 위한 클래스와 규칙 추가

// MiniLottoRule.js
import config from './config.js';

const MiniLottoRule = {
  ...config,
  maxNumber: 30,
  defaultLength: 4,
  // ... 기타 미니 로또 특화 규칙
};

export default MiniLottoRule;

// MiniLotto.js
import Lotto from './Lotto.js';
import MiniLottoRule from './MiniLottoRule.js';

export default class MiniLotto extends Lotto {
  constructor(...numbers) {
    super(...numbers);
    this.rule = MiniLottoRule;
  }

  // ... 미니 로또 특화 메서드
}

이러한 변경사항들은 기존 코드의 구조를 크게 해치지 않으면서 새로운 요구사항을 수용할 수 있도록 합니다. 설정의 중앙화와 객체지향적 설계 덕분에 새로운 기능 추가나 규칙 변경이 비교적 쉽게 이루어질 수 있습니다.

chasj0326 and others added 9 commits July 6, 2024 14:38

docs: Update README.md

a2b83da

feat: 로또 번호 저장 구현

3dc2712

feat: 로또 결제 구현

a3ee751

feat: 로또 결과 계산 구현 및 로또 타입 상수화

b466e19

feat: 로또 시스템 구현

e95ccea

feat: viewer 작성 및 Matcher, Payment static 메소드 전환

f693fe0

feat: controller 구현 및 main 작성

2198c35

feat: currency 유틸 함수 추가

6c1a681

style: 코드 스타일 수정

9d7e79e

chasj0326 added the team1 label Jul 7, 2024

chasj0326 self-assigned this Jul 7, 2024

chasj0326 added 4 commits July 8, 2024 08:31

refactor: lottoData 프로퍼티 수정

712c577

refactor: LottoSystem 멤버 변수 접근 수정

60bc985

fix: lottoViewer 오류 수정 및 보너스볼 로직 버그 수정

d685e27

feat: 로또 규칙 bonusMatch 보완 및 클래스명 수정

1235582

suyeon1218 reviewed Jul 9, 2024

View reviewed changes

seeyoujeong reviewed Jul 12, 2024

View reviewed changes

jgjgill reviewed Jul 15, 2024

View reviewed changes

		@@ -0,0 +1,27 @@
		import { validateLottoMatcher } from '../validations/lottoMatcher.js';

		export default class LottoMatcher {

[차세진] 로또 미션 Step1 #3

Are you sure you want to change the base?

[차세진] 로또 미션 Step1 #3

Uh oh!

Conversation

chasj0326 commented Jul 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

미션을 수행하면서 어려웠던 점

1. 도메인 나누기

2. TDD

리뷰 받고 싶은 부분 & 궁금한 점

Uh oh!

suyeon1218 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!

seeyoujeong 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!

Choose a reason for hiding this comment

Uh oh!

jgjgill 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!

JunilHwang commented Jul 16, 2024

AI가 해주는 리뷰입니다!

코드 분석:

강점:

개선점:

구체적 제안:

학습 포인트:

추가될 수 있는 요구사항 예시와 대응:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

chasj0326 commented Jul 7, 2024 •

edited

Loading