[로또 미션] 안정현 미션 제출합니다. #11

Anhye0n · 2024-05-06T14:45:24Z

안녕하세요! 동호님 시간 내어 코드리뷰해주셔서 감사합니다.

미션에서 포장 및 일급 컬렉션을 사용하라고 되어있었습니다.
해당 방법으로 코드를 작성하니 저의 이해도 부족으로 코드가 보기 불편할 정도로 너무 길어져서 원시 값은 최대한 줄이고, main에서 변수를 만들어 사용하는 방식으로 방향을 바꿨습니다.
그래서 GenerateLotto, WinningLotto이 2개의 클래스에서 각각 하나의 원시값만 사용하여 구현되었습니다. 뭔가 구현은 되었지만 이번 미션의 목적과는 다른 방법으로 미션을 완료하게된 것 같아, 추가적으로 공부하겠습니다.

저번 미션에서 스스로 부족하다 느꼈던 예외처리는 최대한 작성해보았습니다.

보너스 숫자 중복여부
구입 금액 음수 여부
당첨 번호 갯수
당첨 번호 중복 여부
수동 구매 시 중복 여부
수동 구매 갯수 음수 여부
수동 구매를 가격보다 많이 입력한 경우
enum 값 찾기
수동 입력 값 범위

예외 처리 부분을 이정도로 생각해봤는데 혹시 예외처리를 하면서 놓친 부분이나, 처리 방식에 대해 조언해주실 부분이 있다면 감사히 듣겠습니다.

PlusUltraCode · 2024-05-07T03:46:58Z

안녕하세요 정현님 코드 리뷰어 이동호입니다.

과제하느라 고생이 많으셨어요.

코드 리뷰 진행하겠습니다 ^^

PlusUltraCode · 2024-05-07T05:38:43Z

정현씨의 대부분의 예외처리를 해당 객체 안에서 예외처리 함수를 만들고 진행하셨습니다.

만약 예외처리가 한 두개가 아닌 많아지게 될 경우 해당 객채는 정말 해야 되는 일들이 있는데 예외 함수에 가려질 수 있다고
생각해요.

예외 처리 함수들을 따로 클래스 객체로 만들어서 사용하시는게 더 좋아 보입니다

백엔드 팀 리더분들의 코드를 살펴 봤는데 Record라는 것을 사용하여 예외처리를 진행한걸 봤습니다.

Anhye0n · 2024-05-07T09:35:39Z

@PlusUltraCode 감사합니다! 그 부분 참고하여 커밋해보겠습니다.

Anhye0n · 2024-05-21T04:28:06Z

동호님의 코드와 리더분들의 코드를 참고하여 최대한 record로 객체를 담아 에러처리를 하도록 해봤습니다.
처음의 코드가 record를 전혀 고려하지 않은 코드였다보니 record로 다시 수정하는 것이 살짝 어려웠던 것 같습니다.
근데 이제 한번 사용하여 보니 record class로 어떻게 객체를 담아 사용하여야할지 감이 잡힌 것 같습니다!
처음 사용하여 부족한 부분이 있을텐데 알려주시면 바로 반영하겠습니다!

PlusUltraCode · 2024-05-07T04:01:18Z

src/main/java/domain/GenerateLotto.java

