[로또 미션] 김명지 미션 제출합니다. #34

Starlight258 · 2024-06-23T10:02:17Z

안녕하세요 진영님 !
로또 4, 5단계 미션을 완료했습니다 😁

✅ 구현된 코드에 대한 설명은 README.md에서 보실 수 있습니다 !

리뷰 부탁드립니다 !! 감사합니다~ 🙇‍♀️

- 도메인과 밀접한 상수는 도메인에서 관리하도록 이동한다. - 초기화는 기존 값을 초기값으로 변경하는 의미를 가지므로, 특정 값으로 객체를 생성한다는 의미에 맞춰 메서드명을 변경한다.

- 매칭되지 않은 경우를 이미 filter에서 확인하고 있으므로 그 외의 경우 예외를 발생시키도록 수정한다(orElseThrow()).

- BiPredicate를 사용하여 매칭 여부를 판단하도록 작성한다. - enum 객체 내에 함수를 정의하여 캡슐화한다.

- 클래스 내부에서 사용하는 메서드는 private으로 변경한다.

- WinningRank의 findByMatchCountAndBonus 메서드는 호출하는 곳에서 key값만 사용하므로 반환값을 key 값으로 수정한다.

- 로또 수동 구매 기능 지원을 위한 inputView, outputView 메서드 추가, 도메인 메서드 호출 - Lottos: generateLottosByPrice 메서드명 변경 price는 lottoCount를 구하는데에 사용되므로 price가 아닌 lottoCount를 인자로 넘긴다.

- 로또 개수를 나타내는 LottoCount 값 객체를 생성하여 원시형 int를 포장하고 로또 개수에 대한 로직을 모은다.

- inputView는 입력 받는 역할에 집중하고, controller에서 inputView와 outputView를 모두 의존하도록 조정한다.

- count가 0일 경우 기존 price가 0원이 되므로 더한 다음 곱하는 것이 아닌 곱한 가격을 더하도록 수정한다.

- 인자명 수정 - 테스트 코드 검증값 올바르게 수정 - 테스트 설명 수정

- List에서 Set으로 변경하면 내부 원소들의 중복 여부를 확인하지 않아도 된다.

- 로또 당첨 통계를 더 잘 나타내는 이름으로 변경

- 입력받은 컬렉션을 외부에서 조작할 수 있으므로 방어적 복사를 수행하도록 변경한다. - getter는 컬렉션을 외부에 노출하므로 변경 가능성을 제거하기 위해 불변 컬렉션으로 감싸서 반환한다.

jinyoungchoi95

안녕하세요 명지님 😃

몇가지 코멘트남겨두었으니 확인부탁드려요.

감사합니다🙇‍♂️

jinyoungchoi95 · 2024-06-28T14:24:26Z

src/main/java/org/duckstudy/model/Price.java

@@ -13,16 +14,16 @@ public class Price {

    private final int value;

-    public Price(int price) {
+    public Price(final int price) {


파라미터들에 final이 달리게된 이유가 있을까요?

입력값이 객체라면 참조가 변경되는 것을 막고, 기본 값이라면 값 자체가 변경되는 것을 막아서 실수를 방지할 수 있기 때문입니다.

jinyoungchoi95 · 2024-06-28T14:41:41Z

src/main/java/org/duckstudy/model/lotto/LottoCount.java

+    public void validateManualLottoCount(final int totalLottoCount) {
+        if (count > totalLottoCount) {
+            throw new IllegalArgumentException("수동으로 구매할 로또 수가 전체 로또 수를 초과합니다.\n");
+        }
+    }


LottoCount manualLottoCount = new LottoCount(inputView.inputManualLottoCount()); manualLottoCount.validateManualLottoCount(lottoCount.getCount());

에서 사용되고 있는 것으로 보이는데 파라미터를 int가 아닌 LottoCount를 받아야 getter를 통한 도메인로직이 줄어들 것 같아요.

맞습니다! 메서드 파라미터 타입을 변경하는게 좋겠네요! 메서드 리팩토링시 호출하는 함수까지 확인해보겠습니다! 감사합니다~

jinyoungchoi95 · 2024-06-28T14:43:09Z

src/main/java/org/duckstudy/model/lotto/Lottos.java

-    public Lottos(List<Lotto> lottos) {
-        this.lottos = Collections.unmodifiableList(lottos);
+    public Lottos(final List<Lotto> lottos) {
+        this.lottos = new ArrayList<>(lottos);


Collections.unmodifiableList(lottos);에서 변경된 이유가 있을까요?

입력받은 lottos는 참조값이기 때문에, 외부에서 수정되면 Lottos 객체의 내부 상태가 영향을 받기 때문입니다.
그래서 생성자에서는 방어적 복사를 해주었습니다.
그리고 getter로 외부에서 lottos를 참조할때는 unmodifiableList()를 이용해서 불변 컬렉션을 반환하도록 했습니다.

jinyoungchoi95 · 2024-06-28T14:45:47Z

src/main/java/org/duckstudy/model/lotto/constant/WinningRank.java

+    NONE(0, 0, 0, (matchCount, matchBonus) -> matchCount < 3),
+    FIFTH(3, 5_000, 3, (matchCount, matchBonus) -> matchCount == 3),
+    FOURTH(4, 50_000, 4, (matchCount, matchBonus) -> matchCount == 4),
+    THIRD(5, 1_500_000, 5, (matchCount, matchBonus) -> matchCount == 5 && !matchBonus),
+    SECOND(5, 30_000_000, -5, (matchCount, matchBonus) -> matchCount == 5 && matchBonus),
+    FIRST(6, 2_000_000_000, 6, (matchCount, matchBonus) -> matchCount == 6);


의미상으로는 NONE은 matchCount가 0, 1, 2에요. 정확히 도메인의 의미를 전달해주려면 List로 들고있어야하지 않을까요?

맞습니다! 도메인의 범위를 다 다루지 않고 있었네요! 수정하겠습니다~

jinyoungchoi95 · 2024-06-28T14:46:07Z

src/main/java/org/duckstudy/model/lotto/constant/WinningRank.java

+    FIFTH(3, 5_000, 3, (matchCount, matchBonus) -> matchCount == 3),
+    FOURTH(4, 50_000, 4, (matchCount, matchBonus) -> matchCount == 4),
+    THIRD(5, 1_500_000, 5, (matchCount, matchBonus) -> matchCount == 5 && !matchBonus),
+    SECOND(5, 30_000_000, -5, (matchCount, matchBonus) -> matchCount == 5 && matchBonus),


-5가 key로써 어떻게 의미가 전달되고 있을까요?

Map<WinningRank, Integer> statistics;에 당첨 횟수를 key로 저장할때
2등과 3등이 로또 당첨 횟수가 같아서 음수를 붙여준건데, 의미상으로는 명확하지 않네요..

key를 임의의 숫자로 두어서 WinningRank에서 관리하지 않고, map의 key 자체를 WinningRank로 두도록 수정하겠습니다!

private final Map<WinningRank, Integer> statistics;

그렇게하면 의미상으로도 명확할 것 같아요 ! 감사합니다 !

jinyoungchoi95 · 2024-06-28T14:47:06Z

src/main/java/org/duckstudy/model/lotto/LottoStatistics.java

+import java.util.stream.Stream;
+import org.duckstudy.model.lotto.constant.WinningRank;
+
+public class LottoStatistics {


통계보단 결과가 맞지않나요?

통계라고 하기에는 역부족해보이네요 🤔 수정하겠습니다 😀

jinyoungchoi95 · 2024-06-28T14:47:58Z

src/main/java/org/duckstudy/model/lotto/Lotto.java

-
-    public List<LottoNumber> getLotto() {
-        return lotto;
+    public Set<LottoNumber> getLotto() {


출력시 로또번호의 순서는 어떻게 보장할수있을까요?

앗 그렇네요 Set는 순서가 보장되지 않네요!
출력시에 sorted() 추가해서 수정했습니다 !

jinyoungchoi95 · 2024-06-28T15:05:20Z

src/main/java/org/duckstudy/controller/LottoController.java

(반영하지 않으셔도 됩니다.) 엄밀히 따지면 Controller가 아니며, MVC에서의 Conroller와도 달라요.

일단 MVC에서의 Controller를 먼저 이야기하면 Controller는 View를 들고 실제로 랜더링하는 역할은 아니에요. 단순한 콘솔게임이기 때문에 MVC 패턴의 복잡한 정의를 사용하여 구현할 필요는 없어요.

그래서 MVC 패턴을 제거하고보면 Controller라는 네이밍은 클래스와는 어울리지 않아요. Controller는 영문법의 문법상 게임을 컨트롤하는 조이스틱과 같은 의미에요. 조이스틱이 화면을 가지고 게임을 실행하고 출력하는건 맞지 않는거죠. 조이스틱이 컴퓨터나 엔진이 아니니까요.

디자인 패턴은 정의된 것을 그대로 상황에 끼워 사용하는 것이 아닌 특정한 상황에 해결책으로 나온 것이기 때문에 개인적으로 패턴종속적인 학습이나 개발을 하고 계시다면 개념 자체에 매몰될 수 있어요. 향후에도 개발하실때는 네이밍을 너무 패턴 종속적으로 보고 잡고있지는 않는가를 계속 고민하면서 개발하면 도움이 될 것 같아요.

그렇네요..! Controller라고 하기엔 잘못된 입력을 반복적으로 받는 로직도 있고, 비즈니스 로직도 호출하고 여러가지를 하고 있네요.
MVC 패턴으로 먼저 구조를 잡고 개발하다보니 콘솔 애플리케이션 구현에는 적절하지 않은 설계가 된 것 같아요.
앞으로도 너무 틀에 갇혀서 개발하지 않고 시야를 넓히는 식으로 공부하겠습니다. 감사합니다 !! 😃

- 호출하는 함수에서 모두 getter를 이용해 값으로 접근하기 때문에 메서드 파라미터 타입을 LottoCount로 변경하는 것이 좋다.

- WinningRank를 key로 사용하여 LottoStatistics에 저장하면, WinningRank에서 key를 별도로 관리할 필요가 없어지고 의미상 명확하다.

- 콘솔 애플리케이션 구현에 맞게 적절한 패키지명을 부여한다.

Starlight258 · 2024-07-01T11:43:31Z

@jinyoungchoi95
리뷰해주신 사항들 덕분에 많이 배웠습니다 🙇‍♀️
감사합니다 ! 😁

Starlight258 added 21 commits June 18, 2024 16:50

refactor: 리팩토링

5abe2a2

- 도메인과 밀접한 상수는 도메인에서 관리하도록 이동한다. - 초기화는 기존 값을 초기값으로 변경하는 의미를 가지므로, 특정 값으로 객체를 생성한다는 의미에 맞춰 메서드명을 변경한다.

fix: 우승자 예외 처리 버그 수정

685ce55

- 매칭되지 않은 경우를 이미 filter에서 확인하고 있으므로 그 외의 경우 예외를 발생시키도록 수정한다(orElseThrow()).

refactor: 함수형 인터페이스 활용

0f84cc2

- BiPredicate를 사용하여 매칭 여부를 판단하도록 작성한다. - enum 객체 내에 함수를 정의하여 캡슐화한다.

refactor: 메서드 private으로 변경

7def5fb

- 클래스 내부에서 사용하는 메서드는 private으로 변경한다.

test: WinningRank 테스트 작성

6fbbe0b

refactor: 반환값 수정

204eec4

- WinningRank의 findByMatchCountAndBonus 메서드는 호출하는 곳에서 key값만 사용하므로 반환값을 key 값으로 수정한다.

feat: LottoCount 값 객체 생성

3f1a41a

- 로또 개수를 나타내는 LottoCount 값 객체를 생성하여 원시형 int를 포장하고 로또 개수에 대한 로직을 모은다.

refactor: inputView에서 outputView 의존성 제거

e3c8191

- inputView는 입력 받는 역할에 집중하고, controller에서 inputView와 outputView를 모두 의존하도록 조정한다.

fix: 가격 배수 더하기 버그 수정

f01e1ac

- count가 0일 경우 기존 price가 0원이 되므로 더한 다음 곱하는 것이 아닌 곱한 가격을 더하도록 수정한다.

refactor: 리팩토링

2b05742

- 인자명 수정 - 테스트 코드 검증값 올바르게 수정 - 테스트 설명 수정

refactor: 메서드 10줄 이하로 리팩토링

48e7deb

test: LottoCount 테스트 추가

0232fab

test: 테스트 작성

49c785c

refactor: 사용하지 않는 메서드 삭제

c61bd49

refactor: Lotto 내부 컬렉션 Set로 변경

17e4d69

- List에서 Set으로 변경하면 내부 원소들의 중복 여부를 확인하지 않아도 된다.

refactor: 매개변수에 final 사용하여 불변성 표현

dda74be

rename: LottoResult를 LottoStatistics로 변경

6a3581d

- 로또 당첨 통계를 더 잘 나타내는 이름으로 변경

refactor: 필요없는 메서드 제거 및 메서드명 변경

63711c3

docs: README 업데이트

20eb06c

jinyoungchoi95 suggested changes Jun 28, 2024

View reviewed changes

Starlight258 added 6 commits July 1, 2024 17:27

refactor: 메서드 파라미터 타입 변경

7b9c6fa

- 호출하는 함수에서 모두 getter를 이용해 값으로 접근하기 때문에 메서드 파라미터 타입을 LottoCount로 변경하는 것이 좋다.

refactor: LottoStatistics에서 WinningRank 자체를 key로 사용

e715e31

- WinningRank를 key로 사용하여 LottoStatistics에 저장하면, WinningRank에서 key를 별도로 관리할 필요가 없어지고 의미상 명확하다.

rename: LottoStatistics.java -> LottoResult.java

4e486aa

feat: lotto 출력 오름차순으로 수정

2ff823c

rename: 패키지명 변경

4188aec

- 콘솔 애플리케이션 구현에 맞게 적절한 패키지명을 부여한다.

docs: README.md 업데이트

2134c60

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[로또 미션] 김명지 미션 제출합니다. #34

[로또 미션] 김명지 미션 제출합니다. #34

Starlight258 commented Jun 23, 2024

jinyoungchoi95 left a comment

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024

jinyoungchoi95 Jun 28, 2024

Starlight258 Jul 1, 2024 •

edited

Loading

Starlight258 commented Jul 1, 2024

[로또 미션] 김명지 미션 제출합니다. #34

Are you sure you want to change the base?

[로또 미션] 김명지 미션 제출합니다. #34

Conversation

Starlight258 commented Jun 23, 2024

jinyoungchoi95 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Starlight258 Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Starlight258 commented Jul 1, 2024

Starlight258 Jul 1, 2024 •

edited

Loading