[로또] 조윤호 제출합니다 #3

ray-yhc · 2023-03-30T01:48:41Z

로또의 최대, 최솟값, 숫자개수를 여러 클래스에서 사용해야 해서, 어느 클래스에 저장해 놓아도 부자연스럽더라구요. 결국 LottoConfig라는 새로운 클래스에 저장해놓았습니다. 이렇게 하는게 맞는지 모르겠네요~

또 테스트코드에서 List.of()를 사용하게 되면 immutable이라 sort가 안되더라구요. 그래서 immutable 관련 예외가 발생하면 다음과 같이 그냥 무시해버렸는데, 마음에 걸립니다 ㅎㅎ.. (프리코스때도 이 문제로 꽤 고생했던걸로 기억해요!)

// Lotto.java line 17
try {
    numbers.sort(Comparator.naturalOrder());
} catch (UnsupportedOperationException ignored) {}

코드 보시며 다양한 의견과 조언 적어주시면 큰 도움이 될 것 같습니다!

… the list

seong-wooo

저는 util 클래스는 어떤 프로젝트에 갖다 놔도 사용 가능한 클래스라고 생각해요.
LottoUtil 같은 경우는 딱 이 미션에서만 사용가능한 것이겠죠?
이 미션의 핵심 로직이 Util 클래스로 새어나갔다는 뜻..

seong-wooo · 2023-03-30T03:38:41Z

src/main/java/lotto/config/LottoConfig.java

+    public final static int LOTTO_NUM_LENGTH = 6;
+    public final static int LOTTO_MIN_NUM = 1;
+    public final static int LOTTO_MAX_NUM = 45;


public static final 형태로 상수를 관리하는 것은 굉장히 안좋은 방법입니다!
절차 지향적인 방법으로 코드를 작성하고 계신것 같아요

seong-wooo · 2023-03-30T03:40:25Z

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

+        try {
+            numbers.sort(Comparator.naturalOrder());
+        } catch (UnsupportedOperationException ignored) {}


프로덕션 코드에서 테스트코드의 영향을 받아야할까요?
테스트 작성시 문제가 생겼다면 테스트 코드를 변경하는게 맞지 않을까요??

seong-wooo · 2023-03-30T03:42:52Z

src/main/java/lotto/lotto/LottoBundle.java

+        for (Lotto lotto : lottos)
+            sb.append(lotto).append("\n");
+
+        UserOutput.printLottos(lottos.size(), sb.toString());


Domain에서 view에 의존하는 것이 맞을까요?
MVC 패턴을 생각해보면 Model View Controller 가 서로 분리되도록 설계합니다.
왜 분리되어야할까요?

seong-wooo · 2023-03-30T03:48:09Z

src/main/java/lotto/lotto/LottoBundle.java

