-
Couldn't load subscription status.
- Fork 92
[LBP] 현정빈 로또 미션 2-5 단계 제출합니다. #90
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
base: jeongbeanhyun
Are you sure you want to change the base?
Changes from 11 commits
5e2fb30
f9f5a87
1308961
d7bfc50
8188fc6
a63a201
643a77f
2be807c
4a44acc
fa5e7ff
b9c9bad
3f73124
0a91304
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 |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import controller.LottoController; | ||
|
|
||
| public class Application { | ||
|
|
||
| public static void main(String[] args) { | ||
| LottoController lottoController = new LottoController(); | ||
| lottoController.run(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| package controller; | ||
|
|
||
| import model.*; | ||
| import view.ErrorView; | ||
| import view.InputView; | ||
| import view.ResultView; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class LottoController { | ||
|
|
||
| public void run() { | ||
| try { | ||
| Money money = getPurchaseAmount(); | ||
| int manualCount = getManualCount(money); | ||
| LottoTickets lottoTickets = generateLottoTickets(money, manualCount); | ||
|
|
||
| ResultView.printOrderTickets(manualCount, money.getTicketCount() - manualCount); | ||
| ResultView.printPurchasedLottoTickets(lottoTickets); | ||
|
|
||
| processWinningResults(lottoTickets, money); | ||
|
|
||
| InputView.closeScanner(); | ||
|
|
||
| } catch (IllegalArgumentException e) { | ||
| ErrorView.printErrorMessage(e.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| private Money getPurchaseAmount() { | ||
| Money money = new Money(InputView.getPurchaseAmount()); | ||
| validatePurchaseAmount(money); | ||
| return money; | ||
| } | ||
|
|
||
| private int getManualCount(Money money) { | ||
| int manualCount = InputView.getManualTicketCount(); | ||
| validateManualCount(manualCount, money); | ||
| return manualCount; | ||
| } | ||
|
|
||
| private LottoTickets generateLottoTickets(Money money, int manualCount) { | ||
| List<List<Integer>> manualNumbers = InputView.getManualNumbers(manualCount); | ||
| int autoCount = money.getTicketCount() - manualCount; | ||
| return new LottoTickets(manualNumbers, autoCount); | ||
| } | ||
|
|
||
| public void validateManualCount(int manualCount, Money money) { | ||
| if (manualCount > money.getTicketCount()) { | ||
| throw new IllegalArgumentException("수동 구매 개수가 구매 가능한 개수를 초과할 수 없습니다."); | ||
| } | ||
| } | ||
|
|
||
| public void validatePurchaseAmount(Money money) { | ||
| int purchaseAmount = money.getAmount(); | ||
|
|
||
| if (purchaseAmount < 1000) { | ||
| throw new IllegalArgumentException("구매 금액은 1000원 이상이어야 합니다."); | ||
| } | ||
| if (purchaseAmount % 1000 != 0) { | ||
| throw new IllegalArgumentException("구매 금액은 1000원 단위여야 합니다."); | ||
| } | ||
| } | ||
|
|
||
| private void processWinningResults(LottoTickets lottoTickets, Money money) { | ||
| ResultView.printWinningStatistics(createLottoResult(lottoTickets, getWinningNumbers()), money.getAmount()); | ||
| } | ||
|
|
||
| private WinningNumbers getWinningNumbers() { | ||
| List<Integer> winningNumbers = InputView.getWinningNumbers(); | ||
| int bonusNumber = InputView.getBonusNumber(); | ||
| return new WinningNumbers(winningNumbers, bonusNumber); | ||
| } | ||
|
|
||
| private LottoResult createLottoResult(LottoTickets lottoTickets, WinningNumbers winningNumbers) { | ||
| return new LottoResult(lottoTickets.getTickets(), winningNumbers); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package model; | ||
|
|
||
| import java.util.*; | ||
|
|
||
| public class Lotto { | ||
|
|
||
| private final LottoNumbers lottoNumbers; | ||
|
|
||
| public Lotto() { | ||
| this.lottoNumbers = new LottoNumbers(); | ||
| } | ||
|
|
||
| public Lotto(List<Integer> numbers) { | ||
| this.lottoNumbers = new LottoNumbers(numbers); | ||
| } | ||
|
|
||
| public List<Integer> getSortedNumbers() { | ||
| return lottoNumbers.getSortedNumbers(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package model; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.IntStream; | ||
|
|
||
| public class LottoNumbers { | ||
| private static final int LOTTO_MIN_NUMBER = 1; | ||
| private static final int LOTTO_MAX_NUMBER = 45; | ||
| private static final int LOTTO_CREATE_SIZE = 6; | ||
| private static final List<Integer> LOTTO_NUMBER_POOL = | ||
| IntStream.rangeClosed(LOTTO_MIN_NUMBER, LOTTO_MAX_NUMBER) | ||
| .boxed() | ||
| .collect(Collectors.toList()); | ||
|
|
||
| private List<Integer> numbers; | ||
|
|
||
| public LottoNumbers() { | ||
| this.numbers = createLottoNumbers(); | ||
| } | ||
|
|
||
| public LottoNumbers(List<Integer> numbers) { | ||
| validate(numbers); | ||
| this.numbers = List.copyOf(numbers); | ||
| } | ||
|
||
|
|
||
| private void validate(List<Integer> numbers) { | ||
| if (numbers.size() != LOTTO_CREATE_SIZE) { | ||
| throw new IllegalArgumentException("로또 번호는 6개여야 합니다."); | ||
| } | ||
| if (numbers.stream().distinct().count() != LOTTO_CREATE_SIZE) { | ||
| throw new IllegalArgumentException("중복된 숫자가 있습니다."); | ||
| } | ||
| if (numbers.stream().anyMatch(n -> n < LOTTO_MIN_NUMBER || n > LOTTO_MAX_NUMBER)) { | ||
| throw new IllegalArgumentException("로또 번호는 1~45 사이여야 합니다."); | ||
| } | ||
| } | ||
|
|
||
| private List<Integer> createLottoNumbers() { | ||
|
||
| List<Integer> shuffledNumbers = new ArrayList<>(LOTTO_NUMBER_POOL); | ||
|
||
| Collections.shuffle(shuffledNumbers); | ||
|
|
||
| return shuffledNumbers.subList(0, LOTTO_CREATE_SIZE); | ||
| } | ||
|
|
||
| public List<Integer> getSortedNumbers() { | ||
| return numbers.stream() | ||
| .sorted() | ||
| .collect(Collectors.toList()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package model; | ||
|
|
||
| import java.util.EnumMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class LottoResult { | ||
| private final Map<Rank, Integer> matchCountMap = new EnumMap<>(Rank.class); | ||
|
|
||
| public LottoResult(List<Lotto> tickets, WinningNumbers winningNumbers) { | ||
| for (Lotto ticket : tickets) { | ||
| Rank rank = determineRank(ticket, winningNumbers); | ||
| matchCountMap.put(rank, matchCountMap.getOrDefault(rank, 0) + 1); | ||
| } | ||
| } | ||
|
|
||
| private Rank determineRank(Lotto ticket, WinningNumbers winningNumbers) { | ||
| List<Integer> ticketNumbers = ticket.getSortedNumbers(); | ||
| int matchCount = (int) ticketNumbers.stream() | ||
| .filter(winningNumbers.getNumbers()::contains) | ||
| .count(); | ||
| boolean hasBonus = ticketNumbers.contains(winningNumbers.getBonusNumber()); | ||
|
|
||
| if (matchCount == 6) return Rank.FIRST; | ||
| if (matchCount == 5 && hasBonus) return Rank.SECOND; | ||
| if (matchCount == 5) return Rank.THIRD; | ||
| if (matchCount == 4) return Rank.FOURTH; | ||
| if (matchCount == 3) return Rank.FIFTH; | ||
| return Rank.NONE; | ||
| } | ||
|
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. 요구사항 - "하나의 메서드는 10줄을 넘지 않는다"를 충족하고 있지 못한 것 같아요 수정해봅시다 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. 요구사항에 맞게 메서드 분리를 통해 10줄을 넘지 않도록 수정하였습니다. 피드백 감사합니다! private Rank determineRank(Lotto ticket, WinningNumbers winningNumbers) {
int matchCount = countMatchingNumbers(ticket, winningNumbers);
boolean hasBonus = hasBonusNumber(ticket, winningNumbers);
return getRank(matchCount, hasBonus);
}
// 일치하는 숫자 개수 계산
private int countMatchingNumbers(Lotto ticket, WinningNumbers winningNumbers) {
return (int) ticket.getNumbers().stream()
.filter(winningNumbers.getNumbers()::contains)
.count();
}
// 보너스 번호 포함 여부 확인
private boolean hasBonusNumber(Lotto ticket, WinningNumbers winningNumbers) {
return ticket.getNumbers().contains(winningNumbers.getBonusNumber());
}
private Rank getRank(int matchCount, boolean hasBonus) {
if (matchCount == 6) return Rank.FIRST;
if (matchCount == 5 && hasBonus) return Rank.SECOND;
if (matchCount == 5) return Rank.THIRD;
if (matchCount == 4) return Rank.FOURTH;
if (matchCount == 3) return Rank.FIFTH;
return Rank.NONE;
} |
||
|
|
||
| public Map<Rank, Integer> getMatchCountMap() { | ||
| return matchCountMap; | ||
| } | ||
|
|
||
| public double calculateProfitRate(int totalCost) { | ||
| int totalPrize = matchCountMap.entrySet().stream() | ||
| .mapToInt(entry -> entry.getKey().getPrizeMoney() * entry.getValue()) | ||
| .sum(); | ||
| return (double) totalPrize / totalCost; | ||
| } | ||
|
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. stream 안에서 계산 로직과 데이터 변환이 동시에 이루어지고 있어 가독성이 떨어져요 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. 기존 코드에서는 stream 내부에서 데이터 변환과 계산 로직이 동시에 이루어지고 있었습니다. 이렇게 되면 한 줄에 너무 많은 기능이 포함되어 가독성이 떨어지고, 유지보수 시에도 수정이 어렵다는 문제가 있었습니다.
이렇게 리팩토링함으로써, |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package model; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class LottoTickets { | ||
|
|
||
| private final List<Lotto> tickets; | ||
|
|
||
| public LottoTickets(List<List<Integer>> manualNumbers, int autoTicketCount) { | ||
| this.tickets = new ArrayList<>(); | ||
| addManualTickets(manualNumbers); | ||
| addAutoTickets(autoTicketCount); | ||
| } | ||
|
||
|
|
||
| private void addManualTickets(List<List<Integer>> manualNumbers) { | ||
| for (List<Integer> numbers : manualNumbers) { | ||
| tickets.add(new Lotto(numbers)); | ||
| } | ||
| } | ||
|
|
||
| private void addAutoTickets(int autoTicketCount) { | ||
| for (int i = 0; i < autoTicketCount; i++) { | ||
| tickets.add(new Lotto()); | ||
| } | ||
| } | ||
|
|
||
| public List<Lotto> getTickets() { | ||
| return new ArrayList<>(tickets); | ||
| } | ||
|
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. 이곳또한 불변성을 고려하지 못한 설계 같아요 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. List.copyOf()를 사용하여 불변리스트로 변환 후, getTickets()에서 추가적인 복사 없이 불변 리스트를 그대로 반환하도록 수정하였습니다. 피드백 덕분에 불변성에 대해 다시 한번 생각할 수 있었습니다. 감사합니다! |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package model; | ||
|
|
||
| public class Money { | ||
| private static final int LOTTO_PRICE = 1000; | ||
| private final int amount; | ||
|
|
||
| public Money(int amount) { | ||
| this.amount = amount; | ||
| } | ||
|
|
||
| public int getAmount() { | ||
| return amount; | ||
| } | ||
|
|
||
| public int getTicketCount() { | ||
| return amount / LOTTO_PRICE; | ||
| } | ||
| } | ||
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package model; | ||
|
|
||
| public enum Rank { | ||
| FIFTH(3, false, 5_000), | ||
| FOURTH(4, false, 50_000), | ||
| THIRD(5, false, 1_500_000), | ||
| SECOND(5, true, 30_000_000), // 5개 일치 + 보너스 볼 일치 | ||
| FIRST(6, false, 2_000_000_000), | ||
| NONE(0, false, 0); // NONE은 출력에서 제외됨 | ||
|
|
||
| private final int matchCount; | ||
| private final boolean bonusMatch; | ||
| private final int prizeMoney; | ||
|
|
||
| Rank(int matchCount, boolean bonusMatch, int prizeMoney) { | ||
| this.matchCount = matchCount; | ||
| this.bonusMatch = bonusMatch; | ||
| this.prizeMoney = prizeMoney; | ||
| } | ||
|
|
||
| public int getMatchCount() { | ||
| return matchCount; | ||
| } | ||
|
|
||
| public int getPrizeMoney() { | ||
| return prizeMoney; | ||
| } | ||
|
|
||
| public boolean isBonusMatch() { | ||
| return bonusMatch; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| package model; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class WinningNumbers { | ||
| private static final int SIZE = 6; | ||
| private static final int MIN_NUMBER = 1; | ||
| private static final int MAX_NUMBER = 45; | ||
| private final List<Integer> numbers; | ||
| private final int bonusNumber; | ||
|
|
||
| public WinningNumbers(List<Integer> numbers, int bonusNumber) { | ||
| validate(numbers, bonusNumber); | ||
| this.numbers = List.copyOf(numbers); | ||
| this.bonusNumber = bonusNumber; | ||
| } | ||
|
|
||
| private void validate(List<Integer> numbers, int bonusNumber) { | ||
| if (numbers.size() != SIZE) { | ||
| throw new IllegalArgumentException("당첨 번호는 6개의 숫자로 입력해야 합니다."); | ||
| } | ||
| if (numbers.stream().distinct().count() != SIZE) { | ||
| throw new IllegalArgumentException("중복된 숫자가 있습니다."); | ||
| } | ||
| if (numbers.stream().anyMatch(n -> n < MIN_NUMBER || n > MAX_NUMBER)) { | ||
| throw new IllegalArgumentException("당첨 번호는 1부터 45 사이여야 합니다."); | ||
| } | ||
| if (bonusNumber < MIN_NUMBER || bonusNumber > MAX_NUMBER) { | ||
| throw new IllegalArgumentException("보너스 볼은 1부터 45 사이여야 합니다."); | ||
| } | ||
| if (numbers.contains(bonusNumber)) { | ||
| throw new IllegalArgumentException("보너스 볼은 당첨 번호와 중복될 수 없습니다."); | ||
| } | ||
| } | ||
|
||
|
|
||
| public List<Integer> getNumbers() { | ||
| return numbers; | ||
| } | ||
|
|
||
| public int getBonusNumber() { | ||
| return bonusNumber; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package view; | ||
|
|
||
| public class ErrorView { | ||
| public static void printErrorMessage(String message) { | ||
| System.out.println(message); | ||
| } | ||
| } |
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.
네이밍과 해당 메서드의 동작이 다른 것 같아요
또한 디미터의 법칙에서는 "내부의 내부에 접근하지 말라"를 제안하고 있어요 해당 코드는 이를 위반하고 있는 것 같아요
클라이언트가
Lotto클래스의 내부 구조도 알게 되어 결합도도 높아보여요정빈이는 어떻게 생각해요?
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.
기존의
getSortedNumbers()는 정렬된 번호를 반환하는 것으로 해석될 수 있습니다. 그러나 실제로는LottoNumbers의 메서드를 그대로 호출하는 구조였습니다. 그래서 메서드명과 실제 동작이 일치하지 않았습니다.메서드명을
getNumbers()로 변경하여 로또번호를 가져온다는 의미를 명확하도록 하였습니다.디미터의 법칙을 이해하는 데 어려웠습니다.. 제가 이해한 바로는 ‘내가 소유한 객체의 내부 로직을 알아서는 안된다.’였습니다. 더 조사해보니 소유한 객체의 메서드는 호출해도 괜찮으나 내부 로직까지 신경써야 하는 경우면 디미터의 법칙을 위반한다는 것을 깨닫게 되었습니다! 그래서 현재 코드에서는 getSortedNumbers() 메서드가 로또번호를 보여줄 뿐만 아니라 정렬해주는 역할까지 담당하고 있었기 때문에 내부 로직이 노출된다는 것을 알게되었습니다. 그래서
LottoNumbers에서 정렬하는sortNumbers()메서드와 로또 번호를 조회하는getNumbers()메서드로 분류하였습니다. 그리고 로또번호 조회만 가능한getNumbers()를 사용하면 디미터의 법칙을 위반하지 않는 것 같다고 판단하였습니다…! 아래와 같이 수정하였는데, 제가 알맞게 이해하고 리팩토링한 것인지 궁금합니다..!