[LBP] 윤준석 로또 미션 2-5단계 제출합니다. #89

junseok0304 · 2025-03-02T14:59:16Z

스터디 정보

스터디명: LBP
리뷰이: 윤준석
리뷰어: 이지윤

❓ 질문

현재 작성된 코드 중, MVC 패턴 준수를 위해 수정되어야 할 부분들이 있는지 궁금합니다.
더 간결하거나 개선할만 한 부분이 있다면 알려주시면 감사하겠습니다!

< 코드상 구조 >

LottoConstants (Config)
LottoValidator (Config)
WinningRank (Config)

LottoMachineController (Controller)

LottoMachine (Model)
Lotto (Model)
LottoResultChecker (Model)

InputView (View)
ResultView (View)
Application

commit suggest Co-authored-by: Ji_Yun <[email protected]>

# Conflicts: # src/main/java/model/Lotto.java

dd-jiyun

안녕하세요 준석 👋

이번 미션 갑자기 난이도가 올라가서 많이 어려웠을 거 같아요!
그래도 MVC를 준수하기 위해 많이 노력하신 것 같습니다 고생하셨어요 👍

질문주신 부분에 대해서는 리뷰 코멘트로 남겼습니다!

요구사항인 일급컬렉션과 원시 값 포장도 학습하시고 적용해보시면 좋을 것 같아요 😉

추가적으로 궁금하신 점이 있다면 편하게 질문해주세요.

dd-jiyun · 2025-03-06T03:50:46Z

src/main/java/config/LottoConstants.java