+public class GenerateLotto {
+    ArrayList<ArrayList<Integer>> totalLotto = new ArrayList<>();
+


ArrayList<ArrayList> totalLotto 형태로 코드를 작성하셨습니다.
Lotto라는 객체를 하나 만들어서
ArrayList totalLotto 형태로 바꿔주신다면 코드를 읽는것도 수월하고 무엇보다 확장성 및 유지보수에
좋다라고 생각해요.

만약에 프로그램을 만들어논 상태에서 로또의 색깔을 추가해야 된다는 상황이 발생할 경우
Lotto객체를 만들지 않은 프로그램은 GenerateLotto에서 직접 함수를 추가해야되서
GenerateLotto의 역할이 너무 많아진다고 생각해요.
Lotto객체가 있으면 Lotto객체의 멤버 변수를 만들어서 그 부분만 추가할 수 있어 편리하다고 생각합니다.

PlusUltraCode · 2024-05-07T04:11:09Z

src/main/java/domain/GenerateLotto.java

+    private ArrayList<Integer> getOne() {
+        GenerateRandom random = new GenerateRandom();
+
+        Set<Integer> set = new HashSet<>();
+
+        while (set.size() != 6) {
+            set.add(random.generateRandom());
+        }
+        ArrayList<Integer> lotto = new ArrayList<>(set);
+
+
+        Collections.sort(lotto);
+
+        return lotto;
+    }
+


하나의 정수를 1~45번중 랜덤으로 가져와서 6개의 크기가 채워지면
lotto리스트에 넣는 함수로 보입니다.

로또의 번호가 중복이 될 수있어서 contains() 함수를 이용하여
로또 번호가 이미 존재하는지 안하는지를 판별하면 좋을거 같습니다

PlusUltraCode · 2024-05-07T04:15:00Z

src/main/java/domain/GenerateRandom.java

+package domain;
+
+import java.util.Random;
+
+public class GenerateRandom {
+    public int generateRandom() {
+        Random random = new Random();
+
+        return random.nextInt(45) + 1;
+    }
+
+}


따로 랜덤 객체를 만든건 너무 좋은 습관 같습니다.
이렇게 따로 랜덤 객체를 만들어 놓으면
랜덤 값을 받는 로또 객체를 Test할 때 더욱 쉽게 Test할수 있다고 생각해요

PlusUltraCode · 2024-05-07T04:22:03Z

src/main/java/domain/GenerateLotto.java

+    public ArrayList<Integer> getManualOne() {
+
+        Input input = new Input();
+
+        ArrayList<Integer> manualOne = input.setManualNumber();
+
+        for (int one: manualOne){
+            validateLottoRange(one);
+        }
+
+        if (manualOne.stream().distinct().count() != 6) {
+            throw new IllegalArgumentException("로또 번호 중복 입력은 불가능합니다.");
+        }
+
+        Collections.sort(manualOne);
+
+        return manualOne;
+    }


현재 Input객체가 domain에 있습니다.
MVC 구조에서 보시면
View 패키지 안에 Input과 Print 객체를 선언하는게 좋을거 같습니다.

또한 domain 객체 안에 Input()을 사용하여 Text를 입력받기 보다는
controller패키지 안에서 domain 과 view를 사용하여 입력받는것이 좋을거 같습니다.

PlusUltraCode · 2024-05-07T04:54:33Z

src/main/java/Main.java

+import domain.Input;
+import domain.GenerateLotto;
+import domain.WinningLotto;
+import view.Print;
+
+import java.util.ArrayList;
+
+public class Main {
+    public static void main(String[] args) {
+        GenerateLotto lotto = new GenerateLotto();
+        WinningLotto winningLotto = new WinningLotto();
+        Print print = new Print();
+        Input input = new Input();
+
+        int purchaseAmount = input.setPurchaseAmount();
+
+        int manualCount = input.setManualCount();
+
+        if (manualCount > 0) {
+            lotto.getManualLotto(purchaseAmount, manualCount);
+        }
+
+        ArrayList<ArrayList<Integer>> totalLottos = lotto.getLotto(lotto.getRemainingMoney(purchaseAmount, manualCount));
+
+        print.printPurchasedLottoCount(manualCount, totalLottos);
+
+        ArrayList<Integer> winningNumbers = input.setWinningNumber();
+
+        int bonusNumber = input.setBonusNumber(winningNumbers);
+
+        winningLotto.totalCheckLotto(totalLottos, winningNumbers, bonusNumber);
+
+        print.printWinningCount(winningLotto.winningCount, winningLotto.calculateRate(purchaseAmount));
+
+    }
+}


Main 이라는 객체는 최종 마무리 객체라고 생각해요

하나의 controller 객체를 만들어서 대부분의 내용을 그 객체에다 구현해놓고

Main이라는 곳에 마무리 정리 내용을 적는게 더 좋을거 같습니다

PlusUltraCode · 2024-05-07T05:19:01Z

src/main/java/view/Print.java

+    public void printWinningCount(ArrayList<Integer> winningCount, double rate) {
+        System.out.println("\n당첨 통계");
+        System.out.println("---------");
+
+        System.out.printf("3개 일치 (%d원)- %d개\n", WinningNumbers.findByIndex(0), winningCount.get(0));
+        System.out.printf("4개 일치 (%d원)- %d개\n", WinningNumbers.findByIndex(1), winningCount.get(1));
+        System.out.printf("5개 일치 (%d원)- %d개\n", WinningNumbers.findByIndex(2), winningCount.get(2));
+        System.out.printf("5개 일치, 보너스 볼 일치(%d원) - %d개\n", WinningNumbers.findByIndex(3), winningCount.get(3));
+        System.out.printf("6개 일치 (%d원)- %d개\n", WinningNumbers.findByIndex(4), winningCount.get(4));
+
+        System.out.printf("총 수익률은 %.2f입니다.\n", rate);
+
+    }


이 부분은 저도 뭐가 좋은지 잘 모르겠습니다.

for문을 이용하여 System.out.print()의 사용을 줄일지,
아니면 가독성을 위해 하나하나 전개할지는 아직도 고민이 있네요

PlusUltraCode · 2024-05-07T05:24:29Z

src/main/java/domain/WinningLotto.java

+    public double calculateRate(int money) {
+        return (double) (WinningNumbers.findByIndex(0) * winningCount.get(0) + WinningNumbers.findByIndex(1) * winningCount.get(1) + WinningNumbers.findByIndex(2) * winningCount.get(2) + WinningNumbers.findByIndex(3) * winningCount.get(3) + WinningNumbers.findByIndex(4) * winningCount.get(4)) / money;
+    }


이 부분은 for 문을 사용해서 계산하는게 더 좋을거 같습니다.

PlusUltraCode · 2024-05-07T05:30:42Z

src/main/java/domain/Input.java

+public class Input {
+


문제에서 예외상황을 정의해 주지 않아서 대부분의 경우의 수를 고려해야 될 거같습니다.

숫자가 아닌 문자열 형태나 공백을 입력 받았을 때 처리하는 예외도 있으면 좋을거 같아요

PlusUltraCode · 2024-05-07T05:34:00Z

src/main/java/view/Print.java

+
+}


주요 함수마다의 단위테스트가 있으면 더 좋을거 같습니다.

프로그램의 크기가 커짐에 따라 디버깅을 위해 매 프로그램을 재 실행 시키는데에는 많은 시간이 소요된다고 합니다.

각 하나의 함수마다 경우의 수를 고려해서 테스트 하게되면 이러한 부담을 줄일수 있다고 합니다.

PlusUltraCode · 2024-05-07T05:40:51Z

src/main/java/domain/Input.java

+    public int setPurchaseAmount() {
+        System.out.println("구입금액을 입력해주세요.");
+        Scanner scanner = new Scanner(System.in);
+
+        int money = scanner.nextInt();
+
+        if (validateNegativeNumber(money)) {
+            throw new IllegalArgumentException();
+        }
+
+        return money;
+    }
+
+    public ArrayList<Integer> setWinningNumber() {
+        System.out.println("\n지난 주 당첨 번호를 입력해 주세요.");
+        Scanner scanner = new Scanner(System.in);
+
+        String line = scanner.nextLine();
+
+        int count = separateNumber(line).size();
+
+        if (
+                validateNegativeNumber(count)
+                        ||
+                        validateLottoLength(count)
+                        ||
+                        separateNumber(line).stream().distinct().count() != 6
+        ) {
+            throw new IllegalArgumentException();
+        }
+
+        return separateNumber(line);
+    }
+
+    public int setBonusNumber(ArrayList<Integer> winningNumbers) {
+        System.out.println("\n보너스 볼을 입력해 주세요");
+        Scanner scanner = new Scanner(System.in);
+        int bonusNumber = scanner.nextInt();
+
+        winningNumbers.add(bonusNumber);
+
+        if (winningNumbers.stream().distinct().count() != 7) {
+            throw new IllegalArgumentException("보너스 숫자는 중복이 불가능합니다.");
+        }
+        return bonusNumber;
+    }
+
+    public int setManualCount() {
+        System.out.println("\n수동으로 구매할 로또 수를 입력해 주세요.");
+        Scanner scanner = new Scanner(System.in);
+
+        int count = scanner.nextInt();
+
+        if (validateNegativeNumber(count)) {
+            throw new IllegalArgumentException();
+        }
+        return count;
+
+    }
+
+    public ArrayList<Integer> setManualNumber() {
+        Scanner scanner = new Scanner(System.in);
+
+        String line = scanner.nextLine();
+
+        return separateNumber(line);
+    }
+
+    public ArrayList<Integer> separateNumber(String raw) {
+        String[] numberStrings = raw.split(", ");
+        ArrayList<Integer> numbers = new ArrayList<>();
+
+        for (String numberString : numberStrings) {
+            numbers.add(Integer.parseInt(numberString));
+        }
+
+        return numbers;
+    }
+
+    public boolean validateNegativeNumber(int number) {
+        return number < 0;
+    }
+
+    public boolean validateLottoLength(int number) {
+        return number > 6;
+    }
+
+}


전체적으로 함수 이름하고 변수 이름을 잘 작성하신거 같아요

읽는데 불편하지 않고 바로바로 의미를 알 수 있었습니다.

PlusUltraCode · 2024-05-22T04:07:02Z

src/main/java/domain/Input.java

+public record Input() {
+


Input 객체를 record라고 선언하신 이유를 알고싶어요 :)

PlusUltraCode · 2024-05-22T04:08:51Z

src/main/java/exception/BonusNumber.java

+public record BonusNumber(ArrayList<Integer> winningNumbers, int bonusNumber) {
+
+    public BonusNumber {
+        validateDuplication(winningNumbers);
+    }
+
+    private static void validateDuplication(ArrayList<Integer> winningNumbers) {
+        if (winningNumbers.stream().distinct().count() != 7) {
+            throw new IllegalArgumentException("숫자는 중복 불가능합니다.");
+        }
+    }
+}


record 를 이용하여 validate구문을 함께 사용해 예외 처리를 잘 진행 하신걸로 보여요 :)

PlusUltraCode · 2024-05-22T04:09:18Z

src/main/java/exception/Lotto.java

+
+    public Lotto {
+        validateLottoRange(lotto);
+        validateDuplication(lotto);
+        validateLottoLength(lotto);
+    }


:) 더 깔끔해 지셨어요!!

