[로또] 최동근 미션 제출합니다. by reddevilmidzy · Pull Request #445 · woowacourse-precourse/java-lotto-6

reddevilmidzy · 2023-11-07T14:17:53Z

주어진 요구 사항
객체간의 메시지를 통한 협력
테스트를 통한 검증

위 사항들을 지키며 미션을 진행하였습니다.

SJ70

안녕하세요 최동근 님!
코드 잘 읽었습니다!
객체 지향적으로 신경 쓰신 게 많이 보이네요
튼튼한 구조로 잘 작성하신 것 같습니다!

SJ70 · 2023-11-08T15:34:51Z

src/main/java/lotto/model/Lotto.java

+    public Integer count(Lotto lotto) {
+        return (int) IntStream.range(0, Rule.NUMBER_SIZE)
+                .filter(i -> lotto.contains(numbers.get(i)))
+                .count();
+    }


무엇을 세는지 명시되면 좋을 것 같습니다!

저도 동의합니다! 메서드명을 조금 더 자세히 적으면 의도가 드러나 좋을 것 같아요!

로또 생성자에서 꽤나 많은 책임이 있는 것 같아요! 외부에서 validate를 해서 주입하는 건 어떨까요?

SJ70 · 2023-11-08T15:36:54Z

src/main/java/lotto/model/PurchaseAmount.java

+    public static PurchaseAmount of(Integer value) {
+        return new PurchaseAmount(value);
+    }


정적 팩토리 메소드를 사용하신 이유가 궁금합니다!

SJ70 · 2023-11-08T15:40:18Z

src/main/java/lotto/model/WinningLotto.java

+    private void validate(Lotto lotto, Bonus bonus) {
+        if (bonus.hasBonusNumber(lotto)) {
+            throw new IllegalArgumentException(ErrorMessage.BONUS_DUPLICATE.getMessage());
+        }
+    }


bonus.hasBonusNumber(lotto) 라는 문장은
bonus가 lotto라는 BonusNumber를 가지는가? 라고 읽힙니다!

SJ70 · 2023-11-08T15:51:23Z

src/main/java/lotto/controller/InputController.java

+    public WinningLotto getWinningLotto(InputView inputView, OutputView outputView) {
+        Lotto lotto = getWinningNumbers(inputView, outputView);
+        while (true) {
+            try {
+                Bonus bonus = getBonusNumber(inputView, outputView);
+                return new WinningLotto(lotto, bonus);
+            } catch (IllegalArgumentException exception) {
+                outputView.printErrorMessage(exception);
+            }
+        }
+    }
+
+    private Lotto getWinningNumbers(InputView inputView, OutputView outputView) {
+        while (true) {
+            try {
+                return readWinningNumbers(inputView);
+            } catch (IllegalArgumentException exception) {
+                outputView.printErrorMessage(exception);
+            }
+        }
+    }


당첨 번호와 보너스 번호의 재입력 분리 좋습니다!

요구사항에 맞춰서 보너스 번호/당첨 번호 입력받는 로직을 분리하신 점이 좋은 것 같아요!
(저도 이렇게 했어야 했는데 ㅠ.ㅠ)
배워갑니다! ☺️

SJ70 · 2023-11-08T15:55:48Z

src/main/java/lotto/controller/InputController.java

InputController가 거의 모든 메소드마다 inputView와 outputView를 매개변수로 가지네요.
조금 번거로워 보이는데 LottoController로부터 InputController를 분리하신 이유가 있을까요?
inputView와 outputView를 주입을 받아서 사용하지 않으신 이유 또한 궁금합니다!

로또 컨트롤러에서 인풋컨트롤러를 분리한 이유는 예외 시 다시 입력받는 부분을 따로 처리해주고 싶었기 때문입니다!
그리고 뷰들을 매개변수로 가지고 있는 이유는, 사실 처음에 인풋컨트롤러를 만들었을때 인풋뷰, 아웃풋뷰 모두 인풋컨트롤러의 인스턴스 변수였는데, 그렇게 되다보니 로또 컨트롤러와 차이점을 잘 못느껴 구분하고자 매개변수로 받는 방법을 택했습니다!

hyeonji1220 · 2023-11-08T16:32:01Z

안녕하세요 동근님! 저번주에 이어서 이번주에도 방문하였습니다...ㅋㅋ
지난주 남겨주신 피드백 너무 잘보았습니다.. 그리하여 이번주도 부탁드리러 들렀답니다!(당당)
저도 꼼꼼히 살펴보고 리뷰 남기도록 하겠습니다🧐🧐

#879

hey-sung · 2023-11-08T18:08:30Z

src/main/java/lotto/controller/InputController.java

