Skip to content

[2주차] 객체지향 코드 연습(BXXNXXII)#48

Open
BXXNXXII wants to merge 10 commits intoCOW-edu:mainfrom
BXXNXXII:main
Open

[2주차] 객체지향 코드 연습(BXXNXXII)#48
BXXNXXII wants to merge 10 commits intoCOW-edu:mainfrom
BXXNXXII:main

Conversation

@BXXNXXII
Copy link
Copy Markdown

작업 개요

로또 미션 기능 요구사항과 프로그래밍 요구사항을 충족하도록 구현

구현 내용

  • 로또 구매/발행/당첨 판정/통계/수익률 전체 흐름 구현
  • 입력/출력/파싱/도메인 로직 분리
    • view: InputView, OutputView
    • parser: InputParser
    • domain: Rank, WinningLotto, LottoIssuer, LottoResultCalculator, LottoStatistics
  • Lotto 검증 로직 강화
    • 개수/중복/범위 검증
    • List.copyOf로 불변성 강화
  • 예외 발생 시 [ERROR] 형식으로 메시지 출력

테스트

  • ./gradlew.bat test 통과
  • 도메인/파서 단위 테스트 추가
    • LottoTest
    • InputParserTest
    • RankTest
    • WinningLottoTest
    • LottoStatisticsTest
  • 통합 테스트 ApplicationTest 통과

문서

  • 기능 목록 및 학습한 내용/고민한 점 정리: docs/BXXNXXII.md

Copy link
Copy Markdown

@hughryu1125 hughryu1125 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

exception message들을 모아 둔 것, 매직넘버 처리 하신 것, 생성자에서 내부 로직과 관련된 점을 검증하는 것 등 신경을 쓰신게 보였습니다.

더불어 MVC 패턴에서 강조하는 원칙과 Controller에 대한 설계를 고민해보시면 좋을 것 같아요!

Comment on lines +22 to 51
private static void run() {
int purchaseAmount = readPurchaseAmount();
List<Lotto> purchasedLottos = issueLottos(purchaseAmount);
WinningLotto winningLotto = readWinningLotto();
LottoStatistics statistics = calculateStatistics(purchasedLottos, winningLotto);
OutputView.printStatistics(statistics, statistics.calculateProfitRate(purchaseAmount));
}

private static int readPurchaseAmount() {
return InputParser.parsePurchaseAmount(InputView.readPurchaseAmount());
}

private static List<Lotto> issueLottos(int purchaseAmount) {
LottoIssuer lottoIssuer = new LottoIssuer();
List<Lotto> purchasedLottos = lottoIssuer.issue(purchaseAmount);
OutputView.printPurchasedLottos(purchasedLottos);
return purchasedLottos;
}

private static WinningLotto readWinningLotto() {
List<Integer> winningNumbers = InputParser.parseWinningNumbers(InputView.readWinningNumbers());
Lotto winningLotto = new Lotto(winningNumbers);
int bonusNumber = InputParser.parseBonusNumber(InputView.readBonusNumber());
return new WinningLotto(winningLotto, bonusNumber);
}

private static LottoStatistics calculateStatistics(List<Lotto> purchasedLottos, WinningLotto winningLotto) {
LottoResultCalculator calculator = new LottoResultCalculator();
return calculator.calculate(purchasedLottos, winningLotto);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

view와 domain들을 이어주고, 전체적인 흐름을 관리하는 기능은 MVC 패턴에서 controller가 담당하는 것이 적절합니다!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

넵, 작업 중에 추가할까 고민을 했었는데 수정하도록 하겠습니다 :)

Comment on lines +25 to +30
public static void printStatistics(LottoStatistics statistics, double profitRate) {
System.out.println("당첨 통계");
System.out.println("---");
for (Rank rank : Rank.winningRanks()) {
System.out.println(formatRankLine(rank, statistics.getCount(rank)));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

view가 다른 domain들에 대한 의존도가 너무 높은 것 같습니다.
MVC 패턴에서 view는 model들을, model들은 view를 알아서는 안된다는 원칙을 참고하시면 좋을 것 같아요!

Comment on lines +6 to +7
private final Lotto winningLotto;
private final int bonusNumber;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lotto를 composition으로 가지는 구조는 어떤 설계 의도이셨을까요??

재활용을 위한 것이면, 다른 방법들도 있었을텐데 딱 적절하다고 생각되는 composition을 사용하셔서 의도하신건지 궁금해 여쭤봅니다!

Comment on lines +46 to +59
private static void validatePurchaseAmount(int purchaseAmount) {
if (purchaseAmount < LOTTO_PRICE) {
throw new IllegalArgumentException("구입 금액은 1,000원 이상이어야 합니다.");
}
if (purchaseAmount % LOTTO_PRICE != 0) {
throw new IllegalArgumentException("구입 금액은 1,000원 단위여야 합니다.");
}
}

private static void validateLottoNumberRange(int number) {
if (number < LOTTO_MIN_NUMBER || number > LOTTO_MAX_NUMBER) {
throw new IllegalArgumentException("로또 번호는 1부터 45 사이의 숫자여야 합니다.");
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이런 비즈니스 로직과 관련된 검증은 parser보다는, 모델 쪽에서 책임지는 방향은 어떨까요?

parser가 금액과 관련된 정책을 알고 있는 것보단, 해당하는 도메인이 갖고 있는 것이 더 좋을 것 같아요

Comment on lines +24 to +34
public long calculateTotalPrize() {
long totalPrize = 0L;
for (Rank rank : Rank.winningRanks()) {
totalPrize += (long) rank.getPrize() * getCount(rank);
}
return totalPrize;
}

public double calculateProfitRate(int purchaseAmount) {
return (double) calculateTotalPrize() / purchaseAmount * 100;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 두 기능이 LottoStatistics에 있는데 어떤 것을 의도하셨을까요?

LottoResultCalculator가 갖고 있는 구조도 생각해보시면 좋을 것 같아요!

Comment on lines +1 to +16
package lotto.domain;

import lotto.Lotto;

import java.util.List;

public class LottoResultCalculator {
public LottoStatistics calculate(List<Lotto> purchasedLottos, WinningLotto winningLotto) {
LottoStatistics statistics = new LottoStatistics();
for (Lotto lotto : purchasedLottos) {
Rank rank = winningLotto.match(lotto);
statistics.add(rank);
}
return statistics;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LottoStatistics와 더불어 네이밍이나 역할 분리가 살짝 모호한 부분이 있는 것 같습니다.
LottoStatistics에 남긴 리뷰를 함께 보시며 설계를 고민해보시면 좋을 것 같아요!

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.

2 participants