-
Notifications
You must be signed in to change notification settings - Fork 108
[SCG] 이찬형 로또 3-4단계 제출합니다. #183
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: leechan7120
Are you sure you want to change the base?
Changes from 7 commits
ce89c99
4dae662
f70351f
d173524
f5606e2
dbf845c
cb6c1fc
dd7db01
d300697
6720eb7
a2b1dc0
3f87503
85da545
c971249
d9570cf
9cbfaa9
f1782dc
bd9c1dd
aef918e
118535d
bcc4b08
28426d4
29be74a
db39c88
e4a22a5
e5417aa
1f8071b
fe80899
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 |
|---|---|---|
|
|
@@ -5,13 +5,14 @@ | |
|
|
||
| public class LottoDraw { | ||
| private final Lotto drawnLotto; | ||
| private final int bonusNumber; | ||
| private final LottoReceipt receipt; | ||
| private final MatchingCounts counts; | ||
|
|
||
| public LottoDraw(Lotto drawnLotto, LottoReceipt receipt) { | ||
| public LottoDraw(Lotto drawnLotto, int bonusNumber, LottoReceipt receipt) { | ||
| this.drawnLotto = drawnLotto; | ||
| this.bonusNumber = bonusNumber; | ||
| this.receipt = receipt; | ||
|
|
||
| this.counts = getNumberCount(); | ||
| } | ||
|
|
||
|
|
@@ -32,7 +33,10 @@ private LottoResult getLottoResult(Lotto lottoRow) { | |
|
|
||
| int matchingCount = numbers.size(); | ||
|
|
||
| return LottoResult.valueOf(matchingCount); | ||
| boolean matchBonus = lottoRow.numbers().stream() | ||
| .anyMatch(num -> num.number() == 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. bonusNumber 원시값 포장하게 된다면 이 부분도 개선할 수 있어요. (Hint.
Author
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.
lottoRow.numbers()는 List를 반환할 것이고, List에 특정 숫자가 포함되어 있는지를 확인하는 contains 메소드를 활용해서 가독성 측면에서 더욱 간결하게 작성할 수 있을 것 같습니다. 조금 더 찾아보니, anymatch와 contains는 둘 다 선형으로 탐색하다 탐색을 성공하면 바로 return을 하는 것은 똑같지만, contains는 함수를 호출하지 않으니 비용(시간) 면에서 약간 더 빠를 수 있고, equals 메소드를 오버라이딩해서 사용하면 값을 직접 꺼내오지 않고 다양한 연산을 활용할 수 있기에 더욱 개선된다고 볼 수 있을 것 같습니다. |
||
|
|
||
| return LottoResult.valueOf(matchingCount, matchBonus); | ||
| } | ||
|
|
||
| public int getCount(LottoResult result) { | ||
|
|
@@ -43,4 +47,4 @@ public float getRateOfReturn() { | |
| int sumOfReturn = counts.getSumOfReturn(); | ||
| return (float) sumOfReturn / receipt.totalPrice(); | ||
| } | ||
| } | ||
| } | ||
|
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. 여기도 마지막 줄 개행,, 자동 정렬 꼭 적용해보기! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,53 @@ | ||
| package lotto; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class LottoPurchase { | ||
| private static final int LOTTO_PRICE = 1000; | ||
|
|
||
| private final Lottos lottos; | ||
| private final int totalPrice; | ||
| private final int change; | ||
| private final int manualCount; | ||
|
|
||
| public LottoPurchase(int totalPrice, LottoMaker lottoMaker) { | ||
| public LottoPurchase(int totalPrice, List<Lotto> manualLottos, LottoMaker lottoMaker) { | ||
| this.totalPrice = totalPrice; | ||
| this.change = totalPrice % LOTTO_PRICE; | ||
| this.manualCount = manualLottos.size(); | ||
|
|
||
| int totalTicketCount = totalPrice / LOTTO_PRICE; | ||
| int autoCount = totalTicketCount - manualCount; | ||
|
|
||
| int numberOfLotto = this.totalPrice / LOTTO_PRICE; | ||
| change = totalPrice % LOTTO_PRICE; | ||
| if (autoCount < 0) { | ||
| throw new IllegalArgumentException("구입 금액보다 많은 수동 로또를 선택하셨습니다."); | ||
| } | ||
|
|
||
| this.lottos = Lottos.from(numberOfLotto, lottoMaker); | ||
| List<Lotto> combinedLottos = new ArrayList<>(manualLottos); | ||
| for (int i = 0; i < autoCount; i++) { | ||
| combinedLottos.add(lottoMaker.makeLotto()); | ||
| } | ||
|
|
||
| this.lottos = new Lottos(combinedLottos); | ||
| } | ||
|
|
||
| public LottoReceipt printReceipt() { | ||
| return new LottoReceipt(lottos, totalPrice); | ||
| } | ||
|
|
||
| public int getManualCount() { | ||
| return manualCount; | ||
| } | ||
|
|
||
| public int getAutoCount() { | ||
| return getNumberOfLotto() - manualCount; | ||
| } | ||
|
|
||
| public int getNumberOfLotto() { | ||
| return lottos.size(); | ||
| } | ||
|
|
||
| public int getChange() { | ||
| return change; | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ public enum LottoResult { | |
| THREE(3, 5_000), | ||
| FOUR(4, 50_000), | ||
| FIVE(5, 1_500_000), | ||
| BONUS(5, 30_000_000), | ||
| SIX(6, 2_000_000_000); | ||
|
|
||
| private final int matchingCount; | ||
|
|
@@ -15,20 +16,29 @@ public enum LottoResult { | |
| this.reward = reward; | ||
| } | ||
|
|
||
| public static LottoResult valueOf(int count) { | ||
| for (LottoResult result : values()) { | ||
| if (result.matchingCount == count) { | ||
| return result; | ||
| } | ||
| public static LottoResult valueOf(int matchingCount, boolean matchBonus) { | ||
| if (matchingCount == 6) { | ||
| return SIX; | ||
| } | ||
| // 3개 미만은 모두 NONE으로 처리 | ||
| if (count >= 0 && count < 3) { | ||
| if (matchingCount == 5 && matchBonus) { | ||
| return BONUS; | ||
| } | ||
| if (matchingCount == 5) { | ||
| return FIVE; | ||
| } | ||
| if (matchingCount == 4) { | ||
| return FOUR; | ||
| } | ||
| if (matchingCount == 3) { | ||
| return THREE; | ||
| } | ||
| if (matchingCount < 3 && matchingCount >= 0) { | ||
| return NONE; | ||
| } | ||
| throw new IllegalArgumentException("유효하지 않은 당첨 개수입니다: " + count); | ||
| throw new IllegalArgumentException("유효하지 않은 당첨 개수입니다: " + matchingCount); | ||
| } | ||
|
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. Enum 으로 옮겼지만 요구 사항의 "함수(또는 메서드)의 길이가 10라인을 넘어가지 않도록 구현한다." 준수해 볼까요? 이 부분 리뷰가 3번째 이긴한데, 어렵게 느껴지시면 DM 주셔도 좋아요! 🙂 대신!! 충분히 고민 후!! 왜냐면 이미 사용한 곳이 있어서 고민하시면 풀리실 거 같아서요!
Author
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문을 활용한 반복문이었던 것 같은데, code depth가 2 이상이면 안된다는 요구 사항도 있어서 이를 지키려다 자꾸 저런 식으로 짜네요..! 또 다른 방법이 있을까요? Enum 자체 값 mapping을 1, 2도 만들어서 사용하는 방법도 있을 것 같긴 한데, 어차피 결과가 똑같은 걸 중복해서 생성하니 괜히 더 복잡해지는 게 아닐지.. 싶습니다. 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. code depth가 '2' 넘지 말라는 요구 사항이 왜 있었을까요?! 🤔 그걸 해결하기 위한 방법이 무엇이 있을 지 생각해보면, 메서드 분리가 좋은 방향이 될 수 있습니다. 그 뿐만 아니라 다른 방식으로도 개선해볼 수 있죠! public static LottoResult of(int count, boolean matchBonus) {
if (count == 5 && matchBonus) {
return BONUS;
}
return Arrays.stream(values())
.filter(result -> result != BONUS && result != NONE)
.filter(result -> result.matchingCount == count)
.findFirst()
.orElse(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. Stream 활용을 제시해드린 이유는 다른 코드에서 이미 Stream 사용한 사례가 있어서 이 질문도 했었고, 예시를 전달드렸습니다!
Author
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에 대한 것을 좀 찾아보니 되게 많은 상황에서 사용될 수 있겠네요..! stream이 사실 mapping이나 그런 쪽에서만 활용될 수 있을 줄 알았는데 문서를 좀 더 찾아보니까 제가 생각한 것보다 훨씬 많은 방향에 활용될 수 있네요. 메서드 분리에 대한 건 생각을 안 해본 건 아닌데, 그럼 지금 존재하는 메소드의 역할이 좀 애매해지는 것 같아서 그 요구사항 때문에 괜히 객체를 하나 더 생성하는 게 아닌가 싶은 생각이 들었습니다. 제공해주신 예시가 전 가장 좋아보이네요. |
||
|
|
||
| public int getReward(){ | ||
| public int getReward() { | ||
| return reward; | ||
| } | ||
| } | ||
| } | ||
|
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.
Author
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. 메서드가 main의 실행 로직 순서대로 배치되어 있으면 읽을 때 조금 더 빠르게 확인할 수 있을 것 같네요. 변수나 메소드나 중요한 것을 위에 두고, 아래로 갈수록 중요도가 떨어지는 것을 배치하라고 했는데, 지금은 거의 다 필수 로직에 해당하는 것 같아서 프로그램 동작 흐름별로 배치해 봤습니다. 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.
Author
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. main의 메소드들을 입력, 출력, 처리를 담당하는 객체로 따로 분리해서 각각을 호출할 수 있게끔 하면 좋을 것 같습니다. 기존의 메소드는 일부 public으로 변경되겠지만, 가독성이나 책임 측면에서 훨씬 좋을 것 같긴 합니다. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package lotto; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Scanner; | ||
|
|
||
|
|
@@ -9,7 +10,7 @@ public class Main { | |
|
|
||
| public static void main(String[] args) { | ||
|
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라인을 넘어가지 않도록 구현한다. 요구 사항을 지켜볼까요?! 상당히 줄이 길면 비즈니스 흐름을 파악하기 어려워요!! 😭 |
||
| Scanner scanner = new Scanner(System.in); | ||
| int totalPrice; | ||
| int totalPrice, numberOfManual; | ||
|
|
||
| System.out.println("구입금액을 입력해 주세요. (ex. 1000) (숫자가 아닌 경우 0으로 간주)"); | ||
|
|
||
|
|
@@ -21,7 +22,22 @@ public static void main(String[] args) { | |
|
|
||
| System.out.println(); | ||
|
|
||
| LottoPurchase purchase = purchaseAndPrintLotto(totalPrice); | ||
| System.out.println("수동으로 구매할 로또 수를 입력해 주세요."); | ||
| numberOfManual = Integer.parseInt(scanner.nextLine()); | ||
|
|
||
| System.out.println(); | ||
|
|
||
| System.out.println("수동으로 구매할 번호를 입력해 주세요."); | ||
| List<Lotto> manualLottos = new ArrayList<>(); | ||
| for (int i = 0; i < numberOfManual; i++) { | ||
| manualLottos.add(LOTTO_PARSER.parse(scanner.nextLine())); | ||
| } | ||
|
|
||
| LottoPurchase purchase = new LottoPurchase(totalPrice, manualLottos, LOTTO_MAKER); | ||
|
|
||
| System.out.println("\n수동으로 " + numberOfManual + "장, 자동으로 " + | ||
| (purchase.getNumberOfLotto() - numberOfManual) + "개를 구매했습니다."); | ||
|
|
||
| LottoReceipt receipt = purchase.printReceipt(); | ||
| displayReceipt(receipt); | ||
| displayChange(purchase.getChange()); | ||
|
|
@@ -30,16 +46,14 @@ public static void main(String[] args) { | |
| System.out.println("지난주 당첨 번호를 입력해 주세요."); | ||
| Lotto drawnLotto = LOTTO_PARSER.parse(scanner.nextLine()); | ||
|
|
||
| LottoDraw draw = new LottoDraw(drawnLotto, receipt); | ||
| System.out.println("보너스 볼을 입력해 주세요."); | ||
| int bonusNumber = Integer.parseInt(scanner.nextLine()); | ||
|
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.
Author
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. 숫자를 넣을때까지 시도하게 만들어봤습니다..! |
||
|
|
||
| LottoDraw draw = new LottoDraw(drawnLotto, bonusNumber, receipt); | ||
| System.out.println(); | ||
| displayResult(draw); | ||
| } | ||
|
|
||
| private static LottoPurchase purchaseAndPrintLotto(int totalPrice) { | ||
| LottoPurchase purchase = new LottoPurchase(totalPrice, LOTTO_MAKER); | ||
| System.out.println(purchase.getNumberOfLotto() + "개를 구매했습니다."); | ||
| return purchase; | ||
| } | ||
|
|
||
| private static void displayReceipt(LottoReceipt receipt) { | ||
| for (Lotto lotto : receipt.lottos().getLottos()) { | ||
|
|
@@ -63,8 +77,11 @@ private static void displayResult(LottoDraw draw) { | |
| System.out.println("3개 일치 (" + LottoResult.THREE.getReward() + "원)- " + draw.getCount(LottoResult.THREE)); | ||
| System.out.println("4개 일치 (" + LottoResult.FOUR.getReward() + "원)- " + draw.getCount(LottoResult.FOUR)); | ||
| System.out.println("5개 일치 (" + LottoResult.FIVE.getReward() + "원)- " + draw.getCount(LottoResult.FIVE)); | ||
| System.out.println( | ||
| "5개 일치, 보너스 볼 일치 (" + LottoResult.BONUS.getReward() + "원)- " + draw.getCount(LottoResult.BONUS)); | ||
| System.out.println("6개 일치 (" + LottoResult.SIX.getReward() + "원)- " + draw.getCount(LottoResult.SIX)); | ||
|
|
||
| System.out.println("총 수익률은 " + draw.getRateOfReturn() + "입니다."); | ||
| } | ||
| } | ||
|
|
||
| } | ||
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.
int bonusNumber원시값 포장을 할 수 있는 객체가 충분히 있어 보이는데요? 🤔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.
그러네요..? 기껏 왜 LottoNumber를 만들어놓고 쓰지 않았을까요.. 수정했습니다.