+    }
+
+    public PurchaseAmount getPurchaseAmount(InputView inputView, OutputView outputView) {
+        while (true) {


예외가 발생했을 때 메시지를 출력하고 입력을 다시 받는 로직을 재귀로 해결했는데 성능(메모리)관련해서 걱정이 있었는데,
반복문으로 깔끔하게 해결하셨네요..! 이런 방법이 있었네요! 배워갑니다! 👍

hey-sung · 2023-11-08T18:10:40Z

src/main/java/lotto/controller/InputController.java

+    public WinningLotto getWinningLotto(InputView inputView, OutputView outputView) {
+        Lotto lotto = getWinningNumbers(inputView, outputView);
+        while (true) {
+            try {
+                Bonus bonus = getBonusNumber(inputView, outputView);
+                return new WinningLotto(lotto, bonus);
+            } catch (IllegalArgumentException exception) {
+                outputView.printErrorMessage(exception);
+            }
+        }
+    }
+
+    private Lotto getWinningNumbers(InputView inputView, OutputView outputView) {
+        while (true) {
+            try {
+                return readWinningNumbers(inputView);
+            } catch (IllegalArgumentException exception) {
+                outputView.printErrorMessage(exception);
+            }
+        }
+    }


요구사항에 맞춰서 보너스 번호/당첨 번호 입력받는 로직을 분리하신 점이 좋은 것 같아요!
(저도 이렇게 했어야 했는데 ㅠ.ㅠ)
배워갑니다! ☺️

hey-sung · 2023-11-08T18:13:51Z

src/main/java/lotto/controller/InputController.java

InputController로 따로 분리하니 로직도 깔끔하고, 가독성도 좋네요..!!! 배워갑니다! 👍

hey-sung · 2023-11-08T18:26:50Z

src/main/java/lotto/converter/StringToInteger.java

미션에서 문자열에서 정수로 변환하는 일이 잦았는데,
StringToInteger 클래스를 통해 변환도 하고, 검증도 하네요..! 좋은 것 같습니다! 👍

엇 이야기를 들어보니까 변환도 하고 검증도 하는 두가지 일을 하고 있는거 같군요(?)! ㅋㅋㅋ👀

hey-sung · 2023-11-08T18:27:56Z

src/main/java/lotto/model/Lotto.java

+    public Integer count(Lotto lotto) {
+        return (int) IntStream.range(0, Rule.NUMBER_SIZE)
+                .filter(i -> lotto.contains(numbers.get(i)))
+                .count();
+    }


저도 동의합니다! 메서드명을 조금 더 자세히 적으면 의도가 드러나 좋을 것 같아요!

hey-sung · 2023-11-08T18:37:08Z

src/main/java/lotto/service/LottoIssuer.java

LottoIssuer라는 클래스를 따로 만들어서 적용하니 더 깔끔하고 가독성이 좋은 것 같아요..!!
클래스가 하나의 역할을 한다는 원칙에 잘 맞는 것 같습니다! 👍

hey-sung · 2023-11-08T18:37:33Z

src/main/java/lotto/service/PrizeCalculator.java

역시 인터페이스 잘 사용하시네요 👍👍

jinkshower

깔끔한 코드 잘 보고 갑니다
궁금하고 배울점이 많아서 그부분에 코멘트 남겨두고 가봅니다~

jinkshower · 2023-11-08T21:21:45Z

docs/README.md

+* [x] 구입금액이 1000 미만인 경우
+* [x] 구입금액이 1000으로 나누어 떨어지지 않는 경우
+* [x] 구입금액이 100,000을 초과하는 경우
+  * 복권 및 복권기금법 1인당 구매한도 초과 위반


ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 👍

jinkshower · 2023-11-08T21:25:10Z

src/main/java/lotto/controller/LottoController.java

+        outputView.printWinningStatistics(result);
+        Revenue revenue = calculate(winners, amount);
+        outputView.printTotalReturn(revenue);
+    }


이 함수 2~3개 정도로 분리하는건 어떠실까요?

저도 이 부분 메서드 작게 추출하면 더 좋을 것 같네요!

jinkshower · 2023-11-08T21:30:26Z

src/main/java/lotto/model/PurchaseAmount.java

+
+    public Integer exchangeLottoTicket() {
+        return amount / Rule.MONEY_UNIT;
+    }


이부분 어떤 기능인지 조금 더 명확하게 드러낼수 있지 않을까요?

저도 "티켓의 개수를 반환한다"라는 의미가 들어나면 좋을 것 같아요!

jinkshower · 2023-11-08T21:32:39Z

src/main/java/lotto/model/Rank.java

+            }
+        }
+        return null;
+    }


stream()을 이용하면 indent를 줄일수 있지 않을까요? 저는
https://techblog.woowahan.com/2527/
여기서 본 걸 적용해봤습니다

jinkshower · 2023-11-08T21:34:30Z

src/main/java/lotto/model/Result.java

+            result.put(rank, result.getOrDefault(rank, 0) + 1);
+        }
+        return new Result(result);
+    }


enumMap.... getOrDefault... JCF에 대한 학습을 좀 더 하고 싶어지네요 감사해요

jinkshower · 2023-11-08T21:36:31Z

src/main/java/lotto/service/LottoIssuer.java

+
+    public static LottoIssuer of(PurchaseAmount purchaseAmount) {
+        return new LottoIssuer(purchaseAmount.exchangeLottoTicket(), new LottoGenerator());
+    }


정적팩토리메서드를 사용하는 동근님의 기준이 있을까요? (몰라서)

purchaseAmount.exchangeLottoTicket() 를 생성자를 호출하며 선언하기 보다 이미 반환된 int 값을 넘겨주는 것이 어떨까요!

jinkshower · 2023-11-08T21:39:36Z

src/main/java/lotto/view/OutputViewImpl.java

+    public static final String BOUGHT_LOTTO_AMOUNT_MESSAGE_FORMAT = "%d개를 구매했습니다.%n";
+    public static final String RANK_PREFIX = String.format("당첨 통계%n" + "---");
+    public static final String PRIZE_MESSAGE_FORMAT = "%s (%s원) - %d개";
+    public static final String PRIZE_NUMBER_FORMAT = "###,###";


저도 처음에 이렇게 썼는데 이렇게 하면 수익률이 0일때 0.0%로 안나오더라구요. Decimal Format에서 #대신 0을 쓰면 수가 없을때 0을 채워준다고 합니다. 그래서 저는 "#,##0.0%" <- 요렇게 썼어요

jinkshower · 2023-11-08T21:41:06Z

src/test/java/lotto/LottoTest.java

+                Arguments.of(List.of(10, 2, 3, 45, 5, 7), true)
+        );
+    }
+}


MethodSource 너무 좋지 않나요...

jinkshower · 2023-11-08T21:42:54Z

src/test/java/lotto/converter/StringToIntegerTest.java

+        StringToInteger converter = new StringToInteger();
+        assertThat(converter.convert(value)).isEqualTo(Integer.valueOf(value.trim()));
+    }
+}


반복되는 객체 생성은 @beforeeach로 줄일 수가 있다네요!

jinkshower · 2023-11-08T23:18:43Z

src/main/java/lotto/model/Lotto.java

+    public String toString() {
+        return numbers.toString();
+    }
+}


Integer , Boolean을 int boolean대신 쓰는 이유가 궁금합니다 (정말)

참고링크
여기서 모든 원시값과 문자열을 포장했는가를 살짝 잘못이해한거 같아요 ㅎㅎ

jinkshower · 2023-11-08T23:21:19Z

src/test/java/lotto/model/RankTest.java

