2단계 - 로또(자동) #4077

chanSeong95 · 2025-03-28T14:35:01Z

리뷰 부탁드려요.

추가: - InputView.java

추가: - LottoMachine.java - LottoNumber.java - LottoRound.java - LottoRank.java

추가: - Lotto.java - ResultView.java 변경: - LottoRound.java: 로또 번호 리스트에 대한 결과 응답 메소드 추가

testrace

안녕하세요 찬성님 😃
2단계 구현 잘해주셨네요 👍
몇 가지 코멘트 좀 남겨두었는데 확인해 주시고 다시 리뷰 요청해 주세요.

testrace · 2025-03-29T11:47:08Z

src/main/java/lotto/Lotto.java

+    }
+
+    public static void lotto() {
+        int lottoCount = InputView.getPositiveNumberInput("구입금액을 입력해 주세요.") / 1000;


로또 한 장은 1000원이다. 비즈니스 로직으로 볼 수 있지 않을까요?
적절한 도메인 객체에게 책임을 부여하면 어떨까요?

testrace · 2025-03-29T11:47:15Z

src/main/java/lotto/LottoConfig.java

+public class LottoConfig {
+    public static final Integer MAX_LOTTO_NUMBER = 45;
+    public static final int LOTTO_PRICE = 1000;
+    public static final int LOTTO_NUMBER_SIZE = 6;


각 상수가 필요한 객체 내부에 선언해 응집도를 높이면 어떨까요?

testrace · 2025-03-29T11:47:23Z

src/main/java/lotto/LottoMachine.java

+import static lotto.LottoConfig.MAX_LOTTO_NUMBER;
+
+public class LottoMachine {
+    private static final List<Integer> LOTTO_NUMBERS = IntStream.range(1, MAX_LOTTO_NUMBER + 1).boxed().collect(Collectors.toList());


지난 단계에서도 피드백 드린 부분인데요
스트림을 활용하신다면 개행으로 가독성을 높여 보시면 좋겠습니다.

매직 넘버 '1' 도 상수로 치환하면 어떨까요?

testrace · 2025-03-29T11:48:06Z

src/main/java/lotto/Lotto.java

+public class Lotto {
+    public static void main(String[] args) {


애플리케이션의 진입점이니 LottoMain, LottoApplication 같은 이름은 어떠신가요?

testrace · 2025-03-29T11:48:16Z

src/main/java/lotto/LottoNumber.java

+public class LottoNumber {
+    private final Set<Integer> numbers;


여러 개의 숫자를 다루니 클래스명을 복수로 표현하면 어떨까요?

testrace · 2025-03-29T11:50:15Z

src/main/java/lotto/view/ResultView.java

+        long winnings = rankCount.entrySet().stream()
+                .mapToLong(entry -> (long)entry.getKey().getMoney() * entry.getValue())
+                .sum();
+
+        System.out.printf("총 수익률은 %.2f입니다.\n", (double)winnings / (totalCount * LOTTO_PRICE));


당첨금 합산이나 수익률 계산은 비즈니스 로직으로 봐야하지 않을까요?
ResultView는 출력 자체에 집중하면 어떨까요?

testrace · 2025-03-29T11:50:48Z

src/test/java/lotto/LottoNumberTest.java

+import static org.junit.jupiter.api.Assertions.*;
+
+class LottoNumberTest {
+    @DisplayName("LottoNumber 조건, 1~45 사이의 수 and 6개")


하나의 테스트에서 여러가지를 검증한다면 테스트 대상 메서드가 너무 많은 일을 하고 있는건 아닌지 고민해 볼 필요가 있습니다.
앞서 드린 코멘트처럼 번호의 범위에 대한 책임을 분리한다면 테스트도 간단해질 것 같아요.

testrace · 2025-03-29T11:51:10Z

src/test/java/lotto/LottoRankTest.java

+        Assertions.assertThat(LottoRank.valueOfMatchCount(2))
+                .isNull();


null을 활용하면 언제나 NPE의 위험에 노출이 됩니다.
2개 이하 일치하는 '미당첨' 타입을 추가하면 어떨까요?

testrace · 2025-03-29T11:51:29Z

src/test/java/lotto/LottoRankTest.java

+        Assertions.assertThat(LottoRank.valueOfMatchCount(2))
+                .isNull();


null을 활용하면 언제나 NPE의 위험에 노출이 됩니다.
2개 이하 일치하는 '미당첨' 타입을 추가하면 어떨까요?

testrace · 2025-03-29T11:51:34Z

src/test/java/lotto/LottoRoundMachineTest.java

+class LottoRoundMachineTest {
+    @DisplayName("로또 번호 생성 테스트, 검증된 라이브러리만 사용하여 LottoNumber 객체 생성만 확인")
+    @Test
+    public void lottoNumber() throws Exception {
+        Assertions.assertThat(LottoMachine.generateLottoNumber())
+                .isNotNull();


테스트의 의도가 분명하지 않은 것 같아요.
마치 통합테스트를 의도하신 것 같기도 느낌인데요
LottoRoundMachine 이라는 클래스도 없고, null 비교만으로 검증이 충분한지도 검토할 필요가 있다고 생각해요.

- LottoNumbersCondition.java 추가: 로또 번호 set에 대한 유효성 체크 메소드 제공 - LottoStore.java 추가: 로또 판매 및 당첨금 전달 역할 - LottoRank.java: ETC 추가 - ResultView.java: 수익률 계산 로직 제거

chanSeong95 · 2025-03-30T11:27:18Z

리뷰 주신 부분 수정했고 재리뷰 부탁드려요.

LottoNumbers 클래스의 번호의 범위에 대한 책임을 분리하다보니 LottoNumbersCondition 클래스를 추가했고 LottoMachine(LottoRoundMachine 클래스는 일반 클래스 이름 바꾼걸 같이 적용 안했네요.) 도 LottoNumberCondition 클래스에서 상수 설정값을 가져오다 보니 테스트는 편해졌지만 조금 어글리한 부분이 생겼습니다.

LottoMachine.lottoNumbers를 static 변수로 사용 불가. 매번 생성할 필요 없는 변수 값인데 인스턴스 마다 생성되어야 합니다.

혹시 이부분에 대해서는 다른 대안 있을까요?

testrace

안녕하세요 찬성님 😃
피드백 반영 잘해주셨네요 👍
질문에 대한 답변과 소소한 코멘트 남겨두었습니다.
지난 리뷰에서 놓치신 코멘트도 있는 것 같은데 확인해 주시고 리뷰 요청해 주세요.

testrace · 2025-03-30T16:43:18Z

src/main/java/lotto/LottoMain.java

+        Map<LottoRank, Integer> lottoRankToCountMap = lottoRound.checkLottoRank(lottoNumbers);
+        double rateOfReturn = (double) LottoStore.receiveWinnings(lottoRankToCountMap) / (budget * LottoStore.LOTTO_PRICE);


수익률 계산 로직도 객체에게 메시지를 보내 구하면 어떨까요?

Suggested change

Map<LottoRank, Integer> lottoRankToCountMap = lottoRound.checkLottoRank(lottoNumbers);

double rateOfReturn = (double) LottoStore.receiveWinnings(lottoRankToCountMap) / (budget * LottoStore.LOTTO_PRICE);

MatchResult matchResult = lottoRound.checkLottoRank(lottoNumbers);

double rateOfReturn = matchResult.returnRate(구입금액);

testrace · 2025-03-30T16:44:21Z

src/main/java/lotto/LottoNumbersCondition.java

+
+import java.util.Set;
+
+public class LottoNumbersCondition {


의도가 조금 잘못 전달된 것 같아요 😅
책임을 위임한다는 것이 단순히 조건을 분리하기 보다 책임을 수행할 수 있는 객체(상태+행위)를 도출해 보시면 좋을 것 같아요.

new LottoNumber(45); // 생성 가능 new LottoNumber(46); // 생성 불가 new LottoNumber(45).equals(new LottoNumber(45)); // true

Condition 클래스를 제거하고 원시 값을 포장한 객체를 도출한다면 질문 하셨던 '매번 인스턴스를 생성하는 상황'도 해결할 수 있을 것 같아요.

testrace · 2025-03-30T16:44:48Z

src/main/java/lotto/LottoRank.java

+    FOURTH(LottoNumbersCondition.getDefaultInstance().getLottoNumberSize() - 3, 5000),
+    THIRD(LottoNumbersCondition.getDefaultInstance().getLottoNumberSize() - 2, 50000),
+    SECOND(LottoNumbersCondition.getDefaultInstance().getLottoNumberSize() - 1, 1500000),
+    FIRST(LottoNumbersCondition.getDefaultInstance().getLottoNumberSize(), 2000000000),


지난 번에 코멘트 남겼던 부분인데 메시지가 접혀 있어서 확인을 못하신 것 같아요.
이전 코멘트 들 중간에

이 부분을 클릭하면 펼칠 수 있으니 참고해 주세요.

- LottoNumbersCondition.java 제거 - LottoNumber.java 추가 - LottoResult.java 추가: 보유한 로또 당첨 개수 확인 및 수익률 계산

chanSeong95 · 2025-04-02T08:44:41Z

재리뷰 부탁드려요.

입력을 재귀로 받는 것의 경우 사용자가 무한히 실패하게되면 스택 오버플로우가 발생할 것이라 우선 유지했습니다. matchCount를 수식으로 표현한 것은 로또 자체가 국가별로 숫자 및 번호 수가 다르지만 기본룰은 비슷하기 때문에 상수로 빼두고 그 값을 사용하게 했습니다.

testrace

안녕하세요 찬성님 😃
피드백 반영 잘해주셨네요 👍
소소하게 코멘트 남겨두었는데 확인해 주시고 다음 단계 진행해 주세요!

testrace · 2025-04-02T14:18:40Z

src/main/java/lotto/LottoNumber.java

+package lotto;
+
+public class LottoNumber implements Comparable<LottoNumber> {


불변 객체 👍
도메인 객체들은 별도의 패키지로 분리해보시면 좋을 것 같아요.

testrace · 2025-04-02T14:18:46Z

src/main/java/lotto/LottoNumbers.java

-
-    public LottoNumbers(Set<Integer> numbers) {
-        this(LottoNumbersCondition.getDefaultInstance(), numbers);
+    public final static int LOTTO_SIZE = 6;


Suggested change

public final static int LOTTO_SIZE = 6;

public static final int LOTTO_SIZE = 6;

testrace · 2025-04-02T14:19:11Z

src/main/java/lotto/LottoRank.java

+    FOURTH(LOTTO_SIZE - 3, 5000),
+    THIRD(LOTTO_SIZE - 2, 50000),
+    SECOND(LOTTO_SIZE - 1, 1500000),
+    FIRST(LOTTO_SIZE, 2000000000),


matchCount를 수식으로 표현한 것은 로또 자체가 국가별로 숫자 및 번호 수가 다르지만 기본룰은 비슷하기 때문에 상수로 빼두고 그 값을 사용하게 했습니다.

여러 상황을 고려하셨군요!
다만 당첨금은 원화 기준에다가 당첨 금액도 달라질 수 있으니 다양한 상황에도 재사용이 가능할지는 고민해 보셔도 좋을 것 같아요.

testrace · 2025-04-02T14:19:29Z

src/test/java/lotto/LottoNumberTest.java

+class LottoNumberTest {
+    @DisplayName("로또 숫자 범위 테스트")


testrace · 2025-04-02T14:19:35Z

src/test/java/lotto/LottoResultTest.java

+import static org.assertj.core.api.Assertions.assertThat;
+
+class LottoResultTest {
+    @DisplayName("로또 결과 테스트")


DisplayName에 총 당첨금, 수익률 계산 이라는 의도를 명시하면 어떨까요?

brody added 4 commits March 28, 2025 21:47

docs(README.md): 기능 요구 사항 정리

5fa09b7

feat: input 클래스 추가

454d989

추가: - InputView.java

feat: 로또 기본 구조 추가

402f4dd

추가: - LottoMachine.java - LottoNumber.java - LottoRound.java - LottoRank.java

feat: 로또 전체 시나리오 맟 출력 클래스 추가

93c0059

추가: - Lotto.java - ResultView.java 변경: - LottoRound.java: 로또 번호 리스트에 대한 결과 응답 메소드 추가

testrace requested changes Mar 29, 2025

View reviewed changes

refactor:

c3f6ba7

- LottoNumbersCondition.java 추가: 로또 번호 set에 대한 유효성 체크 메소드 제공 - LottoStore.java 추가: 로또 판매 및 당첨금 전달 역할 - LottoRank.java: ETC 추가 - ResultView.java: 수익률 계산 로직 제거

testrace requested changes Mar 30, 2025

View reviewed changes

refactor:

8769d99

- LottoNumbersCondition.java 제거 - LottoNumber.java 추가 - LottoResult.java 추가: 보유한 로또 당첨 개수 확인 및 수익률 계산

testrace approved these changes Apr 2, 2025

View reviewed changes

testrace merged commit 6604028 into next-step:chanseong95 Apr 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2단계 - 로또(자동) #4077

2단계 - 로또(자동) #4077

chanSeong95 commented Mar 28, 2025

testrace left a comment

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

testrace Mar 29, 2025

chanSeong95 commented Mar 30, 2025

testrace left a comment

testrace Mar 30, 2025

testrace Mar 30, 2025

testrace Mar 30, 2025

chanSeong95 commented Apr 2, 2025

testrace left a comment

testrace Apr 2, 2025

testrace Apr 2, 2025

testrace Apr 2, 2025

testrace Apr 2, 2025

testrace Apr 2, 2025

		public class Lotto {
		public static void main(String[] args) {

		public class LottoNumber {
		private final Set<Integer> numbers;

		Assertions.assertThat(LottoRank.valueOfMatchCount(2))
		.isNull();

		Map<LottoRank, Integer> lottoRankToCountMap = lottoRound.checkLottoRank(lottoNumbers);
		double rateOfReturn = (double) LottoStore.receiveWinnings(lottoRankToCountMap) / (budget * LottoStore.LOTTO_PRICE);

		package lotto;

		public class LottoNumber implements Comparable<LottoNumber> {

	public final static int LOTTO_SIZE = 6;
	public static final int LOTTO_SIZE = 6;

		class LottoNumberTest {
		@DisplayName("로또 숫자 범위 테스트")

2단계 - 로또(자동) #4077

2단계 - 로또(자동) #4077

Conversation

chanSeong95 commented Mar 28, 2025

testrace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanSeong95 commented Mar 30, 2025

testrace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanSeong95 commented Apr 2, 2025

testrace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment