[로또] 표현록 미션 제출합니다. by Regyung · Pull Request #1 · Java-JavaScript-Language-Stuty/java-lotto-7

Regyung · 2025-02-21T13:29:29Z

🎰 로또 💸

신경써서 구현한 부분

의존성 관리
각 함수의 크기 최소화

피드백이 필요한 부분

의존성에 관련해서 아직 많이 부족하기 때문에 이게 맞는지 스스로 잘 모르겠습니다.
쓸데없이 길거나 필요 없는 코드가 더 있는지 말해주세요.
코드 지적 적극 환영

고려한 예외

돈이 1000원 단위인가
당첨 번호와 보너스 번호가 숫자인가
당첨 번호와 보너스 번호가 1~45 사이인가
당첨 번호가 중복 없이 6개인가
보너스 번호가 당첨 번호와 중복 없이 1개인가

- 돈 입력 - 당첨 번호 입력 - 보너스 번호 입력 - 발행된 로또 번호 출력 - 당첨 통계 출력

- 돈이 1000원 단위인가 - 당첨 번호와 보너스 번호가 숫자인가 - 당첨 번호와 보너스 번호가 1~45 사이인가 - 당첨 번호가 중복없이 6개인가 - 보너스 번호가 당첨 번호와 중복 없이 1개인가

- LottoService : Controller 대신 간단한 데이터 가공 로직 구현 - Calculator : 로또 구매 장수와 수익률 계산

daeGULLL

몇몇 보완점이 보이지만 아주 잘 짜신 코드같습니다!! 리뷰하던 도중 깔끔한 서비스 코드를 보고 제가 코딩 중에 컨트롤러에 모델을 노출시켜 코드짠걸 뒤늦게 기억해낼수 있었습니다..ㅋㅋ 덕분에 후딱 수정하러 갑니다 마지막 주차 같이 힘내용~~

daeGULLL · 2025-02-27T11:55:16Z

개인적인 취향이긴 한데, messageConstant나 prizeConstant 전부 상품에 관련된 enum이니 이 둘을 한개의 enum으로 합치셔도 좋을 것 같습니다! 상품 관련 메세지는 enum내부 로직으로 생성하게 하셔도 될것 같아유

daeGULLL · 2025-02-27T11:58:37Z

