test #132

boorownie · 2025-08-04T05:05:19Z

test

boorownie

로또 미션 Step 1의 구현을 축하드립니다! 🎉 구입 금액에 따라 로또를 발급하고 출력하는 기본적인 흐름을 잘 구현해주셨습니다. 특히 AutoLotto 클래스에서 TreeSet을 사용하여 중복 없이 로또 번호를 생성하고, ArrayList로 변환하는 과정은 번호 생성 로직을 잘 분리했다는 점에서 인상 깊었습니다.

다만, 앞으로의 학습을 위해 객체지향 설계 원칙에 대해 몇 가지 질문과 제안을 드리고 싶습니다. 각 단계별 요구사항을 충실히 반영하면서도, 더 견고하고 유연한 코드를 만들기 위한 고민을 함께 해보면 좋을 것 같습니다.

boorownie · 2025-08-04T05:26:38Z

src/main/java/controller/LottoController.java

+import java.util.List;
+
+public class LottoController {


InputView와 OutputView를 LottoController에서 직접 생성하고 필드로 가지고 있네요. 이렇게 되면 LottoController가 InputView, OutputView라는 특정 구현에 강하게 의존하게 됩니다. 만약 나중에 다른 방식으로 입출력을 처리하게 된다면 LottoController를 수정해야 할 수도 있습니다.

이런 경우, 의존성을 외부에서 주입하는 의존성 주입(Dependency Injection) 방식을 고려해보면 어떨까요? 예를 들어, LottoController의 생성자에서 InputView와 OutputView를 전달받도록 하면, LottoController는 입출력 방식을 몰라도 되니 더 유연해질 수 있습니다. 🤔

boorownie · 2025-08-04T05:26:38Z

src/main/java/controller/LottoController.java

+        viewLotto(price);
+    }
+
+    public int inputMoney(){


makeLottolist 메서드에서 lottoList를 new LottoList(makeLotto())로 계속 덮어쓰고 있네요. 현재 구현으로는 마지막에 생성된 Lotto 하나만 lottoList에 담기게 됩니다. count만큼의 로또를 제대로 담으려면 lottoList 객체를 메서드 시작 전에 한 번 생성하고, 그 객체에 Lotto를 추가하는 방식으로 수정해야 할 것 같습니다. 🧐

boorownie · 2025-08-04T05:26:38Z

src/main/java/controller/LottoController.java

+        int count = price/1000;
+        outputView.countPrint(count);
+        lottoList = makeLottolist(count);
+    }
+    public LottoList makeLottolist(int count){


makeLotto 메서드에서 AutoLotto 객체를 생성하고, getAutoLotto()로 번호를 가져온 후, 다시 new Lotto(lotto)로 Lotto 객체를 생성하고 있네요. AutoLotto는 이미 로또 번호를 생성하는 역할을 하고 있는데, Lotto 클래스는 그 생성된 번호를 단순히 감싸는 역할만 하고 있는 것 같습니다.

AutoLotto가 생성된 로또 번호를 직접 Lotto 객체로 만들어서 반환하는 것은 어떨까요? 이렇게 하면 makeLotto 메서드의 책임이 더 명확해질 수 있습니다. 예를 들어, AutoLotto 클래스에 createLotto()와 같은 메서드를 추가하여 Lotto 객체를 반환하도록 말이죠.

boorownie · 2025-08-04T05:26:38Z

src/main/java/model/AutoLotto.java

+    private static List<Integer> lottoNums = new ArrayList<>();
+    public AutoLotto(){
+        createAutoLotto();


lottoNums를 static으로 선언하고 createAutoLotto 메서드에서 초기화하고 있네요. static 변수는 여러 인스턴스가 공유하기 때문에, 여러 로또를 구매하는 상황에서 예상치 못한 동작을 유발할 수 있습니다. 예를 들어, AutoLotto 객체를 여러 번 생성하더라도 lottoNums는 계속 같은 리스트를 참조하게 되어, 각 로또가 동일한 번호를 가지게 될 수도 있습니다.

로또 번호는 각 로또 객체마다 독립적인 값을 가져야 하므로, static 대신 인스턴스 변수로 관리하는 것이 더 적절해 보입니다. 👍 createAutoLotto 메서드도 AutoLotto 객체가 생성될 때마다 독립적인 번호를 생성하도록 수정하면 좋을 것 같습니다.

boorownie · 2025-08-04T05:26:39Z

src/main/java/model/AutoLotto.java

+            int num = random.nextInt(MAX_LOTTO_NUMBER) + 1;
+            uniqueNumbers.add(num);
+        }
+
+        lottoNums = new ArrayList<>(uniqueNumbers);
+
+    }
+    public List<Integer> sortLotto(List<Integer> lottoNums){


createAutoLotto 메서드에서 TreeSet을 사용하여 고유한 번호를 생성한 후, lottoNums에 ArrayList로 변환하여 저장하고 있습니다. TreeSet은 이미 정렬된 상태를 유지하는데, lottoNums에 저장된 번호들은 getAutoLotto 메서드를 통해 그대로 반환되고 있습니다.

만약 로또 번호를 항상 정렬된 상태로 사용해야 한다면, TreeSet에서 바로 반환하거나 ArrayList로 변환할 때 정렬을 보장하는 방식을 고려해볼 수 있습니다. 현재 sortLotto 메서드가 있지만, getAutoLotto에서 반환되는 lottoNums가 이미 정렬된 상태라면 sortLotto를 별도로 호출하지 않아도 될 수도 있습니다. 이 부분에 대해 어떻게 생각하시나요? 🤔

boorownie · 2025-08-04T05:26:39Z

src/main/java/model/LottoList.java

+    List<Lotto> lottoList = new ArrayList<>();
+    private int totalPrice;
+
+    public LottoList(Lotto lotto){


LottoList 클래스가 Lotto 객체를 하나씩 추가하는 생성자를 가지고 있네요. 하지만 LottoList는 여러 개의 Lotto를 담는 역할을 해야 하는데, 현재 생성자는 Lotto 하나만 받아 lottoList에 추가하는 방식입니다.

LottoList의 생성자에서 List<Lotto> 전체를 받거나, Lotto 객체를 추가하는 addLotto(Lotto lotto)와 같은 메서드를 제공하여 lottoList를 관리하는 것이 더 일반적인 일급 컬렉션의 형태일 것 같습니다.

boorownie · 2025-08-04T05:26:39Z

src/main/java/model/LottoList.java

+        return lottoList;
+    }


totalPrice 필드에 setTotalPrice 메서드를 통해 값을 누적하고 있는데, 이 totalPrice가 무엇을 의미하는지 네이밍만으로는 명확하게 파악하기 어렵습니다. 🤔 예를 들어, 로또 구매 총액인지, 당첨금 총액인지 등 구체적인 의미를 알 수 있도록 네이밍을 좀 더 명확하게 하는 것이 좋겠습니다.

boorownie · 2025-08-04T05:26:39Z

src/main/java/view/InputView.java

+    public int inputPrice(){
+        System.out.println(INPUT_PRICE);
+        int price = scanner.nextInt();
+        return price;


scanner 필드를 static으로 선언하여 사용하고 있네요. Scanner는 시스템 자원을 사용하기 때문에, 여러 인스턴스에서 공유하는 것은 좋지만, 프로그램 종료 시 close() 메서드를 호출하여 자원을 해제해주는 것이 중요합니다. 현재 코드에서는 Scanner를 닫는 로직이 없어 프로그램 종료 시 자원 누수가 발생할 수 있습니다.

Application 클래스에서 main 메서드가 종료될 때 Scanner를 닫아주거나, InputView 클래스 자체에서 Scanner를 관리하고 종료 시 자동으로 닫히도록 설계하는 것을 고려해볼 수 있습니다.

boorownie · 2025-08-04T05:26:39Z

src/main/java/view/OutputView.java

+
+    }
+    public void lottoPrint(List<Integer> numbers) {


countPrint 메서드에서 구입한 로또 수를 출력하는 부분은 깔끔하게 잘 구현되었습니다. 👍

boorownie · 2025-08-04T05:26:39Z

src/main/java/view/OutputView.java

+        System.out.println(numbers);
+    }


lottoPrint 메서드에서 List<Integer>를 그대로 출력하고 있습니다. 이 부분은 Lotto 클래스에서 getNumbers()를 호출하여 얻은 결과일 텐데요, Lotto 클래스 자체에 toString() 메서드를 오버라이드하여 "[1, 2, 3, 4, 5, 6]"과 같은 형식으로 자체적으로 출력하도록 구현하는 것이 더 객체지향적인 설계일 수 있습니다. 이렇게 하면 OutputView는 Lotto 객체를 받아 그대로 출력하기만 하면 되니, 책임이 더 명확해질 것입니다.

gracefulBrown

테스트 리뷰입니다.

gracefulBrown · 2025-08-07T17:19:36Z

src/main/java/Application.java

@@ -0,0 +1,10 @@
+import controller.LottoController;


Pending 리뷰 테스트 코멘트

gracefulBrown

ㅇㅇㅇ

gracefulBrown

안녕하세요! 로또 미션 재리뷰 요청주셔서 감사합니다. 😊

먼저, 이전 리뷰에서 말씀드렸던 LottoList의 책임 분리, LottoList에 여러 개의 Lotto를 담는 방법, 그리고 AutoLotto의 static 필드 사용 등 여러 부분에 대해 깊이 고민하고 개선하려는 노력이 엿보입니다. 특히 Lotto 객체의 불변성을 위해 numbers 필드를 final로 선언한 점은 좋은 설계라고 생각합니다. 👍

이번 리뷰에서는 이전 지적 사항 중 일부가 아직 반영되지 않은 부분에 대해 다시 한번 질문을 드리고, 새롭게 추가된 Application 클래스와 LottoController의 역할에 대해 함께 이야기 나눠보고자 합니다. 객체지향 설계 원칙을 꾸준히 적용하며 성장해나가는 과정을 응원합니다!

gracefulBrown · 2025-08-07T17:43:08Z

src/main/java/controller/LottoController.java

+
+    public LottoController(){
+        lottoStart();
+    }


lottoStart() 메소드에서 inputMoney()와 viewLotto()를 호출하고 있습니다. lottoStart() 메소드는 로또 구매 과정을 시작하는 역할을 하는 것으로 이해됩니다. 혹시 이 메소드가 이 외에 다른 추가적인 책임을 가지고 있다면, 해당 부분을 분리하여 LottoController의 역할을 더욱 명확하게 하는 것을 고려해보면 어떨까요? 🤔

gracefulBrown · 2025-08-07T17:43:08Z

src/main/java/controller/LottoController.java

+    public void lottoStart() {
+        int price = inputMoney();
+        viewLotto(price);
+    }


viewLotto(int price) 메소드에서 price를 받아 count를 계산하고 makeLottolist(count)를 호출하고 있네요. makeLottolist 메소드 내부의 for 루프에서 lottoList를 계속해서 새로 생성하고 마지막 lottoList만 반환하는 방식이 이전 리뷰에서도 언급되었는데, 이번에도 동일하게 유지되고 있네요. 😅 LottoList는 여러 개의 Lotto 객체를 담는 컬렉션 역할을 해야 하므로, makeLottolist 메소드 상단에서 LottoList 인스턴스를 한 번만 생성하고, 루프 안에서는 해당 LottoList에 Lotto 객체를 추가하는 방식으로 변경하는 것이 LottoList의 역할을 더 명확하게 할 수 있을 것 같습니다. 예를 들어, LottoList lottoCollection = new LottoList();와 같이 생성한 후, 루프 안에서 lottoCollection.add(makeLotto());와 같이 사용하는 방식은 어떨까요?

gracefulBrown · 2025-08-07T17:43:08Z

src/main/java/controller/LottoController.java

+
+    public int inputMoney(){
+        return inputView.inputPrice();
+    }


makeLotto() 메소드에서 AutoLotto 객체를 생성하고 getAutoLotto()를 호출하여 번호를 얻은 후, 바로 outputView.lottoPrint(lotto);를 호출하고 있네요. AutoLotto 객체가 번호를 생성하는 로직을 이미 가지고 있다면, getAutoLotto()를 통해 얻은 번호 목록을 바로 Lotto 객체 생성에 활용하고, 실제 출력은 LottoController의 다른 메소드에서 담당하는 것이 책임 분리 측면에서 더 좋지 않을까 생각합니다. 🤔 makeLotto 메소드는 Lotto 객체를 생성하는 역할에만 집중하는 것은 어떨까요?

gracefulBrown · 2025-08-07T17:43:08Z

src/main/java/model/AutoLotto.java

+
+        // 로또 번호 생성
+        while (uniqueNumbers.size() < CNT_LOTTO_NUMBER) {
+            int num = random.nextInt(MAX_LOTTO_NUMBER) + 1;


lottoNums 필드가 static으로 선언되어 있고, 생성자에서 createAutoLotto()를 호출하여 이 static 필드를 갱신하고 있습니다. 이전 리뷰에서도 이 부분에 대해 말씀드렸었는데, 혹시 static 필드를 유지하기로 결정하신 특별한 이유가 있으신가요? 만약 여러 개의 AutoLotto 객체를 생성하게 된다면, static 필드는 모든 인스턴스가 공유하기 때문에 의도치 않은 동작이 발생할 수 있습니다. 각 AutoLotto 객체는 독립적인 로또 번호 목록을 가져야 할 것 같은데요. 이 lottoNums 필드를 인스턴스 변수로 변경하는 것을 다시 한번 고려해보시는 건 어떨까요? 이렇게 하면 각 AutoLotto 인스턴스가 자신만의 번호 목록을 가지게 되어 더욱 안정적인 코드가 될 것입니다.

gracefulBrown · 2025-08-07T17:43:08Z

src/main/java/model/AutoLotto.java

+
+        lottoNums = new ArrayList<>(uniqueNumbers);
+
+    }
+    public List<Integer> sortLotto(List<Integer> lottoNums){


createAutoLotto 메소드에서 TreeSet을 사용하여 중복을 제거하고 번호를 생성하는 것은 매우 좋은 접근입니다! 👍 TreeSet은 이미 정렬된 상태를 유지하는 자료구조인데, lottoNums에 TreeSet을 ArrayList로 변환하여 할당한 후, sortLotto 메소드에서 다시 정렬을 수행하고 있습니다. TreeSet의 특성을 고려했을 때, 이미 정렬된 TreeSet을 ArrayList로 변환하는 것만으로도 충분하지 않을까요? sortLotto 메소드의 역할이 TreeSet에서 얻은 정렬된 목록을 다시 정렬하는 것이라면, 이 부분에서 좀 더 효율적인 방법을 고민해볼 수 있을 것 같습니다.

gracefulBrown · 2025-08-07T17:43:09Z

src/main/java/model/Lotto.java

+
+public class Lotto {
+    private static final int BONUS_NUMBER_INDEX = 6;
+    private final List<Integer> numbers;


BONUS_NUMBER_INDEX 상수가 static final로 선언되어 있지만, 현재 Lotto 클래스 내부에서는 이 상수를 활용하는 로직이 보이지 않습니다. 이 상수가 Lotto 객체 자체의 번호와 직접적인 관련이 없다면, 다른 클래스로 이동시키거나 혹은 보너스 번호를 포함하는 로직이 있다면 그 의미를 명확히 하는 것이 좋겠습니다.

gracefulBrown · 2025-08-07T17:43:09Z

src/main/java/view/InputView.java

+    public int inputPrice(){
+        System.out.println(INPUT_PRICE);
+        int price = scanner.nextInt();
+        return price;


inputPrice() 메소드에서 scanner.nextInt()를 사용하여 금액을 입력받고 있습니다. 만약 사용자가 숫자가 아닌 값을 입력하는 경우 InputMismatchException이 발생할 수 있습니다. 이 예외를 처리하거나, 입력 값을 문자열로 받은 후 숫자로 변환하는 방식을 사용하면 좀 더 견고한 입력 처리가 가능할 것 같습니다.

gracefulBrown · 2025-08-07T17:43:09Z

src/main/java/view/OutputView.java

+        System.out.println("\n" + count + BUY_MESSAGE);
+
+    }
+    public void lottoPrint(List<Integer> numbers) {


lottoPrint 메소드에서 System.out.println(numbers);를 사용하여 List<Integer>를 직접 출력하고 있습니다. 실행 결과 예시를 보면 로또 번호 목록이 [8, 21, 23, 41, 42, 43]과 같이 출력되는 것을 볼 수 있는데, 이는 List의 기본 toString() 메소드 결과입니다. 사용자에게 좀 더 보기 좋게 번호를 출력하려면, Lotto 객체 내부의 번호 목록을 포맷에 맞게 변환하여 출력하는 로직을 추가하거나, Lotto 객체 자체를 출력하는 방식으로 변경하는 것을 고려해보면 좋을 것 같습니다.

gracefulBrown · 2025-08-07T17:43:09Z

src/main/java/model/LottoList.java

+    }
+    public List<Lotto> getLottoList(){
+        return lottoList;
+    }


LottoList 클래스에서 totalPrice 필드를 가지고 있으며, setTotalPrice와 getTotalPrice 메소드를 통해 총 가격을 관리하고 있습니다. LottoList가 로또 목록을 관리하는 역할과 더불어 구매 금액까지 관리하는 것은 책임이 다소 분산되는 느낌이 있습니다. LottoList는 단순히 Lotto 객체들의 컬렉션 역할에 집중하고, 총 가격 계산과 같은 로직은 별도의 클래스나 메소드에서 처리하는 것이 LottoList 클래스의 책임을 더 명확하게 분리하는 데 도움이 될 것 같습니다. 예를 들어, LottoPurchase와 같은 클래스에서 구매 금액과 로또 목록을 함께 관리하도록 설계하는 것을 고려해볼 수 있습니다.

gracefulBrown · 2025-08-07T17:43:09Z

src/main/java/model/LottoList.java

+
+import java.util.ArrayList;
+import java.util.List;
+


LottoList 클래스에서 lottoList 필드를 List<Lotto> lottoList = new ArrayList<>();로 선언하고, 생성자에서 Lotto lotto를 인자로 받아 lottoList.add(lotto);만 수행하고 있습니다. 이전 리뷰에서도 언급했듯이, LottoList가 여러 개의 Lotto 객체를 담는 역할을 하려면 생성자나 다른 메소드를 통해 Lotto 객체들을 추가할 수 있어야 합니다. 현재 LottoController에서 lottoList = new LottoList(makeLotto());와 같이 사용되는 방식이라면, LottoList가 단 하나의 Lotto 객체만 담게 될 가능성이 있습니다. 여러 개의 Lotto를 담을 수 있도록 LottoList 클래스를 개선해보는 것은 어떨까요? 예를 들어, addLotto(Lotto lotto)와 같은 메소드를 추가하는 것을 고려해볼 수 있습니다.

gracefulBrown

ss

gracefulBrown

새로운 코드를 보니 이전 리뷰 내용을 꼼꼼히 살펴보시고 개선하려는 노력이 엿보여 기쁩니다! 😊 특히 Lotto 객체의 numbers 필드를 final로 선언하여 불변성을 확보한 점, AutoLotto 클래스에서 TreeSet을 활용하여 중복을 제거하고 정렬된 번호를 생성하는 아이디어는 정말 훌륭합니다. 👍

다만, 몇 가지 부분에 대해 함께 고민해보면 더 좋을 것 같아요. 이전 리뷰에서 드렸던 몇 가지 피드백이 아직 반영되지 않은 것 같아 다시 한번 질문드리고, 새로운 코드에서도 몇 가지 개선할 점이 보여 함께 이야기 나눠보고 싶습니다.

gracefulBrown

새로운 코드를 보니 이전 리뷰 내용을 꼼꼼히 살펴보시고 개선하려는 노력이 엿보여 기쁩니다! 😊 특히 Lotto 객체의 numbers 필드를 final로 선언하여 불변성을 확보한 점, AutoLotto 클래스에서 TreeSet을 활용하여 중복을 제거하고 정렬된 번호를 생성하는 아이디어는 정말 훌륭합니다. 👍

다만, 몇 가지 부분에 대해 함께 고민해보면 더 좋을 것 같아요. 이전 리뷰에서 드렸던 몇 가지 피드백이 아직 반영되지 않은 것 같아 다시 한번 질문드리고, 새로운 코드에서도 몇 가지 개선할 점이 보여 함께 이야기 나눠보고 싶습니다.

gracefulBrown

ㅇ

nyeroni added 7 commits May 1, 2024 21:09

feat: 로또 구매할 금액 입력

958541e

feat: 구매한 로또 출력

8627586

feat: 로또 랜덤 번호로 생성

c086150

feat: 로또 생성

d435b8e

feat: 총 구매한 로또를 list로 생성

07b7bc7

feat: 구매할 가격 입력 후 해당 가격 만큼 로또 생성 및 출력

5ed6fea

feat: 실행

2fd7c31

boorownie commented Aug 4, 2025

View reviewed changes

gracefulBrown reviewed Aug 7, 2025

View reviewed changes

src/main/java/Application.java

@@ -0,0 +1,10 @@

import controller.LottoController;

Copy link

gracefulBrown Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending 리뷰 테스트 코멘트

gracefulBrown reviewed Aug 7, 2025

View reviewed changes

gracefulBrown approved these changes Aug 7, 2025

View reviewed changes

gracefulBrown reviewed Aug 7, 2025

View reviewed changes

test #132

Are you sure you want to change the base?

test #132

Uh oh!

Conversation

boorownie commented Aug 4, 2025

Uh oh!

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

gracefulBrown left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gracefulBrown left a comment

Choose a reason for hiding this comment

Uh oh!

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

gracefulBrown left a comment

Choose a reason for hiding this comment

Uh oh!

gracefulBrown left a comment

Choose a reason for hiding this comment

Uh oh!

gracefulBrown left a comment

Choose a reason for hiding this comment

Uh oh!

gracefulBrown left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!