+public class LottoBundle {
+    private List<Lotto> lottos = new ArrayList<>();
+    private Lotto winningLotto;
+    private int bonus;
+


불변 객체로 만들지 않으면 프로그램이 동작할때 어떤 문제가 발생할까요?

jeongyuneo

함수에서 한 가지보다 많은 일들을 수행하고 있는 경우가 많은 것 같아요!

개인적으로 코드 컨벤션을 중요하게 생각해서, 코드 컨벤션에 대해 한 번 읽어보시고 프로그래밍할 때 신경써서 구현하시면 전체적으로 코드 가독성이 더 높아질 것 같아요ㅎㅎ
아래 링크에 나오는 컨벤션이 정답은 아닙니다! 참고하시라고 같이 첨부해드려요😄
자바 코드 컨벤션

jeongyuneo · 2023-03-30T07:03:55Z

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

+        if (numbers.size() != LottoConfig.LOTTO_NUM_LENGTH)
+            throw new IllegalArgumentException("[ERROR] 숫자의 개수가 올바르지 않습니다.");
+        if (numbers.size() != (new HashSet<>(numbers)).size())
+            throw new IllegalArgumentException("[ERROR] 숫자에 중복이 없어야 합니다.");


validate가 한 번에 '개수 검증'과 '중복 검증'이라는 두 가지 일을 처리하고 있네요.
개수 검증과 중복 검증을 분리하면 메소드의 책임이 좀 더 명확해질 것 같아요!

jeongyuneo · 2023-03-30T07:14:11Z

src/main/java/lotto/purchase/PurchaseService.java

+        if (cost <=0 )
+            throw new IllegalArgumentException("[ERROR] 1 이상의 양수를 입력해주세요");


음수와 0 처리는 생각하지 못했던 부분이네요! 👍🏻

jeongyuneo · 2023-03-30T07:16:13Z

src/main/java/lotto/purchase/PurchaseService.java

+        if (cost <=0 )
+            throw new IllegalArgumentException("[ERROR] 1 이상의 양수를 입력해주세요");
+        if (cost % PRICE_PER_LOTTO != 0)
+            throw new IllegalArgumentException("[ERROR] 가격의 배수에 해당하는 가격을 입력해주세요");


검증의 책임을 분리하면 좋을 것 같아요!

jeongyuneo · 2023-03-30T07:49:49Z

src/main/java/lotto/lotto/LottoService.java

+        int num;
+        try {
+            num = Integer.parseInt(input);
+        } catch (NumberFormatException ne) {
+            throw new IllegalArgumentException("[ERROR] 정수를 입력해주세요.");
+        }
+        return num;


이런 식으로 early return 패턴을 사용하는게 더 가독성이 좋을 것 같아요!

Suggested change

int num;

try {

num = Integer.parseInt(input);

} catch (NumberFormatException ne) {

throw new IllegalArgumentException("[ERROR] 정수를 입력해주세요.");

}

return num;

try {

return Integer.parseInt(input);

} catch (NumberFormatException ne) {

throw new IllegalArgumentException("[ERROR] 정수를 입력해주세요.");

}

jeongyuneo · 2023-03-30T07:58:13Z

src/main/java/lotto/lotto/LottoService.java

+        int cost = getNum(userInput);
+        int lottoNum = PurchaseService.getLottoNum(cost);


구입금액은 그냥 정수보다 좀 더 의미있는 값 같아요. '구입금액'이라는 객체를 생성하는 건 어떨까요?
그렇게 되면 '구입금액'이라는 객체를 생성할 때 검증을 할 수 있어서, 구입금액을 구매할 로또 개수를 가져올 때 계산하는 것보다 흐름이 자연스러울 것 같아요!

jeongyuneo · 2023-03-30T08:02:46Z

src/main/java/lotto/lotto/LottoBundle.java

+    /**
+     * 보너스번호 저장하기
+     */
+    public void setBonus(int bonus) {


setter 사용 대신 좀 더 의미있는 네이밍을 하면 좋을 것 같아요!

jeongyuneo · 2023-03-30T08:06:40Z

src/main/java/lotto/lotto/LottoBundle.java

+        Lotto lotto = new Lotto(winningNumList);
+        this.winningLotto = lotto;


Suggested change

Lotto lotto = new Lotto(winningNumList);

this.winningLotto = lotto;

this.winningLotto = new Lotto(winningNumList);

재사용 가능성이 없는 경우 변수에 담지 않고 바로 사용하는게 더 좋은 것 같아요!

jeongyuneo · 2023-03-30T08:19:39Z

src/main/java/lotto/lotto/LottoBundle.java

+    public int[] getPrizeStat() {
+        int[] prizeStat = new int[Prize.values().length];
+        for (Lotto lotto : lottos) {
+            Prize prize = lotto.comparePrize(winningLotto, bonus);
+            if (prize.isPrized())
+                prizeStat[prize.ordinal()]++;
+        }
+        return prizeStat;
+    }
+
+    public int getTotalReward(int[] prizeStat) {


두 메소드는 LottoBundle 내부에서만 사용되네요. private로 선언해주시면 좋을 것 같아요!

jeongyuneo · 2023-03-30T08:22:23Z

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

+        for (Prize p : Prize.values()) {
+            if (
+                    p.isPrized() &&
+                    p.getMatchNum() <= matches &&
+                    p.getGrade() <= prize.getGrade()
+            ) {
+                if (p.isBonus() && !isBonus) // 보너스 true인 경우 isBonus 확인한다.
+                    continue;
+                prize = p;
+            }
+        }


indent(인덴트, 들여쓰기) depth를 3이 넘지 않도록 구현한다. 2까지만 허용한다.

예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.

힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메서드)를 분리하면 된다.
라는 프로그래밍 요구사항을 만족하지 않는 구현 같습니다!

ray-yhc added 22 commits March 29, 2023 13:11

refactor : edit Lotto package structure

8b7f9c7

feat : add LottoBundle, LottoService

a97de00

feat : add utils

ec15121

feat : add purchase

0ada73b

feat : add Prize

583242a

feat : add configs

6ba1265

test : add test for LottoService

f51b94d

test : add test for Lotto

77b2e36

test : add test for PurchaseService

a063735

test : edit LottoTest

5b0a3d4

feat : edit main method

5244242

feat : add PurchaseService

3c03b63

feat : add LottoBundle

d878bb0

feat : add Lotto

b4bb8e7

chore : edit output format

8099c5c

chore : edit comments

fde7c09

test : fix error for CsvSource

184b14e

feat : add lotto sort

51e4d73

feat : add validation code for getNumList

14407eb

feat : add pried field for Prize

44e98a1

chore : remove unused imports

47cd5d9

fix : add validation that bonus does not duplicate with the number in…

f1da751

… the list

ray-yhc requested review from jeongyuneo, minnseong, seong-wooo and great-park March 30, 2023 01:48

seong-wooo reviewed Mar 30, 2023

View reviewed changes

jeongyuneo reviewed Mar 30, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[로또] 조윤호 제출합니다 #3

[로또] 조윤호 제출합니다 #3

ray-yhc commented Mar 30, 2023

seong-wooo left a comment

seong-wooo Mar 30, 2023

seong-wooo Mar 30, 2023

seong-wooo Mar 30, 2023

seong-wooo Mar 30, 2023

jeongyuneo left a comment

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

jeongyuneo Mar 30, 2023

		if (cost <=0 )
		throw new IllegalArgumentException("[ERROR] 1 이상의 양수를 입력해주세요");

		int cost = getNum(userInput);
		int lottoNum = PurchaseService.getLottoNum(cost);

		Lotto lotto = new Lotto(winningNumList);
		this.winningLotto = lotto;

	Lotto lotto = new Lotto(winningNumList);
	this.winningLotto = lotto;
	this.winningLotto = new Lotto(winningNumList);

[로또] 조윤호 제출합니다 #3

Are you sure you want to change the base?

[로또] 조윤호 제출합니다 #3

Conversation

ray-yhc commented Mar 30, 2023

seong-wooo 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

jeongyuneo 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