@@ -0,0 +1,18 @@
+package config;
+
+public enum LottoConstants {


Enum 활용 👍🏻

감사합니다!

dd-jiyun · 2025-03-06T04:00:59Z

src/main/java/config/LottoValidator.java

+
+import java.util.*;
+
+public class LottoValidator {


LottoConstants를 정의했지만 Validator에서 사용하지 않은 이유가 있을까요?

현재 Validator 클래스가 유틸리티 성격을 띠고 있지만, Lotto와 LottoMachine에서도 자체적으로 검증을 하고 있어서 중복되는 코드가 많아 보여요 🤔
도메인 계층에서 검증을 수행하고 있는데, Controller에서 한 번 더 검증하는 이유가 있을까요?

도메인에서 검증하는 방식과 Validator 클래스를 사용하는 방식 각각의 장단점을 비교해 보면 좋을 것 같아요. 그 후에 리팩토링 방향을 고민해보시면 어떨까요?

예외메시지 출력을 위해서였지만
중복되는 코드가 생기는 이슈가 있었네요,
validator에서 사전 입력 검증을 하도록 하고
도메인 내부에서 검증 로직을 메서드로 정리해서 재사용하는 방식도 괜찮을 것 같습니다!

dd-jiyun · 2025-03-06T04:13:52Z

src/main/java/config/LottoValidator.java

+        List<Integer> numbers = Arrays.stream(input.split(","))
+                .map(String::trim)
+                .map(Integer::parseInt)
+                .toList();


현재 메서드의 길이가 요구사항(10라인 제한)을 초과한 것 같아요. 검증 로직이 추가되면 더 길어질 가능성도 있어 보이네요 🤔

이 메서드에서는 입력을 나누는 역할과 검증하는 역할을 함께 하고 있는데, 혹시 입력을 나누는 로직이 다른 곳에서도 필요할 가능성이 있을까요? 만약 그렇다면 별도 메서드로 분리하면 어떤 장점이 있을까요?

입력값을 나누는 로직과 검증하는 로직을 분리하면 재사용성을 재고할 수 있고
테스트 코드 작성시에도 이점이 있을 것 같습니다!

dd-jiyun · 2025-03-06T04:47:14Z

src/main/java/model/LottoMachine.java

+import java.util.stream.IntStream;
+
+public class LottoMachine {
+    private static final int LOTTO_PRICE = LottoConstants.LOTTO_PRICE.getValue();


이미 LottoConstants에서 값을 불러올 수 있는데, 상수를 한 번 더 감싼 이유가 있을까요?

이렇게 하면 상수의 계층이 하나 더 늘어나서 오히려 가독성이 떨어질 수도 있을 것 같아요. 준석님은 어떻게 생각하세요?

그렇네요ㅜㅜ LottoConstants에서 값을 불러와서 사용하는 방식이 나을 것 같습니다!
가독성 측면에서도 이미 정의된 상수를 사용하는 편이 나을 것 같습니다,

dd-jiyun · 2025-03-06T04:53:08Z

src/main/java/model/LottoMachine.java

+        return new Lotto(shuffledNumbers.subList(0, LottoConstants.LOTTO_SIZE.getValue()));
+    }
+
+    public static List<Lotto> generateLottos(List<List<Integer>> manualNumbers, int purchaseAmount) {


현재 메서드가 수동 로또 생성, 자동 로또 생성을 동시에 처리하고 있는 거 같아요. 각각 역할을 분리하면 가독성도 높아지고 한 가지 역할에 집중할 수 있을 것 같아요!

말씀해주신대로, 수동 로또 생성과 자동 로또 생성을 분리해서 SRP를 준수하고
generateLotto에서는 수동로또생성 메서드와 자동로또생성 메서드를
호출하는 방식으로 바꾸면 좋을 것 같습니다! 감사합니다.

dd-jiyun · 2025-03-06T05:32:18Z

src/main/java/model/LottoResultChecker.java

+    }
+
+    private static int updateResults(Map<String, Integer> result, Lotto lotto, Set<Integer> winningSet, int bonusNumber) {
+        int matchCount = (int) lotto.getNumbers().stream().filter(winningSet::contains).count();


stream 사용 👍🏻 체이닝 방식을 쓰면 더 가독성 높아질 것 같아요!

스트림 체이닝에 대해서 공부해보겠습니다!
코드길이 단축과 중간변수를 선언하지 않아도 되어서 가독성과 유지보수에 이점이 있다고 하는군요,

dd-jiyun · 2025-03-06T05:47:46Z

src/main/java/config/WinningRank.java

+    }
+
+    public static Optional<WinningRank> findByMatchCount(int matchCount, boolean hasBonus) {
+        if (matchCount == 5 && hasBonus) {


이와 같이 하드 코딩된 조건문을 사용하는 것보다, Enum이 스스로 결정하고 검증할 수 있도록 하는 건 어떨까요?
예를 들면, isMatching과 같은 메서드를 만들어서 자신과 주어진 값을 일치하는지 확인하는 방법으로요!

이 방법에 대한 준석님의 의견이 궁금합니다!

Enum에서 스스로 상태를 결정해준다는 것에 대해서 생각해보지 못했는데,
그런 방법도 있군요!
규칙을 한 곳에서 관리해주기에도 유리할 것 같습니다 감사합니다.

dd-jiyun · 2025-03-06T05:53:31Z

src/main/java/controller/LottoMachineController.java

+        while (true) {
+            try {
+                purchaseAmount = InputView.getPurchaseAmount();
+                LottoValidator.validatePurchaseAmount(purchaseAmount);
+                break;
+            } catch (IllegalArgumentException e) {
+                ResultView.printInvalidAmountMessage();
+            }
+        }


재입력을 받는 접근은 좋아요 👍🏻
하지만 요구사항에 indent depth가 1로 제한하고 있기 때문에, 재입력보다는 예외 처리를 하면 더 간결해질 것 같습니다!

do while문 사용에 익숙치 않아서 위와 같은 방법으로 작성한 것 같습니다!
지윤님 말씀대로, 예외 발생시 반복입력을 요청하도록 하는 방법도 괜찮은 것 같습니다.

dd-jiyun · 2025-03-06T05:54:39Z

src/main/java/controller/LottoMachineController.java

+import java.util.stream.Collectors;
+
+public class LottoMachineController {
+    public void run() {


run() 메서드가 다양한 책임을 가지고 있어요.
흐름을 표현한 것은 좋지만, 메서드 분리가 되어있지 않으니 가독성이 떨어지는 것 같아요. 각 역할에 맞게 메서드를 분리하면 좋을 것 같습니다.

말씀해주신대로, 런에서 모두 처리하는 것 보다 메서드를 분리해서 호출해서 사용하는편이 더 나은 것 같습니다!

dd-jiyun · 2025-03-06T05:58:46Z

src/main/java/view/InputView.java

+        List<String> manualNumbers = new ArrayList<>();
+        for (int i = 0; i < count; i++) {
+            System.out.printf("로또 번호 %d/%d 입력: ", i + 1, count);
+            manualNumbers.add(scanner.nextLine().trim());
+        }
+        return manualNumbers;


InputView에서 입력한 값을 저장하는 역할까지 맡는 것이 적절할까요?

인풋뷰의 역할은 입력받기에 집중해야하는데, SRP를 준수하지 못한 것 같습니다.
해당 역할은 컨트롤러에서 처리하면 될 것 같습니다.

junseok0304 and others added 6 commits February 19, 2025 23:11

Feat: 구현한 코드를 커밋

c87fff7

Update src/main/java/model/Lotto.java

a701b7b

commit suggest Co-authored-by: Ji_Yun <[email protected]>

Feat: 1단계 미션 보완 커밋

cbcbfb5

Merge remote-tracking branch 'origin/main'

6f38163

# Conflicts: # src/main/java/model/Lotto.java

Fix: 괄호 열기 전 띄어쓰기 수정

0ab362a

Feat: 2,3,4단계 로또 미션 수행

e45c0d3

junseok0304 changed the base branch from main to junseok0304 March 2, 2025 14:59

junseok0304 added 4 commits March 3, 2025 00:01

Fix: 불필요한 주석 삭제 및 개행 삭제

8346e52

Fix: 삼항연산자 삭제, 우승자랭크 enum 사용, else문 삭제

08f759e

Fix: 삼항연산자 삭제, LottoValidator 추가, InputView, ResultView 로직 개선

be25676

Fix: WinningRank 로직 개선

b40189e

junseok0304 changed the title ~~[LBP] 윤준석 로또 미션 2-4단계 제출합니다.~~ [LBP] 윤준석 로또 미션 2-5단계 제출합니다. Mar 4, 2025

dd-jiyun suggested changes Mar 6, 2025

View reviewed changes

Merge branch 'junseok0304' into main

b54e70f

junseok0304 requested a review from dd-jiyun March 8, 2025 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LBP] 윤준석 로또 미션 2-5단계 제출합니다. #89

[LBP] 윤준석 로또 미션 2-5단계 제출합니다. #89

junseok0304 commented Mar 2, 2025 •

edited

Loading

dd-jiyun left a comment •

edited

Loading

dd-jiyun Mar 6, 2025

junseok0304 Mar 7, 2025

dd-jiyun Mar 6, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

dd-jiyun Mar 6, 2025

junseok0304 Mar 8, 2025

[LBP] 윤준석 로또 미션 2-5단계 제출합니다. #89

Are you sure you want to change the base?

[LBP] 윤준석 로또 미션 2-5단계 제출합니다. #89

Conversation

junseok0304 commented Mar 2, 2025 • edited Loading

스터디 정보

❓ 질문

< 코드상 구조 >

dd-jiyun left a comment • edited Loading

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

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

junseok0304 commented Mar 2, 2025 •

edited

Loading

dd-jiyun left a comment •

edited

Loading