-
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 24 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
|
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,70 +1,119 @@ | ||
| package lotto; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.Scanner; | ||
|
|
||
| public class Main { | ||
| private static final LottoMaker LOTTO_MAKER = new LottoMaker(); | ||
| private static final LottoParser LOTTO_PARSER = new LottoParser(); | ||
| private static final Scanner SCANNER = new Scanner(System.in); | ||
|
|
||
| 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 = 0; | ||
| while (price < 1000) { | ||
| price = inputPrice(); | ||
| } | ||
|
|
||
| LottoPurchase purchase = buyLottos(price); | ||
| LottoReceipt receipt = purchase.getReceipt(); | ||
| displayReceiptInfo(receipt, purchase.getChange()); | ||
|
|
||
| System.out.println("구입금액을 입력해 주세요. (ex. 1000) (숫자가 아닌 경우 0으로 간주)"); | ||
| displayResult(runDraw(receipt)); | ||
| } | ||
|
|
||
| private static int inputPrice() { | ||
| System.out.println("구입금액을 입력해 주세요. (ex. 1000)"); | ||
| try { | ||
| totalPrice = Integer.parseInt(scanner.nextLine()); | ||
| int price = Integer.parseInt(SCANNER.nextLine()); | ||
| if (price < 1000) { | ||
| System.out.println("1000원 이상 입력해야 합니다."); | ||
| } | ||
| return price; | ||
| } catch (NumberFormatException e) { | ||
| totalPrice = 0; | ||
| return 0; | ||
| } | ||
| } | ||
|
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() {
while (true) {
int price = tryParsePrice();
if (price >= 1000) {
return price;
}
System.out.println("1000원 이상 입력해야 합니다.");
}
}
private static int tryParsePrice() {
System.out.println("구입금액을 입력해 주세요. (ex. 1000)");
try {
return Integer.parseInt(SCANNER.nextLine());
} catch (NumberFormatException e) {
System.out.println("숫자를 입력해 주세요.");
return 0;
}
}이런 비슷한 형태로 리팩토링 해보는 건 어때요? 제가 구성한 코드는 출력이 이상합니다 ^__^ 그대로.. 말고 수정해 보세요! 제가 구성한 코드도
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. depth 2를 넘기지 말라는 게 너무 과도한 반복문이나 조건문을 활용해서 코드 가독성이 떨어지고, 자바같이 네이밍이 긴 언어 같은 경우엔 한 줄에 코드가 다 안 담길 수 있다는 문제가 있기 때문이지 않을까 싶습니다. 사실 2까지는 괜찮을 거라 생각해요. 저도 과도한 메서드 분리와 코드 depth 사이에서 계속 고민해 보고 있는데, 이게 '코드'가 기준이 되면 안되고 로직이 기준이 되어야 할 것 같습니다. 특정 처리 안에서 너무 분기가 생기게 된다면 메서드 분리가 더욱 효과적이고, 그렇지 않다면 반복문이 메서드보다 훨씬 직관적으로 보일테니까요. 일단 그래도 컨벤션 요구사항에 있었으니 해당 방향에 맞게 수정해보겠습니다. |
||
|
|
||
| System.out.println(); | ||
|
|
||
| LottoPurchase purchase = purchaseAndPrintLotto(totalPrice); | ||
| LottoReceipt receipt = purchase.printReceipt(); | ||
| displayReceipt(receipt); | ||
| displayChange(purchase.getChange()); | ||
| private static int inputManualCount() { | ||
| System.out.println("\n수동으로 구매할 로또 수를 입력해 주세요."); | ||
| return Integer.parseInt(SCANNER.nextLine()); | ||
| } | ||
|
|
||
| System.out.println(); | ||
| System.out.println("지난주 당첨 번호를 입력해 주세요."); | ||
| Lotto drawnLotto = LOTTO_PARSER.parse(scanner.nextLine()); | ||
| private static List<Lotto> inputManualNumbers(int count) { | ||
| if (count <= 0) { | ||
| System.out.println("로또를 구매할 수 없습니다."); | ||
|
|
||
| LottoDraw draw = new LottoDraw(drawnLotto, receipt); | ||
| System.out.println(); | ||
| displayResult(draw); | ||
| } | ||
| System.out.println("\n수동으로 구매할 번호를 입력해 주세요."); | ||
| return java.util.stream.IntStream.range(0, count) | ||
| .mapToObj(i -> LOTTO_PARSER.parse(SCANNER.nextLine())) | ||
| .toList(); | ||
| } | ||
|
|
||
| private static LottoPurchase purchaseAndPrintLotto(int totalPrice) { | ||
| LottoPurchase purchase = new LottoPurchase(totalPrice, LOTTO_MAKER); | ||
| System.out.println(purchase.getNumberOfLotto() + "개를 구매했습니다."); | ||
| private static LottoPurchase buyLottos(int price) { | ||
| int manualCount = inputManualCount(); | ||
| List<Lotto> manuals = inputManualNumbers(manualCount); | ||
|
|
||
| LottoPurchase purchase = new LottoPurchase(price, manuals, LOTTO_MAKER); | ||
| System.out.printf("\n수동 %d장, 자동 %d개를 구매했습니다.\n", | ||
| manualCount, purchase.getNumberOfLotto() - manualCount); | ||
| return purchase; | ||
| } | ||
|
|
||
| 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 LottoDraw runDraw(LottoReceipt receipt) { | ||
| System.out.println("\n지난주 당첨 번호를 입력해 주세요."); | ||
| Lotto winningLotto = LOTTO_PARSER.parse(SCANNER.nextLine()); | ||
| LottoNumber bonus = getValidBonus(winningLotto); | ||
| return new LottoDraw(winningLotto, bonus, receipt); | ||
| } | ||
|
|
||
| private static LottoNumber bonusBall() { | ||
| System.out.println("보너스 볼을 입력해 주세요."); | ||
| try { | ||
| int bonusValue = Integer.parseInt(SCANNER.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 void displayChange(int change) { | ||
| if (change != 0) { | ||
| private static LottoNumber getValidBonus(Lotto winningLotto) { | ||
| while (true) { | ||
| LottoNumber bonus = bonusBall(); | ||
| if (!winningLotto.numbers().contains(bonus)) { | ||
| return bonus; | ||
| } | ||
| System.out.println("보너스 볼은 당첨 번호와 같을 수 없습니다."); | ||
| } | ||
| } | ||
|
|
||
| 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("\n당첨 통계\n---------"); | ||
|
|
||
| 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)); | ||
| Arrays.stream(LottoResult.values()) | ||
| .filter(LottoResult::isDisplayable) | ||
| .forEach(res -> displayRank(res, draw.getCount(res))); | ||
|
|
||
| System.out.println("총 수익률은 " + draw.getRateOfReturn() + "입니다."); | ||
| } | ||
|
|
||
| private static void displayRank(LottoResult res, int count) { | ||
| String label = res.getMatchingCount() + "개 일치"; | ||
| if (res == LottoResult.BONUS) { | ||
| label = "5개 일치, 보너스 볼 일치"; | ||
| } | ||
|
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등의 관한 로직이
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에선 떨어져 나가긴 했는데, 이걸 메시지로 주고 받는 형태라 해야할 진 모르겠네요. 메시지를 주는 주체가 res가 되어야 할까요? |
||
| 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의 실행 로직 순서대로 배치되어 있으면 읽을 때 조금 더 빠르게 확인할 수 있을 것 같네요. 변수나 메소드나 중요한 것을 위에 두고, 아래로 갈수록 중요도가 떨어지는 것을 배치하라고 했는데, 지금은 거의 다 필수 로직에 해당하는 것 같아서 프로그램 동작 흐름별로 배치해 봤습니다.