[유안] 로또 미션 제출합니다. #128

KimSeongGyu1 · 2020-02-20T08:11:38Z

No description provided.

예외조건 처리 구현

6개 숫자가 적절히 들어오지 않을 때 예외처리 구현

* 지난 커밋 때 add하지 않은 Ticket 클래스 추가 *

6개의 숫자를 체크하는 추상 클래스 구현 후 Ticket과 WinningNumbers에서 상속

Ticket 객체와 WinningNumbers 객체의 생성자에 적용

Dto 반환하는 팩토리 interface 기반 구현

무한 인풋 받기 제거

로또 번호 enum대신 싱글톤 패턴 사용 포장 안된 객체들 포장 출력형태 책임 view로 이동

잊고 add 안한 파일들 add

jeonghoon1107

안녕하세요 유안!
몇가지 피드백을 남겼으니 확인부탁드려요 :)

jeonghoon1107 · 2020-02-23T04:50:50Z

src/main/java/domain/lottonumber/LottoNumber.java

+        return NumberCache.cache.stream()
+                .filter(a -> a.getValue() == number)
+                .findAny()
+                .orElse(NumberCache.dummyNumber);


-1이 나와도 되는 것인가요?

당첨번호객체(WinnningNumbers)에서 6개의 숫자와 보너스 번호는 겹치면 안되는데,
DTO에서도 데이터 검증의 책임이 어느정도 있을 수 있다는 생각에 당첨번호 객체에서 가져야 할 검증 책임을 DTO로 떠넘기면서 발생한 문제인 것 같습니다.

6개의 번호 정보만 넘길 DTO를 만들 때도 DTO는 당첨번호객체의 책임을 가지고 있어서
6개의 번호와 보너스번호는 겹치면 안되었는데,
절대 기존의 번호와 겹치지 않을 번호를 생각하느라 더미 번호를 -1로 지정해주었습니다.

다시 생각해보니 무리하게 당첨번호객체의 검증 책임을 DTO로 넘기는 것이 맞지 않다고 생각되어
더미 번호를 제거하고 검증책임을 다시 돌려놓았습니다.

jeonghoon1107 · 2020-02-23T04:55:11Z

src/main/java/LottoApplication.java

+            return InputView.enterMoney();
+        } catch (IllegalArgumentException e) {
+            System.out.println(e.getMessage());
+            throw new IllegalArgumentException();


추적하기 쉽게 예외를 처리할때는 항상 메시지를 같이 넘겨주세요!

메시지가 없는 새로운 에러를 던지는 대신
자기 자신을 다시 던져서 메시지가 유지되도록 해봤습니다.

jeonghoon1107 · 2020-02-23T05:24:00Z

src/main/java/domain/result/LottoRank.java

+                .findFirst()
+                .orElse(NO_WIN);
+
+        if (lottoRank == SECOND && !hasBonus) {


보너스 여부도 하나의 속성으로 포함하면 어떨까요?

보너스 여부를 속성으로 가지지 않아도 동작에 문제가 없어서 빼보았습니다.

보너스 여부를 속성으로 가진다면

코드를 읽을 때 좀 더 명시성이 높아지는 점,

혹시 다른 당첨정보에도 보너스관련 정보가 추가된다면 처리에 도움이 될 수도 있는 점
의 장점이 있을 것으로 생각되는데
특히 명시성이 중요할 것 같아 넣는 것이 맞다고 생각해 다시 추가했습니다.

혹시 제가 잘못 이해했다면 다시 코멘트 부탁드립니다.

jeonghoon1107 · 2020-02-23T05:31:00Z

src/main/java/domain/LottoStore.java

+        return lottoTickets;
+    }
+
+    public static List<LottoTicket> generateTickets(int number, List<Set<Integer>> myNumbers) {


다음 단계를 미리 진행하신 것 같네요 👍
이번 단계에서는 해당 부분에 신경쓰지 않고 객체의 역할을 중점으로 하시면 좋을 것 같아요.
다음 단계에서는 해당 부분을 인터페이스로 추출하면 불필요한 클래스나 부분들이 줄어들 것 같아요!

말씀하신대로 다음단계를 고려하고 만들었습니다.
다음단계에서 인터페이스를 바탕으로 확장할 수 있도록 수정하고, 불필요한 부분은 삭제했습니다.

jeonghoon1107 · 2020-02-23T05:40:00Z

src/main/java/domain/lottonumber/LottoNumbers.java

+    public int findNumberOfMatchingNumbers(LottoNumbers comparingNumber) {
+        return this.lottoNumbers.stream()
+                .filter(comparingNumber::contains)
+                .mapToInt(number -> 1)


count를 활용해보는 것은 어떨까요?

long을 반환하는 것이 마음에 들지 않아 이렇게 했는데
다시보니 count로 센 다음 int로 변환 하는 것이 더 좋아보여 수정했습니다.

jeonghoon1107 · 2020-02-23T05:46:55Z

src/main/java/LottoApplication.java

+public class LottoApplication {
+
+    public static void main(String[] args) {
+        User user = new User();


전체적인 흐름이 User라는 객체를 기반으로 흘러가는데 User 객체가 반드시 필요한가요?

현실에 기반해서 생각하다 보니 User객체를 만들었습니다.

반드시 필요한 객체도 아니고 객체지향적 사고를 연습하는데 방해가 될 수도 있을 것 같아
삭제했습니다.

로또 번호에 더미 값 삭제 에러 메시지 추적 가능하도록 변경 로또 등수 enum에 보너스 여부 추가 로또상점객체 불필요한 메서드 삭제 및 interface기반 확장성 추가 유저 객체 삭제

KimSeongGyu1 · 2020-02-23T08:52:56Z

리뷰 내용 반영하여 코드 수정했습니다.
다시 검토 부탁드립니다.

이전 수정 과정에서 잊고 적절히 수정하지 못한 점 수정

jeonghoon1107

안녕하세요 유안!
간단한 피드백 남겼으니 다음 단계 진행하면서 수정해주세요 :)

jeonghoon1107 · 2020-02-25T11:30:23Z

src/main/java/LottoApplication.java

+
+    public static void main(String[] args) {
+        Money money = enterMoney();
+        List<LottoTicket> randomTickets = new RandomLottoStore().generateTickets(money);


RandomLottoStore는 인스턴스를 생성해야하는 것인가요?

Store은 static method만을 가지고,
Strategy interface를 인자로 받는 것이 더 좋을 것 같습니다.

jeonghoon1107 · 2020-02-25T11:35:51Z

src/main/java/LottoApplication.java

+    private static Money enterMoney() {
+        try {
+            return InputView.enterMoney();
+        } catch (IllegalArgumentException e) {


모든 코드에 print를 하고 다시 해당 예외를 throw하고 있는데
예외를 잡았으면 특정한 행위를 하면 좋을 것 같아요.
아니면 적절한 메시지를 담아 예외를 던지면 어떨까요?

해당부분은 재귀를 통해 동작 제어를 하던 흔적인데
아에 삭제하는 것이 더 좋아보입니다.

jeonghoon1107 · 2020-02-25T11:47:56Z

src/main/java/domain/lottonumber/LottoTicket.java

+
+    private final LottoNumbers lottoNumbers;
+
+    public LottoTicket(LottoNumbersDto lottoNumbersDto) {


dto의 역할은 무엇인가요?
하나의 dto를 다른 두 개의 객체에서 사용하고 해당 dto는 두 객체가 각자 필요한 것들이 다 모여있는 것 같은데 dto가 꼭 필요한가요?

dto는 도메인과 view의 정보 전달을 당담하고 있습니다.
꼭 필요하지는 않지만 테스트하기 편한 구조가 되는 점, 도메인으로 넘어오는 데이터의 유효성이 어느정도 보장 되는 점 등에서 장점이 있다고 생각합니다.
두 객체에 공통되는 부분이 있어 하나의 dto를 같이 사용하도록 했는데, 각 객체별로 dto를 따로 가지고 있는 것이 잘못된 혼용을 막을 수 있을 것 같습니다.

KimSeongGyu1 added 22 commits February 18, 2020 15:26

[docs] README 작성

83f6878

[feature] 로또 숫자 객체 비교 기능 구현

263b9af

[feature] 로또 숫자 생성 예외처리 기능 구현

2395c33

[feature] 돈 객체 생성자 구현

97e02d3

예외조건 처리 구현

[feature] InputView 구입금액 입력 구현

f26dcb6

[feature] 티켓 객체 구현

088692b

6개 숫자가 적절히 들어오지 않을 때 예외처리 구현

[refactor] LottoNumber 클래스 enum type으로 수정

1071d9a

* 지난 커밋 때 add하지 않은 Ticket 클래스 추가 *

[feature] 당첨 번호 객체 구현

3b46dbf

6개의 숫자를 체크하는 추상 클래스 구현 후 Ticket과 WinningNumbers에서 상속

[refactor] LottoNumbersDto 적용 및 관련 로직 수정

25b425c

Ticket 객체와 WinningNumbers 객체의 생성자에 적용

[feature] 티켓과 당첨 번호 중복 확인 구현

62f3572

[feature] 랜덤 팩토리 구현

34361ab

Dto 반환하는 팩토리 interface 기반 구현

[feature] 고정값으로 당첨번호 생성 구현

9518660

[feature] InputView 당첨번호와 보너스볼 입력 구현

44404dc

[feature] OutputView 티켓 구매 출력 구현

164be4e

[feature] 로또 당첨 결과 찾기 메서드 구현

50ebf5a

[refactor / feature] 구조 리팩토링 및 로또 당첨 결과 구현

89b3bd8

[refactor] 리펙토링

efc5d5a

[refactor] 리펙토링, 수익률 에러 수정

9b9f778

[refactor] 리펙토링

dc876fc

[refactor] 리펙토링

c2410ef

무한 인풋 받기 제거

[refactor] 리펙토링

9ac451f

로또 번호 enum대신 싱글톤 패턴 사용 포장 안된 객체들 포장 출력형태 책임 view로 이동

[refactor] 리펙토링

d820406

잊고 add 안한 파일들 add

jeonghoon1107 suggested changes Feb 23, 2020

View reviewed changes

[refactor] 코드 리뷰 반영 리펙토링

3293322

로또 번호에 더미 값 삭제 에러 메시지 추적 가능하도록 변경 로또 등수 enum에 보너스 여부 추가 로또상점객체 불필요한 메서드 삭제 및 interface기반 확장성 추가 유저 객체 삭제

[refactor] 로또 번호 에러 관련 로직 수정

71ba931

이전 수정 과정에서 잊고 적절히 수정하지 못한 점 수정

jeonghoon1107 approved these changes Feb 25, 2020

View reviewed changes

jeonghoon1107 merged commit 21f73bc into woowacourse:kimseonggyu1 Feb 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[유안] 로또 미션 제출합니다. #128

[유안] 로또 미션 제출합니다. #128

KimSeongGyu1 commented Feb 20, 2020

jeonghoon1107 left a comment

jeonghoon1107 Feb 23, 2020

KimSeongGyu1 Feb 23, 2020

jeonghoon1107 Feb 23, 2020

KimSeongGyu1 Feb 23, 2020

jeonghoon1107 Feb 23, 2020

KimSeongGyu1 Feb 23, 2020

jeonghoon1107 Feb 23, 2020

KimSeongGyu1 Feb 23, 2020

jeonghoon1107 Feb 23, 2020

KimSeongGyu1 Feb 23, 2020

jeonghoon1107 Feb 23, 2020

KimSeongGyu1 Feb 23, 2020

KimSeongGyu1 commented Feb 23, 2020

jeonghoon1107 left a comment

jeonghoon1107 Feb 25, 2020

KimSeongGyu1 Feb 25, 2020

jeonghoon1107 Feb 25, 2020

KimSeongGyu1 Feb 26, 2020

jeonghoon1107 Feb 25, 2020

KimSeongGyu1 Feb 26, 2020


		private final LottoNumbers lottoNumbers;

		public LottoTicket(LottoNumbersDto lottoNumbersDto) {

[유안] 로또 미션 제출합니다. #128

[유안] 로또 미션 제출합니다. #128

Conversation

KimSeongGyu1 commented Feb 20, 2020

jeonghoon1107 left a 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

KimSeongGyu1 commented Feb 23, 2020

jeonghoon1107 left a 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