+public class LottoService {
+    private LottoData lottoData;
+
+    public void init(List<List<Integer>> l, List<Integer> n, int bn) {


가독성을 위해 파라미터명을 축약어보단 풀네임으로 쓰는게 더 좋을 것 같습니다!

동의합니다!

daeGULLL · 2025-02-27T12:05:48Z

+    public static int checkMoney(String input) {
+        try {
+            double money = Integer.parseInt(input);
+            if (!isInteger(money/1000)) { throw new IllegalArgumentException(); }


여기 illegalArgumentException에 내용 적는 것을 까먹으신 것 같습니다!!

daeGULLL · 2025-02-27T12:06:28Z

+    }
+
+    public void printStats(int[] results, double profit) {
+        System.out.println("당첨 통계\n---");


이런 게임 진행 메세지도 errorConstants처럼 따로 enum으로 빼셔도 좋을 것 같아요!

동의합니다!

seulnan

너무 고생하셨습니다!

정말 말그대로 의존성관리에 신경을 쓰신게 보이는 코드라 저도 배운게 많았습니다:)

다만, 전체적으로 몇가지 제안하고싶은점은

단위테스트코드를 작성해보시는 건 어때용?저는 이번미션부터 TDD도입을 시도하고있어요..!어렵긴하지만..연습해보면 좋을것같아용
중복코드가 있어보여요! 한번 코드흐름을 깔끔하게 정리해보시고 안쓰는 코드는 삭제하는것도 도움될듯합니당

seulnan · 2025-02-28T11:44:13Z

+        printResult();
+    }
+
+    private void initialize() {


혹시 outputView.newLine()이 너무 반복적으로 호출되는데 한번만 호출하는 방식으로 변경하거나 아니면 아예 출력순서를 담당하는 별도 메서드로 분리하는게 더 좋을 것 같아요!

seulnan · 2025-02-28T11:44:48Z

+    }
+
+    public void printStats(int[] results, double profit) {
+        System.out.println("당첨 통계\n---");


동의합니다!

seulnan · 2025-02-28T11:45:12Z

+        for (int i = 0; i < results.length; i++) {
+            System.out.println(m[i].getMessage() + " - " + results[i] + "개");
+        }
+        System.out.println("총 수익률은 " + String.format("%.1f", profit) + "%입니다.");


printf를 활용하면 더 코드가 간단해질 것 같아요ㅎㅎ

seulnan · 2025-02-28T11:48:18Z

    }

-    // TODO: 추가 기능 구현
+    public List<Integer> getNumbers() {


LottoData.java에서도 똑같은 getNumbers가 있던데 LottoData만 service에서 사용하고있더라구요 여기에 있는 lotto의 중복코드를 삭제해도 되지않을까요??

seulnan · 2025-02-28T11:55:06Z

+        data.lottos.addAll(l.stream().map(Lotto::new).toList());
+        data.numbers.addAll(n);
+        data.bonusNumber = bn;
+        data.numbers.add(data.bonusNumber);


numebrs에서 보너스번호와 당첨번호를 둘다 관리하는데 따로 관리하는게 명확한 데이터구조를 유지하고 확장성면에서 더 좋지 않을까요??

seulnan · 2025-02-28T11:56:36Z

+    public void matchNumber() {
+        List<Integer> intersectionList;
+        for (Lotto l : lottoData.getLottos()) {
+            intersectionList = l.getNumbers().stream().filter(n -> lottoData.getNumbers().contains(n)).toList();
+            if (intersectionList.size() < 3) continue;
+            if (intersectionList.size() == 6 && !isContainBonus(intersectionList)) {
+                lottoData.addPrize(4);
+                continue;
+            }
+            lottoData.addPrize(intersectionList.size() - 3);
+        }
+    }
+


Suggested change

public void matchNumber() {

List<Integer> intersectionList;

for (Lotto l : lottoData.getLottos()) {

intersectionList = l.getNumbers().stream().filter(n -> lottoData.getNumbers().contains(n)).toList();

if (intersectionList.size() < 3) continue;

if (intersectionList.size() == 6 && !isContainBonus(intersectionList)) {

lottoData.addPrize(4);

continue;

}

lottoData.addPrize(intersectionList.size() - 3);

}

}

public void matchNumber() {

for (Lotto lotto : lottoData.getLottos()) {

int matchCount = (int) lotto.getNumbers().stream()

.filter(lottoData.getNumbers()::contains)

.count();

if (matchCount >= 3) {

int index = (matchCount == 6 && !isContainBonus(lotto.getNumbers())) ? 4 : matchCount - 3;

lottoData.addPrize(index);

}

}

}

여기 if문에서 .size()를 계속 중복호출하고있는것같아요 matchCount라는 변수에 담아서 사용하는것은 어떨까요??

seulnan · 2025-02-28T11:58:13Z

+
+    private final int value;
+
+    PrizeConstants(String value) {


첨부터 int로 선언하면 string value를 굳이 int로 변환하지않아도 되지않을까요??

Regyung added 10 commits February 19, 2025 22:21

docs(README): 과제 정리 및 구현 함수 정리

54beff8

feat(view): Input과 Output View 구현

ed7a2f2

- 돈 입력 - 당첨 번호 입력 - 보너스 번호 입력 - 발행된 로또 번호 출력 - 당첨 통계 출력

feat(utils): Error, Message, Prize 상수 Enum 추가

b6dddba

feat(utils): 로또 난수 생성 기능 구현

2bba416

feat(model): 데이터 저장을 위한 객체 구현

9d75b1c

feat(controller): 주요 동작 로직 구현

395711a

- LottoService : Controller 대신 간단한 데이터 가공 로직 구현 - Calculator : 로또 구매 장수와 수익률 계산

feat(config): 의존성 관리를 위한 AppConfig 구현

04cb204

test: 간단한 테스트 실행

a8d1311

docs(README): 구현 함수 수정 및 재정리

77e403d

daeGULLL added a commit to 2025JavaGroupStudy/java-lotto-7 that referenced this pull request Feb 27, 2025

feat(view): 입출력 뷰 Java-JavaScript-Language-Stuty#1

1da0dd3

daeGULLL reviewed Feb 27, 2025

View reviewed changes

seulnan reviewed Feb 28, 2025

View reviewed changes

Uh oh!

Conversation

Regyung commented Feb 21, 2025

🎰 로또 💸

신경써서 구현한 부분

피드백이 필요한 부분

고려한 예외

Uh oh!

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

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants