-
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 16 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 |
|---|---|---|
| @@ -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; | ||
| } | ||
| } | ||
| } |
|
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 |
|---|---|---|
|
|
@@ -6,65 +6,111 @@ | |
| public class Main { | ||
| private static final LottoMaker LOTTO_MAKER = new LottoMaker(); | ||
| private static final LottoParser LOTTO_PARSER = new LottoParser(); | ||
| private static final Scanner SC = new Scanner(System.in); | ||
|
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. 변수를 봤을 때 익숙하지 않다면 명확하게 알 수 없는 게 단점이라 생각합니다. 일반적으로 저렇게 많이 쓰길래 그냥 썼는데, 사실 이유 없이 그냥 쓴다는 게 좋을 수는 없기 때문에 수정하는 게 바람직해 보이네요. 수정했습니다! |
||
|
|
||
| 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 price = inputPrice(); | ||
| if (price < 1000) { | ||
| return; | ||
| } | ||
|
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. inputPrice라는 함수에서 사용자에게 필요한 정보를 전달했고, 돈을 필요한 만큼 내지 않았으니 inputPrice에서 메시지를 return하고, 해당 코드는 main을 종료하기 위한 용도로 사용했는데, 제대로 된 금액을 받을 때까지 재시도하게 만드는 것이 훨씬 바람직해 보입니다. 수정했습니다! |
||
|
|
||
| System.out.println("구입금액을 입력해 주세요. (ex. 1000) (숫자가 아닌 경우 0으로 간주)"); | ||
| LottoPurchase purchase = buyLottos(price); | ||
| LottoReceipt receipt = purchase.printReceipt(); | ||
|
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.
|
||
| displayReceiptInfo(receipt, purchase.getChange()); | ||
|
|
||
| try { | ||
| totalPrice = Integer.parseInt(scanner.nextLine()); | ||
| } catch (NumberFormatException e) { | ||
| totalPrice = 0; | ||
| } | ||
| displayResult(runDraw(receipt)); | ||
| } | ||
|
|
||
| System.out.println(); | ||
| private static LottoPurchase buyLottos(int price) { | ||
| int manualCount = inputManualCount(); | ||
| List<Lotto> manuals = inputManualNumbers(manualCount); | ||
|
|
||
| LottoPurchase purchase = purchaseAndPrintLotto(totalPrice); | ||
| LottoReceipt receipt = purchase.printReceipt(); | ||
| displayReceipt(receipt); | ||
| displayChange(purchase.getChange()); | ||
| LottoPurchase purchase = new LottoPurchase(price, manuals, LOTTO_MAKER); | ||
| System.out.printf("\n수동 %d장, 자동 %d개를 구매했습니다.\n", | ||
| manualCount, purchase.getNumberOfLotto() - manualCount); | ||
| return purchase; | ||
| } | ||
|
|
||
| System.out.println(); | ||
| System.out.println("지난주 당첨 번호를 입력해 주세요."); | ||
| Lotto drawnLotto = LOTTO_PARSER.parse(scanner.nextLine()); | ||
| private static LottoDraw runDraw(LottoReceipt receipt) { | ||
| System.out.println("\n지난주 당첨 번호를 입력해 주세요."); | ||
| Lotto winningLotto = LOTTO_PARSER.parse(SC.nextLine()); | ||
| LottoNumber bonus = getValidBonus(winningLotto); | ||
| return new LottoDraw(winningLotto, bonus, receipt); | ||
| } | ||
|
|
||
| LottoDraw draw = new LottoDraw(drawnLotto, receipt); | ||
| System.out.println(); | ||
| displayResult(draw); | ||
| private static LottoNumber getValidBonus(Lotto winningLotto) { | ||
| while (true) { | ||
| LottoNumber bonus = bonusBall(); | ||
| if (!winningLotto.numbers().contains(bonus)) { | ||
| return bonus; | ||
| } | ||
| System.out.println("보너스 볼은 당첨 번호와 같을 수 없습니다."); | ||
| } | ||
| } | ||
|
|
||
| private static LottoPurchase purchaseAndPrintLotto(int totalPrice) { | ||
| LottoPurchase purchase = new LottoPurchase(totalPrice, LOTTO_MAKER); | ||
| System.out.println(purchase.getNumberOfLotto() + "개를 구매했습니다."); | ||
| return purchase; | ||
| private static LottoNumber bonusBall() { | ||
| System.out.println("보너스 볼을 입력해 주세요."); | ||
| try { | ||
| int bonusValue = Integer.parseInt(SC.nextLine()); | ||
| return new LottoNumber(bonusValue); | ||
| } catch (NumberFormatException e) { | ||
| System.out.println("숫자를 입력해 주세요."); | ||
| return bonusBall(); | ||
|
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.
재귀로 호출하면 어떤 장단점이 있을까요? 🙂 (Hint. Runtime 시점)
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. 장점으로는 우선 코드가 단순해지고, 복잡한 경우에 있어서 변수를 여러 개 할당할 필요 없이 지속적으로 변수를 유지해 사용할 수 있다는 것 정도가 있겠네요. 단점으로는 위와 같은 경우, 숫자가 아닌 것이 계속 들어오게 된다면 메모리에 함수가 계속 호출되게 되어 결국 프로그램이 터져버린다거나, 너무 많은 메모리가 pile되어 성능 저하도 발생할 수 있을 것 같습니다. 코드 컨벤션..때문에 재귀를 쓰긴 했는데, 메소드 분리를 통해 반복문으로 구현하는 방식으로 바꾸는 게 나을까요? 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 static int inputPrice() { | ||
| System.out.println("구입금액을 입력해 주세요. (ex. 1000)"); | ||
| try { | ||
| int price = Integer.parseInt(SC.nextLine()); | ||
| if (price < 1000) { | ||
| System.out.println("1000원 이상 입력해야 합니다. 로또를 살 수 없으므로 종료합니다."); | ||
| } | ||
| return price; | ||
| } catch (NumberFormatException e) { | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| private static int inputManualCount() { | ||
| System.out.println("\n수동으로 구매할 로또 수를 입력해 주세요."); | ||
| return Integer.parseInt(SC.nextLine()); | ||
| } | ||
|
|
||
| private static void displayReceipt(LottoReceipt receipt) { | ||
| for (Lotto lotto : receipt.lottos().getLottos()) { | ||
| List<String> nums = lotto.numbers().stream() | ||
| .map(LottoNumber::toString) | ||
| .toList(); | ||
| System.out.println("[" + String.join(", ", nums) + "]"); | ||
| private static List<Lotto> inputManualNumbers(int count) { | ||
| if (count <= 0) { | ||
| System.out.println("로또를 구매할 수 없습니다."); | ||
|
|
||
| } | ||
| System.out.println("\n수동으로 구매할 번호를 입력해 주세요."); | ||
| return java.util.stream.IntStream.range(0, count) | ||
| .mapToObj(i -> LOTTO_PARSER.parse(SC.nextLine())) | ||
| .toList(); | ||
| } | ||
|
|
||
| private static void displayChange(int change) { | ||
| if (change != 0) { | ||
| private static void displayReceiptInfo(LottoReceipt receipt, int change) { | ||
| receipt.lottos().getLottos().forEach(lotto -> { | ||
| List<String> s = lotto.numbers().stream().map(Object::toString).toList(); | ||
| System.out.println("[" + String.join(", ", s) + "]"); | ||
| }); | ||
| if (change > 0) { | ||
| System.out.println(change + "원이 남았습니다."); | ||
| } | ||
| } | ||
|
|
||
| private static void displayResult(LottoDraw draw) { | ||
| System.out.println("당첨 통계"); | ||
| System.out.println("---------"); | ||
|
|
||
| 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("6개 일치 (" + LottoResult.SIX.getReward() + "원)- " + draw.getCount(LottoResult.SIX)); | ||
|
|
||
| System.out.println("\n당첨 통계\n---------"); | ||
| for (LottoResult res : LottoResult.values()) { | ||
| if (res == LottoResult.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. 개인적으로 Enum 서로를 비교하는 행위가 외부에 드러나는 것보다 내부에서
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. Enum이 C/C++에선 단순 자료 열거로 사용되는 경우를 많이 봤어서 enum이 행위를 가진다는 게 익숙하진 않네요. 보다 보면 좀 익숙해지지 않을까 싶습니다..! 수정했습니다! |
||
| continue; | ||
| } | ||
| displayRank(res, draw.getCount(res)); | ||
| } | ||
| System.out.println("총 수익률은 " + draw.getRateOfReturn() + "입니다."); | ||
| } | ||
|
|
||
| private static void displayRank(LottoResult res, int count) { | ||
| String label = res == LottoResult.BONUS ? "5개 일치, 보너스 볼 일치" : res.getMatchingCount() + "개 일치"; | ||
|
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. '3항 연산자' 사용을 지양하는 요구 사항이 있었습니다! 🙂 그리고 왜 '3항 연산자' 사용을 지양할까요?! 찬형님 의견이 궁금해요.
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. 전 코드를 짤 때 자주 애용하는 편이라 사실 쓰지 말라고 하는 방향이 그리 익숙하진 않습니다..! 다만 사용하게 된다면 if-else 같은 코드 블록이 나뉘어져 있는 것들에 비해 코드 가독성이 떨어지는 것은 사실이고, 구분자가 확실히 보이지 않아 한 눈에 안 들어올 것 같습니다. 이외에도 다른 이유가 더 있나요? |
||
| System.out.printf("%s (%d원)- %d\n", label, res.getReward(), count); | ||
| } | ||
| } | ||
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.
private접근 제어자로 설정된 메서드도 순서가 중요할 수 있어요! 찬형님이 다른 사람의 코드를 보았을 때, 어떤식으로 순서가 정렬되어 있다면 보기 편할지? 상상해볼까요?!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의 실행 로직 순서대로 배치되어 있으면 읽을 때 조금 더 빠르게 확인할 수 있을 것 같네요. 변수나 메소드나 중요한 것을 위에 두고, 아래로 갈수록 중요도가 떨어지는 것을 배치하라고 했는데, 지금은 거의 다 필수 로직에 해당하는 것 같아서 프로그램 동작 흐름별로 배치해 봤습니다.