[로또 main2] 조윤호 제출합니다 by ray-yhc · Pull Request #4 · SoftwareMaestro-Backend-Study/java-lotto

ray-yhc · 2023-04-03T09:01:53Z

main1 리뷰해주신 내용들 반영 + main2 요구사항 적용했습니다!

Bonus 클래스 추가하면서 좀 허점이 보이지만...!! 오늘은 여기까지인 것 같아요😭 일단 제출해봅니당
이펙티브 자바 읽기 시작하면서, 정적 팩토리 메소드, 빌더 등등을 적용시켜 보았는데, 아낌없이 조언 남겨주시면 감사하겠습니다!

great-park · 2023-04-05T16:46:15Z

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

+                    p.isPrized() &&
+                            p.getMatchNum() <= matches &&
+                            p.getGrade() <= prize.getGrade()
+            ) {


41~43 라인을 하나의 메소드로 뽑으면 가독성도 좋아지고, 메소드 이름으로 어떤 조건인지 표현 가능할 것 같습니다!

great-park · 2023-04-05T16:56:09Z

src/main/java/lotto/lotto/lottoCreator/RandomLottoCreator.java

+
+    @Override
+    public Lotto getLotto(int min, int max, int num){
+        List<Integer> numbers = Randoms.pickUniqueNumbersInRange(min,max,num);


stream() 사용하시면 22라인까지 1줄로 표현할 수 있을 것 같아요

great-park · 2023-04-05T16:59:59Z

src/main/java/lotto/view/UserOutput.java

+    private UserOutput() {
+    }
+
+    public static void printLottos(LottoOutputModel model) {


System.out.printf 사용하시면 15~~19 라인을 1줄로 줄일 수 있을 것 같고, 20~~22 라인도 stream().map 사용하시면 더욱 가독성 있게 작성할 수 있을 것 같습니다

jeongyuneo

반복문 사용의 경우 Stream API 사용이 가능한지 고려해보시고, 반복문 대신 Stream API를 사용해보시면 좋을 것 같아요!

jeongyuneo · 2023-04-06T05:50:30Z

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

+        private void validateBonusNumber(List<Integer> numbers, int num){
+            if (numbers.contains(num))
+                throw new IllegalArgumentException("[ERROR] 보너스 번호와 당첨 번호에 중복이 있습니다.");
+        }


사용되지 않는 메소드는 삭제해주세요!

jeongyuneo · 2023-04-06T06:26:05Z

src/main/java/lotto/lotto/lottoCreator/LottoCreator.java

@@ -0,0 +1,8 @@
+package lotto.lotto.lottoCreator;
+
+import camp.nextstep.edu.missionutils.Randoms;


안쓰는 import는 정리해주세요!

jeongyuneo · 2023-04-06T06:27:45Z

src/main/java/lotto/lotto/lottoCreator/LottoCreator.java

+import lotto.lotto.Lotto;
+
+public interface LottoCreator {
+    public Lotto getLotto(int min, int max, int num);


인터페이스는 컴파일 시 자동으로 접근제한자를 public으로 인식하기 때문에 접근제한자는 생략하셔도 무방합니다!

jeongyuneo · 2023-04-06T06:42:52Z

src/main/java/lotto/lotto/lottoCreator/ManualLottoCreator.java

+    private void validateNumRange(int min, int max) {
+        for (int n : numbers)
+            if (n < min || n > max)
+                throw new IllegalArgumentException("[ERROR] 올바른 범위 내의 숫자를 입력해주세요.");
+    }


로또 숫자의 의미를 가장 잘 표현할 수 있는 타입이 정수형일까요?
로또 숫자와 같이 단순 정수가 아닌 의미있는 값은 객체로 관리하는 것이 좋다고 생각합니다!

jeongyuneo · 2023-04-06T06:45:03Z

src/main/java/lotto/lotto/lottoCreator/RandomLottoCreator.java

+import java.util.List;
+
+public class RandomLottoCreator implements LottoCreator{
+    private static final LottoCreator instance = new RandomLottoCreator();


상수 네이밍은 대문자와 언더바(_)로 구성해주세요!
ex)

private static final LottoCreator INSTANCE = new RandomLottoCreator(); private static final LottoCreator LOTTO_CREATOR = new RandomLottoCreator();

jeongyuneo · 2023-04-06T07:28:37Z

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

+        int commas = input.length() - input.replace(String.valueOf(","), "").length();
+        if (commas + 1 != strList.size())
+            throw new IllegalArgumentException("[ERROR] 정수와 ,로 이루어진 배열을 입력해주세요.");


구분자에 대한 검증같은데, 예외 메시지가 어색한 것 같습니다.
그리고 구분자로 사용되는 쉼표(,)는 상수화하면 구분자가 바뀌었을 때 예외 메시지와 replace()에서 사용되는 구분자를 실수없이 수정할 수 있을 것 같아요!

jeongyuneo · 2023-04-06T10:43:01Z

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

+        UserOutput.printLottos(
+                new LottoOutputModel(
+                        lottoNum - manualNum, manualNum, bundle.getLottosNumList()
+                ));


출력을 위한 값들을 담고 있는 객체가 정말 필요할까요?
로또 게임을 여러번 진행한다고 하면, LottoOutputModel이 계속 생성될텐데 객체 생성시 초기화된 값들을 그대로 꺼내서 사용하는 행위는 불필요해보입니다!

jeongyuneo · 2023-04-06T10:51:04Z

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

+        List<LottoCreator> creators = Stream.generate(RandomLottoCreator::from)
+                .limit(lottoNum - manualNum)
+                .collect(Collectors.toList());


RandomLottoCreator가 2개 이상 생성되어야 할까요?
한 번 생성된 RandomLottoCreator에서 getLotto()만 새롭게 호출해도 랜덤으로 생성된 로또를 발급할 수 있을 것 같아요.

jeongyuneo · 2023-04-06T10:53:36Z

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

+        }
+    }
+
+    public List<Integer> getNumList(String input) {


해당 메소드에 책임이 너무 많은 것 같아요. 메소드의 책임이 최소화되도록 하면 좋을 것 같아요!

jeongyuneo · 2023-04-06T13:39:43Z

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

@@ -0,0 +1,35 @@
+package lotto.purchase;
+
+public class PurchaseService {


PurchaseService는 유틸성 클래스로 사용하는 것보다 객체로 생성하는게 더 좋지 않을까요?

저도 같은 생각했네요.
앞으로 유틸 클래스 금지..

seong-wooo

전반적으로 느낀점

새로운 지식을 습득하고 적용해보신 것은 굉장히 좋습니다!
하지만 아직은 투머치한거같아요.
일단 객체지향적으로 설계하는 것에 더 집중해봅시다!
코드를 짤 때, 변경에 유연하게 대처할 수 있는가? 를 우선으로 생각하면서 짜야할 것 같아요.

그리고 VO (값 객체)란 무엇인가에 대해서 알아보면 더 좋은 설계를 하실 수 있을 것 같아요

seong-wooo · 2023-04-06T14:03:27Z

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

@@ -0,0 +1,35 @@
+package lotto.purchase;
+
+public class PurchaseService {


저도 같은 생각했네요.
앞으로 유틸 클래스 금지..

seong-wooo · 2023-04-06T14:14:52Z

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

+        this.bonus = builder.bonus;
+    }
+
+    public static class Builder {


빌더 패턴을 사용하신 이유가 궁금해요!

ray-yhc added 15 commits April 3, 2023 11:15

feat: copy code from yoonhocho branch

fe17173

feat: add lotto creators

66761c9

feat: add output models

85ecc2d

feat: add main

a4f2913

feat: add purchasing manual lotto

54b297e

feat: use EnumMap for prize, status

2d8b6a1

feat: add creation manual&auto lotto

c8a3902

feat: edit printLottos

9852c03

feat: edit printLottos

b909ad2

feat: delete deprecated methods

dffd69c

chore: edit appearences

72f238d

feat: edit lotto validations

e4ee5d6

feat: edit purchase validations

6710f7a

feat: edit winning lotto creation process, use builder pattern

55db99a

feat: add Bonus object

1e073f7

great-park approved these changes Apr 6, 2023

View reviewed changes

jeongyuneo reviewed Apr 6, 2023

View reviewed changes

seong-wooo reviewed Apr 6, 2023

View reviewed changes

		@@ -0,0 +1,8 @@
		package lotto.lotto.lottoCreator;

		import camp.nextstep.edu.missionutils.Randoms;

		@@ -0,0 +1,35 @@
		package lotto.purchase;

		public class PurchaseService {

Conversation

ray-yhc commented Apr 3, 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!

jeongyuneo 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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants