Lotto 숙제 제출 by yenny07 · Pull Request #1 · zeepy-harvard/java-lotto

yenny07 · 2021-03-06T00:11:02Z

예외 처리도 못하고 보너스번호 중복 방지도 못하고 열거체도 하나뿐이고..
클래스가 이렇게 하나뿐이어도 되는지 의구심도 들고 아쉬운 게 많아요.. 피드백 주시면 더 고쳐보겠습니다..!!!!

- buyTickets() 구현 - makeTicket() 구현

- buyTickets()에서 일정 장수의 티켓을 생성하는 로직을 makeTickets()로 분리 - 기존 makeTicket() -> createNumbers()

- 차례대로 메소드를 호출하는 run() 작성

- inputWinningNumber() - inputBonusNumber()

- enum Rank - 맞춘 번호 개수를 구하는 matcher() - 당첨된 등수를 구하는 getRank()

- printResult()

KimGyeong

코멘트를 남기기엔 너무 큰 요구 사항들이 많아서 이렇게 따로 리스트를 정리해 적어드립니다.

1.테스트 코드 작성하기
2. Ticket Class 구현하기
3. 당첨 번호를 담당하는 Class 구현하기
4. Input, Output을 담당하는 Class 구현하기
5. 각 예외 발생 상황에 따른 예외처리 하기.

자바는 객체지향 프로그래밍 언어이고, 객체지향 프로그래밍을 지향해야 해요. 클래스를 나누는 것부터 시작을 해봅시다.
과제의 학습목표만을 따르지 말고, 이전의 학습 목표들과 더 넓게 학습 목표들을 잡아보세요. 막연하게 돌아가는 애플리케이션을 만드는 것이 목적이 아닌 숙제들이니까요.
수정한 후에 요청해주세요.

KimGyeong · 2021-03-06T05:19:42Z

src/main/java/lotto/LottoMachine.java

+        int ticketCount;
+        int bonusNumber;
+        List<List<Integer>> tickets;
+        List<Integer> winningNumber;
+        int[] hits;


초기화 하지 않은 값들은 위험해요! 만약 초기화가 되지 않은 값에 접근을 한다면 NullPointerException이 발생하겠죠? 아래에 초기화를 해주면서 선언을 해주면 좋을 것 같습니다.

KimGyeong · 2021-03-06T05:20:39Z

src/main/java/lotto/LottoMachine.java

+        String inputMoney = scanner.nextLine();
+        int ticketCount = Integer.parseInt(inputMoney) / 1000;


Scanner에는 Integer값을 받는 함수도 존재합니다! 공부를 해볼까요?

- Ticket, WinningNumber 테스트를 위한 생성자 추가

KimGyeong

저번 미션보다 많이 발전한 모습이 눈에 띱니다. 좋아지고 있어요!!
클래스의 책임과 메소드의 책임에 대해 생각해보면서 리팩토링을 진행하시면 좋을 것 같습니다.
이것과 더불어 코멘트들 확인해주세요. 수정한 후에 다시 리뷰 요청을 해주시면 되겠습니다.

KimGyeong · 2021-03-09T15:44:41Z

src/main/java/lotto/LottoMachine.java

+            hitCount = ticket.stream()
+                    .filter(winningNumber::contains)
+                    .count();


KimGyeong · 2021-03-09T16:04:05Z

src/main/java/lotto/LottoMachine.java

+    private Rank getRank(long hitCount, Ticket ticket, int bonusNumber) {
+        Rank rank = null;
+
+        switch ((int) hitCount) {
+            case 3:
+                rank = Rank.THREE;
+                break;
+            case 4:
+                rank = Rank.FOUR;
+                break;
+            case 5:
+                rank = (ticket.contains(bonusNumber)) ? Rank.BONUS : Rank.FIVE;
+                break;
+            case 6:
+                rank = Rank.SIX;
+                break;
+            default:
+                rank = null;
+        }
+
+        return rank;


Rank를 반환해주는 메소드네요. Rank에게 책임을 전가하면 어떨까요? Rank의 메소드로 만들면 좋을 것 같아요.

KimGyeong · 2021-03-09T16:05:56Z

src/main/java/lotto/Output.java

+        for (Rank rank : ranks) {
+            System.out.println(rank.getString() + hits[rank.ordinal()] + "개");
+            totalPrize += rank.getPrize() * hits[rank.ordinal()];
+        }


Output 클래스는 오직 출력만을 담당하는 클래스여야 해요. 이 로직은 다른 곳에서 수행하여 그 값들만 받아 출력하는 방법은 없을까요?

KimGyeong · 2021-03-09T16:07:31Z

src/main/java/lotto/Rank.java

+    public String getString() {
+        return hit + "개 일치 (" + prize + "원) - ";
+    }


get~의 메소드명은 일반적으로 getter 메소드로써 사용을 합니다. 필드의 값을 반환하기 위해서요.
여기서 사용하는 String, 즉 "개 일치 (" + prize + "원) - "는 Output 클래스에서 사용하는 것을 권장드립니다.

KimGyeong · 2021-03-09T16:09:04Z

src/main/java/lotto/WebUILottoApplication.java

+        }
+
+        int ticketCount = inputMoney / 1000;
+        System.out.println(ticketCount + "개를 구매했습니다.");


Output클래스가 있음에도 print메소드를 사용하네요. 어떻게 수정하면 main 클래스에서 직접적으로 System.out.println 메소드를 호출하지 않을 수 있을까요?

KimGyeong · 2021-03-09T16:11:06Z

src/main/java/lotto/WinningNumber.java

+public class WinningNumber extends ArrayList<Integer> {
+
+    private int bonusNumber;


상속은 조심해서 사용해야 합니다. 만약 WinningNumber가 Set이라는 자료구조로 변경 될 경우에, 이 클래스에서 사용하는 ArrayList의 메소드들을 모두 수정해줘야 하겠죠? 상속이 아닌 필드값을 List나 ArrayList로 변경하는 것은 어떨까요?

KimGyeong · 2021-03-09T16:12:20Z

src/main/java/lotto/LottoMachine.java

+            hitCount = ticket.stream()
+                    .filter(winningNumber::contains)
+                    .count();
+            if (hitCount >= 3) {


요구사항 중 하나인 indent(인덴트, 들여쓰기) depth를 2단계에서 1단계로 줄여보세요.가 지켜지지 않네요. 어떻게 수정하는 것이 좋을까요?

- 연관있었던 메소드들 수정

- printTicketCount(), printTickets() 작성

KimGyeong

이전보다 많이 좋아졌네요. 여기서 로또 번호들이 enum으로 구현된다면 어떨까요? 그렇게 된다면 1부터 45인 것에 대해 덜 신경써도 되겠죠? 현재 코드에서 만약 로또번호가 4~50으로 변경된다면 1부터 45인 것들을 모두 다 찾아서 고쳐야합니다. 변경 가능성과 확장 가능성을 모두 열어두고 그 상황에 어떻게 해야 더 수월한 작업이 이루어질 수 있을지 고민해봅시다.
수정할 사항들이 조금 더 있어요. 아래에 적어둘게요.

Input과 Output은 main 메소드에서만 사용하게 변경할 수 있을까요? 입력과 출력, 그리고 비즈니스 로직의 흐름을 main 메소드에서만 확인할 수 있게요.
예외 처리가 아직 많이 부족하네요. 본인이 생각하고 발견할 수 있는 예외 처리들을 모두 구현해보세요. 또한 그 케이스에 따른 테스트 코드들도 생기면 더욱 좋구요.

수정한 후에 리뷰 요청 다시 해주세요!

KimGyeong · 2021-03-10T16:00:49Z

src/main/java/lotto/Rank.java

+            case 4:
+                return Rank.FOUR;
+            case 5:
+                return (ticket.contains(bonusNumber)) ? Rank.BONUS : Rank.FIVE;


KimGyeong · 2021-03-10T16:01:28Z

src/main/java/lotto/LottoMachine.java

+    private void calculateHits(WinningNumber winningNumber, long hitCount, Ticket ticket) {
+        if (hitCount >= 3) {
+            Rank rank = Rank.getRank(hitCount, ticket, winningNumber.getBonusNumber());
+            hits[rank.ordinal()] += 1;


rank가 null이면 어떻게 되나요?

KimGyeong · 2021-03-11T01:44:37Z

src/main/java/lotto/Ticket.java

+
+    public Ticket() {
+        createNumbers();
+    }
+
+    public Ticket(ArrayList<Integer> testInput) {
+        this.addAll(testInput);
+    }
+
+    private void createNumbers() {
+
+        for (int i = 1; i <= 45; i++) {
+            this.add(i);
+        }
+
+        Collections.shuffle(this);
+        this.subList(6, 45).clear();
+        Collections.sort(this);
+    }


Suggested change

public Ticket() {

createNumbers();

}

public Ticket(ArrayList<Integer> testInput) {

this.addAll(testInput);

}

private void createNumbers() {

for (int i = 1; i <= 45; i++) {

this.add(i);

}

Collections.shuffle(this);

this.subList(6, 45).clear();

Collections.sort(this);

}

private List<Integer> ticketNumbers;

public Ticket() {

this.ticketNumbers = createNumbers();

}

public Ticket(ArrayList<Integer> testInput) {

this.ticketNumbers = testInput;

}

private List<Integer> createNumbers() {

List<Integer> numbers = new ArrayList<>();

for (int i = 1; i <= 45; i++) {

numbers.add(i);

}

Collections.shuffle(numbers);

numbers.subList(6, 45).clear();

return Collections.sort(numbers);

}

이런 형태이면 상속을 제거해도 되겠죠? 상속을 사용했을 때의 단점은 매우 강합니다.
예를 들어, List가 아닌 Set 자료구조를 사용하라는 요구사항이 발생했을 경우에는 변경해야 할 사항들과 고려해야 할 사항들이 많아지거든요.
상속에 대해 공부해보면 좋을 것 같아요.

KimGyeong

코멘트를 여러가지 남겼어요. 변수명에는 원시타입을 넣지 않는게 좋을 것 같아요. 참고할만한 자료를 같이 작성해줄게요. (참고자료)
예외에 따른 테스트도 추가하면 좋을 것 같아요. 예외 처리가 되는지 안되는지 확인할 수 있겠죠?
주요 로직에 있는 예외 처리에 대한 피드백은 다음에 남길게요.
수정하고 다시 리뷰 요청 해주세요!

KimGyeong · 2021-03-16T15:10:36Z

src/main/java/lotto/Input.java

+        Numbers[] lottoNum = Numbers.values();
+        List<Numbers> sixNumbers = new ArrayList<>();
+
+        for (String num : stringNumber) {
+            sixNumbers.add(lottoNum[Integer.parseInt(num)]);
+        }


Suggested change

Numbers[] lottoNum = Numbers.values();

List<Numbers> sixNumbers = new ArrayList<>();

for (String num : stringNumber) {

sixNumbers.add(lottoNum[Integer.parseInt(num)]);

}

List<Numbers> lottoNumbers = Arrays.stream(stringNumber)

.map(number -> Integer.parseInt(number))

.map(number -> Numbers.get(number))

.collect(Collections.toList());

스트림을 활용하면 여러 변수 선언을 줄일 수 있겠죠? 대신 Numbers 에서 메소드가 하나 필요하네요.
스트림을 사용해보는건 어때요?

mapToInt을 활용하는 방법도 있겠군요.

KimGyeong · 2021-03-16T15:12:30Z

src/main/java/lotto/Input.java

+        Numbers[] lottoNum = Numbers.values();
+        Numbers bonusNumber = lottoNum[intBonusNumber];


Numbers를 모두 가져온다면 사용하지 않는 친구들까지 모두 불러오는거겠죠? 메소드를 만들어 필요한 아이들만 꺼내오는건 어떨까요?

KimGyeong · 2021-03-16T15:13:02Z

src/main/java/lotto/Input.java

+        if (!(sixNumbers.stream()
+                .allMatch(new HashSet<>()::add))) {


Set을 활용하는 방법 좋네요! 👍

KimGyeong · 2021-03-16T15:18:28Z

src/main/java/lotto/WinningNumber.java

+
+    public WinningNumber() {
+        sixNumbers = new ArrayList<>();
+        bonusNumber = Numbers.ZERO;


당첨번호에 보너스 숫자가 있어야 하는데, 0으로 초기화를 하는게 옳을까요?

KimGyeong · 2021-03-16T15:18:32Z

src/main/java/lotto/Numbers.java

+package lotto;
+
+public enum Numbers {
+    ZERO(0),


이 상수는 어디서 사용하나요?

KimGyeong · 2021-03-16T15:19:48Z

src/main/java/lotto/Ticket.java

+        List<Integer> intNumbers = new ArrayList<>();
+        for (int i = 0; i < ticketNumbers.size(); i++) {
+            intNumbers.add(ticketNumbers.get(i).getNum());
+        }
+
+        return intNumbers;


이것도 stream으로 간결하게 할 수 있지 않을까요?

KimGyeong · 2021-03-16T15:20:21Z

src/main/java/lotto/WinningNumber.java

+    public void setSixNumbers() {
+        Input input = new Input();
+        sixNumbers = input.inputWinningNumber();
+    }
+
+    public void setBonusNumber() {
+        Input input = new Input();
+        bonusNumber = input.inputBonusNumber();
+
+        if (sixNumbers.contains(bonusNumber)) {
+            throw new IllegalArgumentException("보너스 번호가 이미 당첨 번호에 존재합니다.");
+        }
+    }


setter를 사용하지 않고 할 수 있는 방법은 없을까요?

KimGyeong

더 이상 수정할 것은 크게 보이지 않네요. 남긴 코멘트만 한번 더 생각해보면 좋을 것 같아요.
처음과 비교했을 때 더 좋은 코드로 보이나요? 본인의 생각을 가지고 본인만의 스타일을 고수해보는 것도 좋을 것 같아요. 고생했습니다.

KimGyeong · 2021-03-19T15:23:35Z

src/main/java/lotto/WinningNumber.java

+    private void inputSixNumbers() {
+        Input input = new Input();
+        String userInputNumbers = input.inputLine();
+        String[] commaRemoved = userInputNumbers.split(",");


입력이 1, 2 , 3이라면 ["1","2 "," 3"]과 같이 띄어쓰기도 같이 나뉘어지지 않나요?

KimGyeong · 2021-03-19T15:28:23Z

src/main/java/lotto/WinningNumber.java

+    private void inputSixNumbers() {
+        Input input = new Input();
+        String userInputNumbers = input.inputLine();
+        String[] commaRemoved = userInputNumbers.split(",");
+
+        List<Numbers> sixNumbers = Arrays.stream(commaRemoved)
+                .mapToInt(Integer::parseInt)
+                .mapToObj(Numbers::get)
+                .collect(Collectors.toList());
+
+        input.validateWinningNumber(sixNumbers);
+
+        this.sixNumbers = sixNumbers;
+    }
+
+    private void inputBonusNumber() {
+        Input input = new Input();
+        int userInputNumber = input.inputInt();
+        Numbers bonusNumber = Numbers.get(userInputNumber);
+
+        if (sixNumbers.contains(bonusNumber)) {
+            throw new IllegalArgumentException("보너스 번호가 이미 당첨 번호에 존재합니다.");
+        }
+
+        this.bonusNumber = bonusNumber;
+    }


입력을 받은 값을 바탕으로 WinningNumber 객체 생성을 하면 어떨까요? Input 클래스와 WinningNumber 클래스의 의존성을 제거할 수 있고, 필드값에 대한 예외처리를 클래스 메소드에서 안해도 되니까요.

yenny07 added 7 commits March 5, 2021 15:56

feat : 티켓 구매 기능 구현

6eda990

- buyTickets() 구현 - makeTicket() 구현

refactor : 메소드 분리

640fa82

- buyTickets()에서 일정 장수의 티켓을 생성하는 로직을 makeTickets()로 분리 - 기존 makeTicket() -> createNumbers()

refactor : 상위 메소드 run() 작성

6c03a6c

- 차례대로 메소드를 호출하는 run() 작성

feat : 당첨번호, 보너스번호 입력 기능 구현

3c0a5db

- inputWinningNumber() - inputBonusNumber()

feat : 번호 맞추기, 당첨 기능 구현

7af18e1

- enum Rank - 맞춘 번호 개수를 구하는 matcher() - 당첨된 등수를 구하는 getRank()

feat : 결과 출력 기능 구현

ee528e0

- printResult()

docs : 리드미 작성

e24e2e7

KimGyeong reviewed Mar 6, 2021

View reviewed changes

yenny07 added 8 commits March 8, 2021 14:46

refactor : Ticket 클래스 작성

8a1c42b

refactor : Ticket 클래스 수정

2beb458

refactor : Input 클래스 작성

87957f7

refactor : WinningNumber 클래스 작성

0ca8777

refactor : Output 클래스 작성

4ae009a

refactor : int[] hits를 인스턴스변수로 수정

cbc915b

refactor : 예외처리 로직 추가

39c04d2

test : 테스트코드 작성

77901d2

- Ticket, WinningNumber 테스트를 위한 생성자 추가

KimGyeong reviewed Mar 9, 2021

View reviewed changes

yenny07 added 6 commits March 10, 2021 19:22

refactor : 등수를 반환하는 로직 Rank로 이동

76ef1bf

refactor : Output 클래스에서 결과 출력을 위한 메소드 분리

0f06bb7

refactor : 수익률 계산 메소드, 출력 메소드 분리

c1e064b

refactor : 당첨 여부 계산 메소드 분리

3910c9b

refactor : WinningNumber 클래스에서 당첨번호를 필드로 변경

9fe225a

- 연관있었던 메소드들 수정

refactor : Output 클래스 외 print문 제거

7d691e2

- printTicketCount(), printTickets() 작성

KimGyeong reviewed Mar 11, 2021

View reviewed changes

yenny07 added 6 commits March 15, 2021 19:57

refactor : Ticket 클래스에서 티켓번호를 필드로 변경.

50c5237

refactor : 보너스번호 확인 메소드 작성

f66333c

refactor : 로또 번호 1~45를 열거체 Numbers로 구현

d038c2e

refactor : Input, Output은 메인에서만 사용하게끔 수정

13bd7d6

fix : 티켓 번호가 열거형 문자열로 보이는 것, 숫자 0이 포함되는 것 수정

6dff1e6

refactor : 당첨번호도 Input 클래스에서 처리하도록 수정

6074dad

yenny07 added 2 commits March 16, 2021 22:33

refactor : 입력에 대한 예외 처리 구현

eeb47c8

refactor : Rank에서 null 발생 시 예외 처리 구현

a850c49

KimGyeong reviewed Mar 16, 2021

View reviewed changes

yenny07 added 5 commits March 17, 2021 15:34

refactor : 당첨 번호 입력 메소드 수정

3fde30f

refactor : 열거형 상수 Numbers.ZERO 삭제

fc82762

refactor : List<Number> -> List<Integer> 변환 시 stream 활용

a4382b9

refactor : 당첨 번호의 setter 메소드 숨기기

cde9820

test : 예외 처리에 대한 테스트 코드 작성

0f2339a

KimGyeong approved these changes Mar 19, 2021

View reviewed changes

		String inputMoney = scanner.nextLine();
		int ticketCount = Integer.parseInt(inputMoney) / 1000;

		public class WinningNumber extends ArrayList<Integer> {

		private int bonusNumber;

		Numbers[] lottoNum = Numbers.values();
		Numbers bonusNumber = lottoNum[intBonusNumber];

		if (!(sixNumbers.stream()
		.allMatch(new HashSet<>()::add))) {

Conversation

yenny07 commented Mar 6, 2021

Uh oh!

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

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

KimGyeong left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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

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

2 participants

KimGyeong left a comment •

edited

Loading