+class RankTest {
+
+    private static final Lotto winningLotto = new Lotto(List.of(3, 14, 15, 9, 2, 6));
+    private static final Bonus bonusNumber = new Bonus(5);


이렇게 따로 두면 되는데 전 이걸 다~~ 썼네요. 하하하하 배워갑니다

JaeMin-1 · 2023-11-09T02:39:06Z

src/main/java/lotto/model/Result.java

+        for (Rank rank : ranks) {
+            result.put(rank, result.getOrDefault(rank, 0) + 1);
+        }
+        return new Result(result);


EnumMap에 대해 배워갑니다 감사합니다!

JaeMin-1 · 2023-11-09T02:42:06Z

src/main/java/lotto/model/Rank.java

+    public static Rank from(Integer match, Boolean isSameBonus) {
+        if (SECOND.match.equals(match) && SECOND.isSameBonus.equals(isSameBonus)) {
+            return SECOND;
+        }
+        for (Rank prize : Rank.values()) {
+            if (prize.match.equals(match)) {
+                return prize;
+            }
+        }
+        return null;
+    }


if 문 3개로 코딩해서 뭔가 고쳐보고 싶었는데 이렇게도 할 수 있군요 배워갑니다!

hyeonji1220

역시 테스트 코드 장인 동근님👏👏 오늘도 꼼꼼한 테스트에 놀라면서 갑니다!
수고 많으셨어요!

hyeonji1220 · 2023-11-09T06:46:00Z

src/main/java/lotto/controller/InputController.java

+    }
+
+    private Lotto readWinningNumbers(InputView inputView) {
+        List<Integer> numbers = Arrays.stream(inputView.readWinningNumbers().split(NUMBER_SEPARATOR))


split으로 하고 넘어갈 경우 1,2,3,4,5,6,,,,, 이런 입력에 대해서는 예외를 던지지 않는데
이러한 형식은 예외처리가 필요없다고 생각하시는지 궁금합니다!

split을 하고 ,,, 라는 부분은 로또로 변환하며 !NUMERIC_PATTERN.matcher(source.trim()).matches() 에서 걸리게 되는 점이 맞을까요??
split한 개수가 6이 맞는지도 검증이 있으면 좋을 것 같습니다!

split한 개수가 6개가 맞는지에 대한 검증은 아래 Lotto 객체를 생성하고 그 곳에서 수행됩니다!
1,2,3,4,5,6,,,,, 오 이부분은 놓쳤네요
사실 1,2,3,4,5,6, 처럼 마지막에 "," 가 있는 경우는 예외 처리 하지 않았는데 여러개가 올때는 필요해보이네요👍

hyeonji1220 · 2023-11-09T06:48:36Z

src/main/java/lotto/controller/LottoController.java

+    }
+
+    private List<Lotto> buyLotto(PurchaseAmount amount) {
+        LottoIssuer lottoIssuer = LottoIssuer.of(amount);


이렇게 하면 LottoIssuer 객체가 PurchaseAmount 객체에 의존성을 갖게될 것 같습니다!
불필요한 의존관계는 유지 보수를 어렵게한다고 생각해서 특별한 이유가 없다면 의존관계를 최소화하기 위해
여기서 LottoIssuer.of(purchaseAmount.exchangeLottoTicket()) 이렇게 넣어줘도 좋을 것 같아요!

hyeonji1220 · 2023-11-09T07:01:43Z

src/main/java/lotto/view/OutputViewImpl.java

+    }
+
+    private String convertPrizeFormat(Integer prize) {
+        return new DecimalFormat(PRIZE_NUMBER_FORMAT).format(prize);


돈 관련 숫자 처리에 DecimalFormat를 쓰는 방법이 종종 보이네요! 덕분에 다시 알아갑니다😋

hyeonji1220 · 2023-11-09T07:05:54Z

src/test/java/lotto/service/PrizeCalculatorTest.java

+                        Collections.nCopies(second, SECOND),
+                        Collections.nCopies(third, THIRD),
+                        Collections.nCopies(fourth, FOURTH),
+                        Collections.nCopies(fifth, FIFTH)


nCopies라는 메소드가 있었군요! 동근님의 테스트 코드에서는 어떤 새로운 메소드를 발견하게 될까 기대하면서 보게됩니다ㅎㅎ

kyeoungchan

도메인과 서비스별로 책임 분리를 엄청 잘해주셨네요! 많이 배우고 갑니다

kyeoungchan · 2023-11-09T06:29:24Z

src/main/java/lotto/view/InputViewImpl.java

+    protected String readLine(String message) {
+        System.out.println(message);
+        String input = Console.readLine();
+        System.out.println();


이건 빼도 똑같이 빈줄이 나와서 입력 예시랑 같은 형태로 나오지 않을까요?

kyeoungchan · 2023-11-09T06:45:29Z

src/main/java/lotto/constant/Rule.java

@@ -0,0 +1,14 @@
+package lotto.constant;
+
+public class Rule {


ErrorMessage는 Enum인데 Rule은 일반 클래스로 작성하신 차이를 두는 기준이 있으실까요?

enum 사용시 값을 사용하려면 Rule.MIN_NUMBER.getValue() 이런 식으로 사용하게 되는데 정수관련된 값들은 뒤에 getValue 메서드를 사용하지 않고 바로 값을 가져오고 싶어서 사용했습니다!

kyeoungchan · 2023-11-09T07:17:00Z

src/main/java/lotto/service/Calculator.java

@@ -0,0 +1,5 @@
+package lotto.service;
+
+public interface Calculator<T> {


구현체가 하나인데 인터페이스로 분리한 이유가 혹시 있으실까요?

인터페이스를 사용하여 추상화하는 편을 좋아하는거 같습니다 ㅎㅎ

dgnppr

전반적인 코드 구조가 깔끔해서 읽기 좋네요!
이번주도 고생많으셨습니다 ㅎㅎ

dgnppr · 2023-11-09T08:40:30Z

src/main/java/lotto/controller/LottoController.java

+        outputView.printWinningStatistics(result);
+        Revenue revenue = calculate(winners, amount);
+        outputView.printTotalReturn(revenue);
+    }


저도 이 부분 메서드 작게 추출하면 더 좋을 것 같네요!

dgnppr · 2023-11-09T08:41:43Z

src/main/java/lotto/converter/StringToInteger.java

+            throw new IllegalArgumentException(ErrorMessage.INVALID_TYPE.getMessage(), e);
+        }
+    }
+}


컨버터 역할을 하는 커스텀 클래스에서 검증하는 부분 좋네요!👍

dgnppr · 2023-11-09T08:43:14Z

src/main/java/lotto/model/Lotto.java

+    public Integer count(Lotto lotto) {
+        return (int) IntStream.range(0, Rule.NUMBER_SIZE)
+                .filter(i -> lotto.contains(numbers.get(i)))
+                .count();
+    }


로또 생성자에서 꽤나 많은 책임이 있는 것 같아요! 외부에서 validate를 해서 주입하는 건 어떨까요?

dgnppr · 2023-11-09T08:45:39Z

src/main/java/lotto/view/OutputView.java

+    void printTotalReturn(Revenue revenue);
+
+    void printErrorMessage(Exception exception);
+}


OutputView를 인터페이스로 도입한 이유를 알 수 있을까요? 구현체가 하나이고, 자주 변경되는 기능이 아닐 것 같아서요. 모킹 테스트를 위함인가요?

저도 이부분이 궁금하네요 👍

oyoungsun · 2023-11-09T09:25:15Z

src/main/java/lotto/constant/ErrorMessage.java

+
+    INVALID_TYPE("잘못된 타입입니다"),
+    INVALID_LENGTH("잘못된 길이의 당첨 번호입니다."),
+    INVALID_LOTTO_RANGE("잘못된 범위의 로또 숫자입니다."),


String.format으로 로또의 범위를 구체적으로 알려줄 수 있을것 같아요!

정답은 없겠지만 이번에는 예외 메시지를 좀 폭넓게 사용하여 더 많은 곳에서 같은 예외 메시지를 던져보고 싶었습니다!

oyoungsun · 2023-11-09T09:27:47Z

src/main/java/lotto/controller/InputController.java

+                outputView.printErrorMessage(exception);
+            }
+        }
+    }


코드리뷰를 보다보니, try-catch에 대해서도 한번 더 분리를 할 수 있는 점을 알았어요! 예외 핸들링을 통해 try-catch를 분리하는 것은 어떻게 생각하세요?

oyoungsun · 2023-11-09T09:29:24Z

src/main/java/lotto/controller/InputController.java

+    }
+
+    private Lotto readWinningNumbers(InputView inputView) {
+        List<Integer> numbers = Arrays.stream(inputView.readWinningNumbers().split(NUMBER_SEPARATOR))


split을 하고 ,,, 라는 부분은 로또로 변환하며 !NUMERIC_PATTERN.matcher(source.trim()).matches() 에서 걸리게 되는 점이 맞을까요??
split한 개수가 6이 맞는지도 검증이 있으면 좋을 것 같습니다!

oyoungsun · 2023-11-09T09:31:27Z

src/main/java/lotto/model/PurchaseAmount.java

+
+    public Integer exchangeLottoTicket() {
+        return amount / Rule.MONEY_UNIT;
+    }


저도 "티켓의 개수를 반환한다"라는 의미가 들어나면 좋을 것 같아요!

oyoungsun · 2023-11-09T09:31:58Z

src/main/java/lotto/service/LottoIssuer.java

+
+    public static LottoIssuer of(PurchaseAmount purchaseAmount) {
+        return new LottoIssuer(purchaseAmount.exchangeLottoTicket(), new LottoGenerator());
+    }


purchaseAmount.exchangeLottoTicket() 를 생성자를 호출하며 선언하기 보다 이미 반환된 int 값을 넘겨주는 것이 어떨까요!

oyoungsun · 2023-11-09T09:33:55Z

src/main/java/lotto/view/OutputView.java

+    void printTotalReturn(Revenue revenue);
+
+    void printErrorMessage(Exception exception);
+}


저도 이부분이 궁금하네요 👍

ssuzyn

하나의 클래스가 하나의 역할을 할 수 있도록 분리시킨 노력이 보입니다 !! 잘 배우고 갑니다

ssuzyn · 2023-11-09T09:34:18Z

src/main/java/lotto/controller/InputController.java

