-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2단계 - 로또(수동)] 오찌(오지훈) 미션 제출합니다. #438
Changes from 13 commits
c2a725a
18b9248
78a8ecf
6901251
fd1dd79
af74366
90fe99c
77537f7
fba05f3
2dd2b25
d40e678
8ec4b36
5b9054a
19dffa8
209840f
9ad9ef7
44f0a5e
3feafd9
9475824
dd3506a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,64 +1,127 @@ | ||
package controller; | ||
|
||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Collection; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
import model.Lotto; | ||
import model.LottoFactory; | ||
import model.LottoOrder; | ||
import model.LottoPurchasingMoney; | ||
import model.LottoRank; | ||
import model.WinningLotto; | ||
import model.WinningStatistics; | ||
import model.dto.LottoDto; | ||
import model.dto.WinningStatisticsDto; | ||
import view.InputView; | ||
import view.OutputView; | ||
|
||
public class LottoController { | ||
public void run() { | ||
LottoPurchasingMoney inputLottoPurchasingMoney = inputMoney(); | ||
LottoPurchasingMoney lottoPurchasingMoney = inputPurchasingMoney(); | ||
|
||
List<Lotto> lotteries = purchaseLottoTickets(inputLottoPurchasingMoney); | ||
OutputView.printPurchasedTickets(lotteries); | ||
LottoOrder lottoOrder = inputLottoOrder(lottoPurchasingMoney); | ||
|
||
WinningLotto winningLotto = inputWinningNumbers(); | ||
List<Lotto> lotteries = purchaseLotteries(lottoOrder); | ||
printPurchasedLotteries(lottoOrder, lotteries); | ||
|
||
WinningLotto winningLotto = inputWinningLotto(); | ||
|
||
WinningStatistics winningStatistics = calculateStatistics(lotteries, winningLotto); | ||
|
||
OutputView.printStatistics(winningStatistics, inputLottoPurchasingMoney); | ||
printWinningStatistics(winningStatistics, lottoPurchasingMoney); | ||
} | ||
|
||
private WinningStatistics calculateStatistics(List<Lotto> lotteries, WinningLotto winningLotto) { | ||
WinningStatistics winningStatistics = new WinningStatistics(); | ||
|
||
lotteries.forEach(lotto -> { | ||
LottoRank lottoRank = LottoRank.getRank( | ||
winningLotto.countMatching(lotto), | ||
winningLotto.containBonusBall(lotto) | ||
); | ||
winningStatistics.put(lottoRank); | ||
}); | ||
|
||
return winningStatistics; | ||
private void printPurchasedLotteries(LottoOrder lottoOrder, List<Lotto> lotteries) { | ||
List<LottoDto> lotteriesDto = lotteries.stream() | ||
.map(lotto -> new LottoDto(lotto.getLottoNumbers())) | ||
.collect(Collectors.toUnmodifiableList()); | ||
OutputView.printPurchasedLotteries(lottoOrder, lotteriesDto); | ||
} | ||
|
||
private LottoPurchasingMoney inputMoney() { | ||
private LottoPurchasingMoney inputPurchasingMoney() { | ||
try { | ||
return LottoPurchasingMoney.valueOf(InputView.inputMoney()); | ||
} catch (IOException | IllegalArgumentException e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return inputMoney(); | ||
return inputPurchasingMoney(); | ||
} | ||
} | ||
|
||
private LottoOrder inputLottoOrder(LottoPurchasingMoney lottoPurchasingMoney) { | ||
try { | ||
int manualLottoCount = InputView.inputManualLottoCount(); | ||
return new LottoOrder(lottoPurchasingMoney, manualLottoCount); | ||
} catch (IOException | IllegalArgumentException e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return inputLottoOrder(lottoPurchasingMoney); | ||
} | ||
} | ||
|
||
private List<Lotto> purchaseLotteries(LottoOrder lottoOrder) { | ||
return Stream.of(purchaseManualLotteries(lottoOrder), purchaseAutoLotteries(lottoOrder)) | ||
.flatMap(Collection::stream) | ||
.collect(Collectors.toUnmodifiableList()); | ||
} | ||
|
||
private List<List<Integer>> inputManualLottoNumbers(LottoOrder lottoOrder) { | ||
try { | ||
return InputView.inputManualLottoNumbers(lottoOrder.getManualLottoCount()); | ||
} catch (IOException e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return inputManualLottoNumbers(lottoOrder); | ||
} | ||
} | ||
|
||
private List<Lotto> purchaseManualLotteries(LottoOrder lottoOrder) { | ||
LottoFactory lottoFactory = LottoFactory.getInstance(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재는 LottoFactory를 싱글톤으로 만들어서 유지하고 있는데, 사실상 유틸 클래스와 다름이 없다는 생각도 듭니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 질문이네요. 그래서 설계하실 때는 최대한 정적 클래스를 지양하시고, 도메인 로직과 관련이 없지만 가공 등의 로직이 필요한 경우만 유틸 (정적) 클래스로 만드시는 게 좋습니다. (ex. String <-> LocalDate 변환을 돕는 LocalDateUtils) |
||
try { | ||
List<List<Integer>> manualLottoNumbers = inputManualLottoNumbers(lottoOrder); | ||
return manualLottoNumbers.stream() | ||
.map(lottoFactory::generateManual) | ||
.collect(Collectors.toUnmodifiableList()); | ||
} catch (IllegalArgumentException e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return purchaseManualLotteries(lottoOrder); | ||
} | ||
} | ||
|
||
private List<Lotto> purchaseAutoLotteries(LottoOrder lottoOrder) { | ||
LottoFactory lottoFactory = LottoFactory.getInstance(); | ||
List<Lotto> autoLotteries = new ArrayList<>(); | ||
for (int i = 0; i < lottoOrder.getAutoLottoCount(); i++) { | ||
autoLotteries.add(lottoFactory.generateAuto()); | ||
} | ||
return autoLotteries; | ||
} | ||
|
||
private WinningLotto inputWinningNumbers() { | ||
private WinningLotto inputWinningLotto() { | ||
try { | ||
return WinningLotto.of(InputView.inputWinningNumbers(), InputView.inputBonusBall()); | ||
} catch (IOException | IllegalArgumentException e) { | ||
OutputView.printErrorMessage(e.getMessage()); | ||
return inputWinningNumbers(); | ||
return inputWinningLotto(); | ||
} | ||
} | ||
|
||
private List<Lotto> purchaseLottoTickets(LottoPurchasingMoney inputLottoPurchasingMoney) { | ||
LottoFactory ticketFactory = LottoFactory.getInstance(); | ||
return ticketFactory.generateLotteries(inputLottoPurchasingMoney); | ||
private WinningStatistics calculateStatistics(List<Lotto> lotteries, WinningLotto winningLotto) { | ||
WinningStatistics winningStatistics = new WinningStatistics(); | ||
|
||
lotteries.forEach(lotto -> { | ||
LottoRank lottoRank = LottoRank.getRank( | ||
winningLotto.countMatching(lotto), | ||
winningLotto.containBonusBall(lotto) | ||
); | ||
winningStatistics.put(lottoRank); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 당첨 통계를 map에 저장하는 로직이 controller에 있어서 부자연스러울까요...? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네네 안그래도 이 부분에서 위에 남긴 코멘트 때문에 이야기하려고 했는데 ㅎㅎ |
||
|
||
return winningStatistics; | ||
} | ||
|
||
private void printWinningStatistics(WinningStatistics winningStatistics, LottoPurchasingMoney lottoPurchasingMoney) { | ||
OutputView.printStatistics(new WinningStatisticsDto(winningStatistics.getWinningCounts(), | ||
winningStatistics.getEarningsRate(lottoPurchasingMoney))); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
package model; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.function.Function; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.IntStream; | ||
|
||
|
@@ -10,12 +11,12 @@ public class LottoNumber { | |
private static final int MAX_LOTTO_NUMBER = 45; | ||
static final String INVALID_LOTTO_NUMBER_RANGE = "[ERROR] 로또 번호는 1부터 45 사이의 숫자여야 합니다."; | ||
|
||
private static final List<LottoNumber> LOTTO_NUMBER_POOL; | ||
static final Map<Integer, LottoNumber> LOTTO_NUMBER_POOL; | ||
|
||
static { | ||
LOTTO_NUMBER_POOL = IntStream.rangeClosed(MIN_LOTTO_NUMBER, MAX_LOTTO_NUMBER) | ||
.mapToObj(LottoNumber::new) | ||
.collect(Collectors.toUnmodifiableList()); | ||
.boxed() | ||
.collect(Collectors.toUnmodifiableMap(Function.identity(), LottoNumber::new)); | ||
} | ||
|
||
private final int number; | ||
|
@@ -27,7 +28,7 @@ private LottoNumber(int number) { | |
|
||
public static LottoNumber valueOf(int number) { | ||
validateRange(number); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
아 네네 그런 경우는 어쩔 수 없겠네요 :) |
||
return LOTTO_NUMBER_POOL.get(number - 1); | ||
return LOTTO_NUMBER_POOL.get(number); | ||
} | ||
|
||
private static void validateRange(int number) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package model; | ||
|
||
public class LottoOrder { | ||
static final String TOO_MANY_MANUAL = "[ERROR] 수동 로또 구매 수는 전체 구매 수를 넘을 수 없습니다."; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 수동 구매 개수와 자동 구매 개수를 저장할 방법을 골똘히 고민하다가, 우아한객체지향 세미나에서 주문, 배달과 같은 모든 상태를 객체로 만들어서 관리했던 것이 생각나서 저도 구매 정보를 LottoOrder로 만들어서 저장했습니다!
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
private final int manualLottoCount; | ||
private final int autoLottoCount; | ||
|
||
public LottoOrder(LottoPurchasingMoney lottoPurchasingMoney, int manualLottoCount) { | ||
validateManualCount(lottoPurchasingMoney, manualLottoCount); | ||
this.manualLottoCount = manualLottoCount; | ||
this.autoLottoCount = lottoPurchasingMoney.getPurchasableCount() - manualLottoCount; | ||
} | ||
|
||
public int getManualLottoCount() { | ||
return manualLottoCount; | ||
} | ||
|
||
public int getAutoLottoCount() { | ||
return autoLottoCount; | ||
} | ||
|
||
private void validateManualCount(LottoPurchasingMoney lottoPurchasingMoney, int manualLottoCount) { | ||
if (lottoPurchasingMoney.getPurchasableCount() < manualLottoCount) { | ||
throw new IllegalArgumentException(TOO_MANY_MANUAL); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,38 @@ | ||
package model; | ||
|
||
import java.util.Arrays; | ||
import java.util.EnumMap; | ||
import java.util.Map; | ||
|
||
public class WinningStatistics { | ||
private final EnumMap<LottoRank, Integer> winningStatistics; | ||
private final EnumMap<LottoRank, Integer> winningCounts; | ||
|
||
public WinningStatistics() { | ||
winningStatistics = new EnumMap<>(LottoRank.class); | ||
Arrays.stream(LottoRank.values()) | ||
.forEach(lottoRank -> winningStatistics.put(lottoRank, 0)); | ||
winningCounts = new EnumMap<>(LottoRank.class); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요런건 필드에서 바로 초기화하는게 더 깔끔할 것 같아요 :) |
||
for (LottoRank each : LottoRank.values()) { | ||
winningCounts.put(each, 0); | ||
} | ||
} | ||
|
||
public void put(LottoRank lottoRank) { | ||
winningStatistics.put(lottoRank, winningStatistics.get(lottoRank) + 1); | ||
winningCounts.put(lottoRank, winningCounts.get(lottoRank) + 1); | ||
} | ||
|
||
public int get(LottoRank lottoRank) { | ||
return winningStatistics.get(lottoRank); | ||
return winningCounts.get(lottoRank); | ||
} | ||
|
||
public double getEarningsRate(int money) { | ||
public Map<LottoRank, Integer> getWinningCounts() { | ||
return winningCounts; | ||
} | ||
|
||
public double getEarningsRate(LottoPurchasingMoney lottoPurchasingMoney) { | ||
long totalPrize = getTotalPrize(); | ||
return totalPrize / (double)money; | ||
return totalPrize / (double)lottoPurchasingMoney.getAmount(); | ||
} | ||
|
||
long getTotalPrize() { | ||
return winningStatistics.keySet().stream() | ||
.mapToLong(lottoRank -> lottoRank.getPrizeMoney() * winningStatistics.get(lottoRank)) | ||
return winningCounts.keySet().stream() | ||
.mapToLong(lottoRank -> lottoRank.getPrizeMoney() * winningCounts.get(lottoRank)) | ||
.sum(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package model.dto; | ||
|
||
import java.util.List; | ||
import model.LottoNumber; | ||
|
||
public class LottoDto { | ||
private final List<LottoNumber> lottoNumbers; | ||
|
||
public LottoDto(List<LottoNumber> lottoNumbers) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분에 DTO 관련 질문이 하나 더 있습니다! 제가 코드를 작성하고 보니까 controller에서 DTO에 필요한 값을 다 꺼내다가 생성자로 넣어주었는데요, 그냥 DTO 생성자에 원본 객체를 넣어주고 생성자 안에서 getter로 꺼내 쓰는 쪽이 조금 더 좋은 코드라고 보시나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네네 (파라미터가 많은 경우) 코드도 깔끔해지고 어떤 객체의 데이터를 꺼내서 만든 DTO야, 라는 것이 생성자에서 드러나면 해당 DTO를 사용하는 클라이언트 입장에서도 이해하기 쉬울 것 같아요 :) |
||
this.lottoNumbers = List.copyOf(lottoNumbers); | ||
} | ||
|
||
public List<LottoNumber> getLottoNumbers() { | ||
return lottoNumbers; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드를 작성하고 보니 controller가 너무 난잡한게 아닌가 하는 생각이 들어요!
service 레이어를 분리해볼까 라고 생각은 했지만 아직 controller와 service의 분리 기준도 잘 모르겠고요!
생각해보면 controller에서 여러 메서드를 public으로 열어 놓고 main에서 여러 메서드를 호출하면서 진행해야 하는 게 아닐까 하는 생각도 하고 있습니다. 닉은 어떻게 생각하시나요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 저는 main은 클린하게 유지되어야 한다고 생각해요. (애플리케이션을 실행시키는 역할만)
말씀하신대로
controller에서 여러 메서드를 public으로 열어 놓고 main에서 여러 메서드를 호출하면서 진행
하게 되면 오히려 Model과 View를 통해 애플리케이션의 흐름을 제어하는 Controller의 책임이 main으로 분산된다고 생각하거든요.난잡하다고 느끼신 부분은 많은 private 메서드 때문일지도 모르겠네요. 그럴때마다 객체가 너무 많은 일을 하고 있지는 않은지 고민해보시면 좋아요.
아래에 따로 코멘트 남길게요 :)