-
Notifications
You must be signed in to change notification settings - Fork 86
[로또] 김시영 미션 제출합니다. #108
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
[로또] 김시영 미션 제출합니다. #108
Changes from all commits
f709bec
4f6403d
08f79a8
d5dfd5c
6a11768
f8c7d6c
1364893
f2af3c4
608255c
e65cda3
e708800
3974147
aaec4b7
845570f
182d396
fa0cb60
ebd94ee
5f32192
aaf9a42
8bc96eb
23a5b8e
3427ca1
6423647
3314072
b673b11
380426d
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,16 @@ | ||
import controller.LottoController; | ||
import domain.generator.NumberGenerator; | ||
import domain.generator.RandomNumberGenerator; | ||
import view.InputView; | ||
import view.ResultView; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
InputView inputView = new InputView(); | ||
ResultView resultView = new ResultView(); | ||
NumberGenerator numberGenerator = new RandomNumberGenerator(); | ||
|
||
LottoController controller = new LottoController(inputView, resultView, numberGenerator); | ||
controller.run(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package controller; | ||
|
||
import static domain.constant.LottoConstants.LOTTO_PRICE; | ||
|
||
import domain.LottoResult; | ||
import domain.Lottos; | ||
import domain.WinningLotto; | ||
import domain.generator.NumberGenerator; | ||
import java.util.List; | ||
import view.InputView; | ||
import view.ResultView; | ||
|
||
public class LottoController { | ||
private static final String ERROR_INSUFFICIENT_AMOUNT = "금액이 부족합니다."; | ||
private final InputView inputView; | ||
private final ResultView resultView; | ||
private final NumberGenerator numberGenerator; | ||
|
||
public LottoController(InputView inputView, ResultView resultView, NumberGenerator numberGenerator) { | ||
this.inputView = inputView; | ||
this.resultView = resultView; | ||
this.numberGenerator = numberGenerator; | ||
} | ||
|
||
public void run() { | ||
Lottos lottos = setUpLottos(); | ||
WinningLotto winningLotto = setUpWinningLotto(); | ||
LottoResult lottoResult = new LottoResult(lottos, winningLotto); | ||
resultView.printResult(lottoResult); | ||
} | ||
|
||
private Lottos setUpLottos() { | ||
int amount = inputView.readAmount(); | ||
int count = amount / LOTTO_PRICE; | ||
int manualCount = inputView.readManualLottoCount(); | ||
if (manualCount > count) { | ||
throw new IllegalArgumentException(ERROR_INSUFFICIENT_AMOUNT); | ||
} | ||
int autoCount = count - manualCount; | ||
|
||
List<List<Integer>> manualNumbers = inputView.readManualNumbers(manualCount); | ||
Lottos lottos = Lottos.create(manualNumbers, autoCount, numberGenerator); | ||
|
||
resultView.printLottoCount(manualCount, autoCount); | ||
resultView.printLottos(lottos); | ||
return lottos; | ||
} | ||
|
||
private WinningLotto setUpWinningLotto() { | ||
return WinningLotto.create( | ||
inputView.readWinningNumbers(), | ||
inputView.readBonusNumber() | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package domain; | ||
|
||
import static domain.constant.LottoConstants.LOTTO_NUMBER_COUNT; | ||
|
||
import domain.generator.NumberGenerator; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class Lotto { | ||
public static final String ERROR_INVALID_SIZE = "로또 번호는 " + LOTTO_NUMBER_COUNT + "개여야 합니다."; | ||
public static final String ERROR_DUPLICATION = "로또 번호는 중복될 수 없습니다."; | ||
private final List<LottoNumber> numbers; | ||
|
||
public Lotto(NumberGenerator generator) { | ||
this(generator.generate()); | ||
} | ||
|
||
public Lotto(List<LottoNumber> numbers) { | ||
validate(numbers); | ||
this.numbers = new ArrayList<>(numbers); | ||
} | ||
|
||
private void validate(List<LottoNumber> numbers) { | ||
validateSize(numbers); | ||
validateDuplication(numbers); | ||
} | ||
|
||
private void validateSize(List<LottoNumber> numbers) { | ||
if (numbers.size() != LOTTO_NUMBER_COUNT) { | ||
throw new IllegalArgumentException(ERROR_INVALID_SIZE); | ||
} | ||
} | ||
|
||
private void validateDuplication(List<LottoNumber> numbers) { | ||
if (numbers.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 연산을 if문 내부에 넣게 되면 보기 힘들 수 있어요. 요런 것들은 라인을 분리하던거 별도로 메서드로 분리해보면 좋을 것 같습니다. indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다. 요 요구사항이 있는 이유는 각 메서드마다 책임을 최대한 작게 가져가기 위함이에요. 한번 이런걸 시도해보면 좋을 것 같습니다. |
||
.distinct() | ||
.count() != LOTTO_NUMBER_COUNT) { | ||
throw new IllegalArgumentException(ERROR_DUPLICATION); | ||
} | ||
} | ||
|
||
public boolean contains(LottoNumber number) { | ||
return numbers.contains(number); | ||
} | ||
|
||
public int countMatch(Lotto other) { | ||
return (int) numbers.stream() | ||
.filter(other::contains) | ||
.count(); | ||
} | ||
|
||
public List<LottoNumber> getNumbers() { | ||
return numbers; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package domain; | ||
|
||
import static domain.constant.LottoConstants.*; | ||
|
||
public class LottoNumber { | ||
public static final String ERROR_OUT_OF_RANGE = | ||
"로또 번호는 " + LOTTO_MIN_NUMBER + "부터 " + LOTTO_MAX_NUMBER + " 사이여야 합니다."; | ||
private final int number; | ||
|
||
public LottoNumber(int number) { | ||
validateRange(number); | ||
this.number = number; | ||
} | ||
|
||
private void validateRange(int number) { | ||
if (number < LOTTO_MIN_NUMBER || number > LOTTO_MAX_NUMBER) { | ||
throw new IllegalArgumentException(ERROR_OUT_OF_RANGE); | ||
} | ||
} | ||
|
||
public int getNumber() { | ||
return number; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) return true; | ||
if (o == null || getClass() != o.getClass()) return false; | ||
LottoNumber number1 = (LottoNumber) o; | ||
return number == number1.number; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Integer.hashCode(number); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package domain; | ||
|
||
import java.util.Arrays; | ||
import java.util.function.BiPredicate; | ||
|
||
public enum LottoRank { | ||
FIRST((matchCount, bonusMatch) -> matchCount == 6, 6, 2000000000), | ||
SECOND((matchCount, bonusMatch) -> matchCount == 5 && bonusMatch, 5, 30000000), | ||
THIRD((matchCount, bonusMatch) -> matchCount == 5 && !bonusMatch, 5, 1500000), | ||
FOURTH((matchCount, bonusMatch) -> matchCount == 4, 4, 50000), | ||
FIFTH((matchCount, bonusMatch) -> matchCount == 3, 3, 5000), | ||
MISS((matchCount, bonusMatch) -> matchCount < 3, 0, 0); | ||
|
||
private final BiPredicate<Integer, Boolean> condition; | ||
private final int matchCount; | ||
private final int prize; | ||
|
||
LottoRank(BiPredicate<Integer, Boolean> condition, int matchCount, int prize) { | ||
this.condition = condition; | ||
this.matchCount = matchCount; | ||
this.prize = prize; | ||
} | ||
|
||
public static LottoRank findRank(int matchCount, boolean bonusMatch) { | ||
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. 음 정적으로 Rank를 반환하는 메서드를 만든건 좋은 것 같아요. 그런데, 만약 Enum의 값이 바뀌면 이 메서드도 수정되어야 해요. 로직이 살짝 흩어져있다는 느낌이 들어요. 어떻게 하면 변경에도 실수할만한 요소를 줄일 수 있을까요? 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. 2등과 5등 조건 때문에 처음에는 정적으로 작성했었는데 찾아보니 BiPredicate를 활용해서 각 Enum이 스스로 정하도록 구현하는 방법이 더 유지보수에 좋을 것 같습니다! 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. 오 BiPredicate까지 하실거라곤 생각도 못했었요. |
||
return Arrays.stream(values()) | ||
.filter(rank -> rank.condition.test(matchCount, bonusMatch)) | ||
.findFirst() | ||
.orElse(MISS); | ||
} | ||
|
||
public int getMatchCount() { | ||
return matchCount; | ||
} | ||
|
||
public int getPrize() { | ||
return prize; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package domain; | ||
|
||
import static domain.constant.LottoConstants.LOTTO_PRICE; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
public class LottoResult { | ||
private final Map<LottoRank, Integer> result = new HashMap<>(); | ||
private final int totalCost; | ||
private final int totalPrize; | ||
|
||
public LottoResult(Lottos lottos, WinningLotto winningLotto) { | ||
for (Lotto lotto : lottos.getLottos()) { | ||
int matchCount = winningLotto.countMatch(lotto); | ||
boolean bonusMatch = winningLotto.matchBonus(lotto); | ||
LottoRank rank = LottoRank.findRank(matchCount, bonusMatch); | ||
result.merge(rank, 1, Integer::sum); | ||
} | ||
totalCost = lottos.getLottos().size() * LOTTO_PRICE; | ||
totalPrize = calculateTotalPrize(); | ||
} | ||
|
||
private int calculateTotalPrize() { | ||
return result.entrySet().stream() | ||
.mapToInt(entry -> entry.getKey().getPrize() * entry.getValue()) | ||
.sum(); | ||
} | ||
|
||
public double calculateProfitRate() { | ||
if (totalCost == 0) { | ||
return 0; | ||
} | ||
return (double) totalPrize / totalCost; | ||
} | ||
|
||
public int getMatchCountByRank(LottoRank rank) { | ||
return result.getOrDefault(rank, 0); | ||
} | ||
|
||
public Map<LottoRank, Integer> getResult() { | ||
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을 반환해서 연산하는군요. 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. 추가적으로 일급컬렉션을 왜 사용하는지?에 대해서 학습하고 리뷰 답변할 떄 적어주시면 좋을 것 같습니다. 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을 반환하기보다 관련 연산들을 메서드로 제공하도록 개선하겠습니다! |
||
return new HashMap<>(result); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package domain; | ||
|
||
import domain.generator.NumberGenerator; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class Lottos { | ||
private final List<Lotto> lottos; | ||
|
||
public Lottos(List<Lotto> lottos) { | ||
this.lottos = lottos; | ||
} | ||
|
||
public static Lottos create(List<List<Integer>> manualNumbers, int autoCount, NumberGenerator generator) { | ||
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<Lotto> totalLottos = new ArrayList<>(); | ||
for (List<Integer> numbers : manualNumbers) { | ||
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를 이용해서 Lotto 객체를 생성하는 로직 또한 Lotto에 있으면 어떨까요? 요구사항 중에 |
||
totalLottos.add(new Lotto(() -> numbers.stream().map(LottoNumber::new).toList())); | ||
} | ||
for (int i = 0; i < autoCount; i++) { | ||
totalLottos.add(new Lotto(generator)); | ||
} | ||
|
||
return new Lottos(totalLottos); | ||
} | ||
|
||
|
||
public List<Lotto> getLottos() { | ||
return new ArrayList<>(lottos); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package domain; | ||
|
||
import java.util.List; | ||
|
||
public class WinningLotto { | ||
public static final String ERROR_DUPLICATE_BONUS = "보너스 번호는 당첨 번호와 중복될 수 없습니다."; | ||
private final Lotto winningLotto; | ||
private final LottoNumber bonusNumber; | ||
|
||
public WinningLotto(Lotto winningLotto, LottoNumber bonusNumber) { | ||
validateBonusNumber(winningLotto, bonusNumber); | ||
this.winningLotto = winningLotto; | ||
this.bonusNumber = bonusNumber; | ||
} | ||
|
||
public static WinningLotto create(List<Integer> winningNumbers, int bonusNumber) { | ||
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<LottoNumber> numbers = winningNumbers.stream() | ||
.map(LottoNumber::new) | ||
.toList(); | ||
Lotto lotto = new Lotto(numbers); | ||
return new WinningLotto(lotto, new LottoNumber(bonusNumber)); | ||
} | ||
|
||
private void validateBonusNumber(Lotto winningLotto, LottoNumber bonusNumber) { | ||
if (winningLotto.contains(bonusNumber)) { | ||
throw new IllegalArgumentException(ERROR_DUPLICATE_BONUS); | ||
} | ||
} | ||
|
||
public int countMatch(Lotto other) { | ||
return winningLotto.countMatch(other); | ||
} | ||
|
||
public boolean matchBonus(Lotto other) { | ||
return other.contains(bonusNumber); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package domain.constant; | ||
|
||
public class LottoConstants { | ||
public static final int LOTTO_MIN_NUMBER = 1; | ||
public static final int LOTTO_MAX_NUMBER = 45; | ||
public static final int LOTTO_NUMBER_COUNT = 6; | ||
public static final int LOTTO_PRICE = 1000; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package domain.generator; | ||
|
||
import domain.LottoNumber; | ||
import java.util.List; | ||
|
||
public interface NumberGenerator { | ||
List<LottoNumber> generate(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package domain.generator; | ||
|
||
import static domain.constant.LottoConstants.LOTTO_MAX_NUMBER; | ||
import static domain.constant.LottoConstants.LOTTO_MIN_NUMBER; | ||
import static domain.constant.LottoConstants.LOTTO_NUMBER_COUNT; | ||
|
||
import domain.LottoNumber; | ||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.List; | ||
|
||
public class RandomNumberGenerator implements NumberGenerator { | ||
|
||
@Override | ||
public List<LottoNumber> generate() { | ||
List<Integer> candidates = new ArrayList<>(); | ||
for (int i = LOTTO_MIN_NUMBER; i <= LOTTO_MAX_NUMBER; i++) { | ||
candidates.add(i); | ||
} | ||
Collections.shuffle(candidates); | ||
|
||
return candidates.stream() | ||
.limit(LOTTO_NUMBER_COUNT) | ||
.map(LottoNumber::new) | ||
.toList(); | ||
} | ||
} |
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.
this 생성자를 이용해서, validate 로직활용한 것도 좋네요. 요런식으로 default 생성자하나를 두고,
다른 방법으로 init하는 생성자가 추가될 때, 기존 validate 룰을 지키기 좋습니다.
불변식이란 키워드도 한번 알아보면 좋을 것 같아요