+import java.util.Arrays;
+import java.util.List;
+
+public class InputController {


입력받는 부분을 따로 Controller로 분리한 것이 인상깊습니다 !!

ssuzyn · 2023-11-09T09:36:26Z

src/main/java/lotto/model/Lotto.java

+    private void validateLength(List<Integer> numbers) {
+        if (numbers.size() != Rule.NUMBER_SIZE) {
+            throw new IllegalArgumentException(ErrorMessage.INVALID_LENGTH.getMessage());
+        }
+    }
+
+    private void validateDuplicate(List<Integer> numbers) {
+        if (Set.copyOf(numbers).size() != Rule.NUMBER_SIZE) {
+            throw new IllegalArgumentException(ErrorMessage.LOTTO_DUPLICATE.getMessage());
+        }
+    }
+
+    private void validateRange(List<Integer> numbers) {
+        if (numbers.stream()
+                .anyMatch(number -> number < Rule.MIN_NUMBER || number > Rule.MAX_NUMBER)) {
+            throw new IllegalArgumentException(ErrorMessage.INVALID_LOTTO_RANGE.getMessage());
+        }
+    }


로또 번호를 검증하는 부분은 따로 분리해서 관리하는 것이 어떨까요??
Lotto 클래스에서 너무 많은 역할이 있는 것 같아요 !!

GIVEN53

책임 분리에 신경을 많이 쓰신게 느껴지네요 코드 재밌게 읽었습니다
마지막 4주차도 화이팅!

GIVEN53 · 2023-11-09T09:30:37Z

src/main/java/lotto/model/Bonus.java

+
+public class Bonus {
+
+    private final Integer number;


wrapper class로 선언하신 이유가 궁금합니다!

제가봐도 int가 더 좋아보이는군요!ㅎㅎ

GIVEN53 · 2023-11-09T09:34:41Z

src/main/java/lotto/model/Rank.java

+        if (SECOND.match.equals(match) && SECOND.isSameBonus.equals(isSameBonus)) {
+            return SECOND;
+        }
+        for (Rank prize : Rank.values()) {


values()는 호출할 때마다 enum의 모든 객체를 배열로 생성해서 리턴해요 반복적으로 사용되는 만큼 static 변수로 초기화해서 재사용하면 최적화할 수 있을 것 같아요

GIVEN53 · 2023-11-09T09:39:26Z

src/main/java/lotto/constant/Rule.java

+    public static final int MAX_NUMBER = 45;
+    public static final int NUMBER_SIZE = 6;
+    public static final int MIN_MONEY = 1_000;
+    public static final int MAX_MONEY = 100_000;


최대 구입금액도 제한두셨군요 👍🏻

GIVEN53 · 2023-11-09T09:41:25Z

src/main/java/lotto/converter/StringToInteger.java

+    @Override
+    public Integer convert(String source) {
+        validate(source);
+        return Integer.valueOf(source.trim());


validateRange()에 성공하면 int 형변환을 두 번 수행하게 되는데 한 번으로 줄이면 좋을 것 같아요

검증하는 메서드 따로, 형변환 하는 메서드 따로 분리하기 위해 그런식으로 사용하긴 했습니다만 중복이라는 점이 걸리긴 하네요

GIVEN53 · 2023-11-09T09:43:49Z

src/main/java/lotto/model/Rank.java

+                return prize;
+            }
+        }
+        return null;


null보다 미당첨을 의미하는 enum을 리턴하면 어떨까요?

저도 같은 생각입니다! 예를 들어 MISS, FAIL 등등..

Hanjaemo

3주차 미션 고생하셨습니다! 남은 일주일도 화이팅해서 꼭 합격하시길 기원합니다🔥🔥

Hanjaemo · 2023-11-09T10:07:47Z

src/main/java/lotto/model/Lotto.java

+    }
+
+    private void validateLength(List<Integer> numbers) {
+        if (numbers.size() != Rule.NUMBER_SIZE) {


메서드(validateLength)와 오류 메시지(INVALID_LENGTH)의 경우 length라는 네이밍을 사용했는데 상수의 경우 NUMBER_SIZE라고 지으신 이유가 있을까요?
저는 통일하는 것이 좋을 것 같다고 느껴집니다!

Hanjaemo · 2023-11-09T10:09:31Z

src/main/java/lotto/model/Lotto.java

+    }
+
+    private void validateDuplicate(List<Integer> numbers) {
+        if (Set.copyOf(numbers).size() != Rule.NUMBER_SIZE) {


이 부분 머리가 띵하네요,,
항상 Set<> set = new HashSet<> 이렇게 생성해서 검증했는데, copyOf를 사용하면 되는군요!!!

Hanjaemo · 2023-11-09T10:29:44Z

src/main/java/lotto/model/Lotto.java

+    }
+
+    public Integer count(Lotto lotto) {
+        return (int) IntStream.range(0, Rule.NUMBER_SIZE)


정수형으로 변환하신 이유가 있을까요?? 궁금합니다!

이 메서드는 당첨 번호랑 현재 로또 번호가 몇개 겹치는지 세는 메서드입니다! 아래에도 의미가 부족하다는 코멘트를 여럿 받은거 보니 메서드 명을 좀 더 의미 있게 짓거나 스트림 연산 메서드를 분리해야 겠네요 ㅎㅎ

Hanjaemo · 2023-11-09T11:11:56Z

src/main/java/lotto/model/Rank.java

+                return prize;
+            }
+        }
+        return null;


저도 같은 생각입니다! 예를 들어 MISS, FAIL 등등..

Hanjaemo · 2023-11-09T11:13:35Z

src/main/java/lotto/model/PurchaseAmount.java

+    @Override
+    public String toString() {
+        return "PurchaseAmount{" +
+                "amount=" + amount +
+                "}";
+    }


모든 클래스에 toString()을 재정의 하시는 이유가 궁금합니다!

이펙티브 자바를 읽다가 모든 클래스에서 toString() 을 재정의하라는 키워드가 있어서 적용해보았습니다!
이유는 toString 메서드의 용도는 로깅을 찍기위한 메서드로 언제 어디서 사용될 지 모르기에 해당 객체가 가진 정보를 출력할 의무를 가지고(?) 재정의 하였습니다

Hanjaemo · 2023-11-09T11:25:35Z

src/main/java/lotto/model/Revenue.java

+
+public record Revenue(Double number) {
+
+    private static final String ROUND_FORMAT = "%.1f";


"%.1f"는 출력을 담당하는 객체에서 관리하는 것은 어떨까요?
소수점 출력 형식을 변경하라는 요구 사항이 �왔을 때 저는 이 모델 객체보다 출력 로직을 먼저 볼 것 같아요!

Hanjaemo · 2023-11-09T11:27:47Z

src/main/java/lotto/service/LottoGenerator.java

@@ -0,0 +1,18 @@
+package lotto.service;


LottoGenerator, LottoIssuer, PrizeCalculator클래스를 모델이 아닌 서비스 패키지에 놓으신 이유가 있을까요?

Hanjaemo · 2023-11-09T11:30:27Z

src/main/java/lotto/model/PurchaseAmount.java

+
+public class PurchaseAmount {
+
+    private final Integer amount;


래퍼 클래스를 사용하면 null을 허용할텐데, 모든 필드를 래퍼 클래스로 사용하신 이유가 궁금합니다!

Hanjaemo · 2023-11-09T11:36:51Z

src/main/java/lotto/model/Rank.java

+    public static Rank from(Integer match, Boolean isSameBonus) {
+        if (SECOND.match.equals(match) && SECOND.isSameBonus.equals(isSameBonus)) {
+            return SECOND;
+        }
+        for (Rank prize : Rank.values()) {
+            if (prize.match.equals(match)) {
+                return prize;
+            }
+        }
+        return null;


enum에 람다식을 사용할 수 있더라구요??
이를 활용하면 코드가 훨씬 간결해지실 것 같아요!

제가 참고했던 블로그입니다!
https://inpa.tistory.com/entry/JAVA-%E2%98%95-%EC%97%B4%EA%B1%B0%ED%98%95Enum-%ED%83%80%EC%9E%85-%EB%AC%B8%EB%B2%95-%ED%99%9C%EC%9A%A9-%EC%A0%95%EB%A6%AC#name_%EB%A9%94%EC%86%8C%EB%93%9C

Hanjaemo · 2023-11-09T11:38:40Z

src/main/java/lotto/model/Rank.java

+
+    private final Integer match;
+    private final Boolean isSameBonus;
+    private final Integer prize;


상금은 정수의 최댓값을 충분히 넘길 수도 있을 것 같아요! Long을 사용해보시는 건 어떨까요?

acycoco

많이 배워갑니다!! 4주차도 화이팅입니다!

acycoco · 2023-11-09T15:54:05Z

src/main/java/lotto/converter/StringToInteger.java

+    }
+
+    private void validateType(String source) {
+        if (source == null || !NUMERIC_PATTERN.matcher(source.trim()).matches()) {


개인적으로 이부분은 메서드로 한번 더 분리하는 게 더 가독성이 좋을 것 같습니다!

acycoco · 2023-11-09T15:58:58Z

src/main/java/lotto/model/Rank.java

+    FOURTH(4, false, 50_000, "4개 일치"),
+    THIRD(5, false, 1_500_000, "5개 일치"),
+    SECOND(5, true, 30_000_000, "5개 일치, 보너스 볼 일치"),
+    FIRST(6, false, 2_000_000_000, "6개 일치");


모델에서 메세지에 대한 부분까지 가지고 있으면
메세지 내용이 변경될 때 모델 부분을 수정해야되는 문제점을 가지고 있을 것 같습니다

acycoco · 2023-11-09T16:01:34Z

src/test/java/lotto/LottoTest.java

+                Arguments.of(List.of(10, 2, 3, 45, 5, 7), true)
+        );
+    }
+}


이런식으로도 작성할 수 있군요! 배워갑니다!

ChoiWonYu

다른 분들이 제가 궁금한 부분들을 모두 말씀해 주셔서 코멘트가 적네요...!
코드 잘보고 갑니다 고생하셨습니다! 😁

ChoiWonYu · 2023-11-10T07:29:59Z

src/main/java/lotto/controller/InputController.java

+import java.util.Arrays;
+import java.util.List;
+
+public class InputController {


입력에 대한 처리를 이 객체에서 하는 것 같은데
굳이 inputView를 LottoController에 주입하고 여기에 전달해 주는 이유가 궁금합니다..!

로또 컨트롤러가 메인컨트롤러 느낌이고, 인풋컨트롤러는 정상적인 입력을 받을때까지 입력받는 역할을 담당하고 있는 컨트롤러입니다!
사실 처음에 인풋컨트롤러를 만들었을때 인풋뷰, 아웃풋뷰 모두 인풋컨트롤러의 인스턴스 변수였는데, 그렇게 되다보니 로또 컨트롤러와 차이점을 잘 못느껴 구분하고자 매개변수로 받는 방법을 택했는데, 적으면서 다시 생각해보니 아예 inputView를 로또 컨트롤러에서 가지고 있을 필요가 없을거 같군요 ㅎㅎ👍

ChoiWonYu · 2023-11-10T07:42:45Z

src/main/java/lotto/model/PurchaseAmount.java

+    private final Integer amount;
+
+    private PurchaseAmount(Integer amount) {
+        validate(amount);


특정 유효성 검사 로직을 정적 팩토리 메서드가 아닌 생성자에서 하신 이유가 궁금합니다!

그것도 방법이겠군요! static 메서드를 사용할때에는 항상 조심하면서 사용하는 편이라 정적 팩토리 메서드 내부 대신 객체 생성 후 사용하는 방식으로 했습니다!

ChoiWonYu · 2023-11-10T07:46:58Z

src/main/java/lotto/model/Rank.java

+
+    public static Rank from(Integer match, Boolean isSameBonus) {
+        if (SECOND.match.equals(match) && SECOND.isSameBonus.equals(isSameBonus)) {
+            return SECOND;


이렇게 처리되면 보너스 번호 조건을 가지는 rank가 변할 때마다 수정이 되어야 하겠네요! rank의 조건만 변경해줘도 수정 없이 반영되도록 할 수 있을 것 같습니다..!

reddevilmidzy added 30 commits November 2, 2023 22:18

docs: 구현 기능 목록 생성

37f73e9

feat: 로또 구입금액 입력

4a26ead

feat: 로또 발행

450a1af

feat: 구입금액 만큼 로또 구매

6aa4b01

feat: 발행한 로또 수량 및 번호 출력

927344c

feat: 당첨 번호 입력

b6c33ea

feat: 보너스 번호 입력

77d8b57

feat: 당첨 번호와 구입한 로또 중 3개 일치 판별

fae78cf

test: 구매한 로또 번호와 당첨 로또 번호 중 일치하는 번호의 갯수 반환

4b00e53

test: 구매한 로또 번호와 보너스 번호가 일치하는지 확인

a8148fd

test: 당첨 번호와 구입한 로또 및 보너스 일치 판별

451b059

feat: 수익률 계산

bc6a50b

feat: 당첨 내역 출력

c5483f2

feat: 수익률 출력

6b76d26

feat: 당첨 번호내에 중복이 있는 경우 예외

3c5fa3f

feat: 당첨 번호가 1부터 45 범위의 수가 아닌 경우 예외

be25eb8

chore: Lotto 패키지 변경

4a7aabe

refactor: 보너스 숫자 1급 컬랙션으로 변경

4763179

feat: 보너스 번호가 1부터 45 범위의 수가 아닌 경우 예외

640ed01

refactor: 구입금액 1급컬렉션으로 변경

4da61a4

feat: 구입 금액이 1000원 미만 100000 초과인 경우 예외

af4b2af

feat: 구입금액이 1000으로 나누어 떨어지지 않는 경우 예외

f0e1b64

feat: 보너스 번호가 당첨 번호랑 중복인 경우 예외

8dd18b6

feat: 구입금액 혹은 보너스 번호가 숫자가 아닌 경우 예외

9d82095

feat: 당첨 번호가 숫자가 아닌 경우 예외

c3e823f

feat: 예외 상황시 에러 문구 출력

6dbfff6

feat: 예외 발생시 해당 부분 다시 입력

32c30e0

refactor: 컨트롤러 클래스 책임 분리

c7a081f

refactor: 컨트롤러 클래스 run() 메서드 책임 분리

1bd5120

refactor: OutputView 인터페이스 생성

88aa448

reddevilmidzy added 14 commits November 6, 2023 17:31

style: 코드 개행 수정

b654848

docs: 리팩토링 목록 추가

4971c64

feat: toString 오버라이드

1dab946

feat: 앞뒤 공백이 있다면 공백 제거후 Integer 변환

118722e

refactor: 입력받는 메서드들 중복 제거

33286d3

refactor: 문자열 상수로 변경

624dc8b

refactor: 메서드명 변경

309ab09

docs: 주석 제거

4fcc507

refactor: enum명 및 필드명 변경

ad79fdf

refactor: View 메시지 상수화

46c3be2

refactor: Rank 문자 하드코딩 제거

f12af7c

docs: 구현 기능 목록 수정

34fd0c5

refactor: 수익률 1급 컬렉션 적용

7c05372

style: 파라미터 명 변경

4a2c8d3

SJ70 reviewed Nov 8, 2023

View reviewed changes

hey-sung reviewed Nov 8, 2023

View reviewed changes

jinkshower reviewed Nov 8, 2023

View reviewed changes

JaeMin-1 reviewed Nov 9, 2023

View reviewed changes

hyeonji1220 reviewed Nov 9, 2023

View reviewed changes

kyeoungchan reviewed Nov 9, 2023

View reviewed changes

dgnppr reviewed Nov 9, 2023

View reviewed changes

oyoungsun reviewed Nov 9, 2023

View reviewed changes

ssuzyn reviewed Nov 9, 2023

View reviewed changes

GIVEN53 reviewed Nov 9, 2023

View reviewed changes

Hanjaemo reviewed Nov 9, 2023

View reviewed changes

acycoco reviewed Nov 9, 2023

View reviewed changes

ChoiWonYu reviewed Nov 10, 2023

View reviewed changes

		@@ -0,0 +1,5 @@
		package lotto.service;

		public interface Calculator<T> {


		public record Revenue(Double number) {

		private static final String ROUND_FORMAT = "%.1f";

Conversation

reddevilmidzy commented Nov 7, 2023

Uh oh!

SJ70 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyeonji1220 commented Nov 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinkshower left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinkshower Nov 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SJ70 left a comment •

edited

Loading

jinkshower Nov 8, 2023 •

edited

Loading