PlusUltraCode · 2024-05-22T04:10:20Z

src/main/java/exception/ManualNumber.java

+
+    public ManualNumber {
+        validateNegativeNumber(count);
+        checkRemainingMoney(money, count);
+    }
+
+    private void validateNegativeNumber(int number) {
+        if (number < 0) {
+            throw new IllegalArgumentException("음수는 입력될 수 없습니다.");
+        }
+    }
+    public void validateLottoRange(ArrayList<Integer> lotto) {
+        if (lotto.stream().allMatch(number -> number >= 1 && number <= 45)) {
+            throw new IllegalArgumentException("1 ~ 45사이의 값만 입력할 수 있습니다.");
+        }
+    }
+
+    public void checkRemainingMoney(int money, int manualCount) {
+        if (money < manualCount * LOTTO_PRICE) {
+            throw new IllegalArgumentException("구입 금액을 넘은 갯수 입니다.");
+        }
+    }
+}


validateLottoRange를 빼먹은거 같아요

PlusUltraCode · 2024-05-22T06:23:25Z

안녕하세요 정현씨 :)

record 관련해서 리펙토링을 진행 해주신걸 볼 수 있었습니다.

validate 를 관리하기 위해 record를 선언해주신걸로 보입니다.

다만 아쉬웠던 점은 Lotto record 파일을 이용하여 Lottos를 만들어 보는걸 추천드립니다.

Lotto라는 원시값을 이용하여 Lottos로 원시값 포장이라고 부릅니다 :)

Anhye0n added 3 commits May 6, 2024 21:17

step2

07464d3

step3

a5d3898

step4

5f2878c

feat: record 리팩토링

ac16599

refactor: 미사용 파일 제거

b8c0186

PlusUltraCode reviewed May 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[로또 미션] 안정현 미션 제출합니다. #11

[로또 미션] 안정현 미션 제출합니다. #11

Anhye0n commented May 6, 2024

PlusUltraCode commented May 7, 2024

PlusUltraCode commented May 7, 2024

Anhye0n commented May 7, 2024

Anhye0n commented May 21, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 7, 2024

PlusUltraCode May 22, 2024

PlusUltraCode May 22, 2024

PlusUltraCode May 22, 2024

PlusUltraCode May 22, 2024

PlusUltraCode commented May 22, 2024

		public class GenerateLotto {
		ArrayList<ArrayList<Integer>> totalLotto = new ArrayList<>();

[로또 미션] 안정현 미션 제출합니다. #11

Are you sure you want to change the base?

[로또 미션] 안정현 미션 제출합니다. #11

Conversation

Anhye0n commented May 6, 2024

PlusUltraCode commented May 7, 2024

PlusUltraCode commented May 7, 2024

Anhye0n commented May 7, 2024

Anhye0n commented May 21, 2024

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

PlusUltraCode commented May 22, 2024