[1단계 - 로또 구현] 현구막(최현구) 미션 제출합니다. #275

Hyeon9mak · 2021-02-18T11:30:42Z

안녕하세요 코니! 현구막 입니다. 페어인 포츈과 즐겁게 구현하다보니 시간 가는줄 모르고 제출이 많이 늦었습니다.
아직 많이 부족한 코드지만, 코니가 해주시는 피드백에 따라 열심히 고쳐볼게요!

로또 구현을 진행하면서 궁금했던 것들을 몇 가지 질문드리고 싶어요!

1. MVC에서 Model(domain)의 연결지점

제가 레이싱 게임 미션을 진행하면서까지 생각했던 MVC 구조는 아래와 같았어요.

그런데 포츈과 페어를 진행하면서, 포츈이 작성한 레이싱 게임 Controller 코드를 보고,
'아..! 이 구조가 MVC 관계를 더 명확하게 보여주는 것 같다! Controller가 정말 View-Model 사이의 통신만 도와주네!'
라고 인사이트를 얻어서, 아래와 같은 방식의 MVC 구조로 구현했어요.

그런데 구현이 끝난 상태의 모습을 보니 LottoGameController는 LottoGame에게 명령만 내리기 때문에 너무 가볍고,LottoGame은 서비스 로직을 소화하기 때문에 무겁고, 가지고 있는 필드 멤버가 너무 많다는 생각이 들어서 고민이 생기기 시작했어요.

두 구조 모두 장단점이 있는 것 같은데, 프로젝트마다 어떤 구조를 채택해야할지 기준점을 알고 싶어요!

2. 일급 컬렉션 클래스는 정확히 컬렉션 역할만?

이번 미션의 주제가 OOP였기 때문에 '클래스 파일을 하나라도 더 쪼개보는게 어떤가' 라는 생각에 우선 컬렉션 역할만 수행하도록 했습니다.
그러다가 '일급 컬렉션 클래스에 서비스 로직이 포함되어도 괜찮지 않을까?' 라는 생각이 들었는데, 막상 서비스 로직이 포함되면
일급 컬렉션 이라는 이름이 안어울리는 것 같고...

확신을 얻을 수 있는 자료가 있을까요?!

3. 방어적 복사는 어느정도까지?

    private final List<LottoNumber> lottoNumbers;

    public LottoNumbers() {
        this(new ArrayList<>());
    }

    public LottoNumbers(final List<LottoNumber> lottoNumbers) {
        this.lottoNumbers = new ArrayList<>(lottoNumbers);
    }

위와 같이 외부 객체와의 연결을 끊기 위한 방어적 복사는 기준이 잡힌 것 같은데,

    public void add(final LottoNumber lottoNumber) {
        lottoNumbers.add(lottoNumber);
    }

이렇게 내부 원소만 변하는 경우에도 아래와 같이 방어적 복사를 진행해야 하는지 궁금합니다.

    public void add(final LottoNumber lottoNumber) {
        lottoNumbers.add(lottoNumber);
        this.lottoNumbers = new ArrayList<>(lottoNumbers);
    }

감사합니다!!

Hyeon9mak · 2021-02-19T03:25:25Z

[TDD] 테스트 코드 우선 작성 - 5

내용

구현 기능 목록을 먼저 작성하는 연습
테스트 시작전 최대한의 설계 진행
테스트 작성이 쉬운 설계 고민
전략 패턴 학습과 적용
구현 기능 목록을 지속적으로 업데이트하면서 테스트 진행

링크

https://gmlwjd9405.github.io/2018/07/06/strategy-pattern.html

[OOP] 원시값 포장 - 3

내용

LottoNumber, Money, Rank 등의 원시 값을 포장
getter/setter 사용을 지양함으로서, '객체에 메세지를 보내라'를 지키려함
포장한 객체의 검증로직을 통해 원시값을 신뢰하고 서비스 로직을 구현 해나갈 수 있음

링크

https://manualz.kr/entry/JAVA-원시값-포장에-대해서-알아보록-하겠습니다

[OOP] 일급 컬렉션 - 4

내용

List<LottoNumber> 를 LottoNumbers로 대체
List<LottoTicket>을 LottoTickets로 대체

링크

https://jojoldu.tistory.com/412

[JDK] Enum - 5

내용

맞춘 로또 번호별 등수와 상금을 정리하기 위한 Rank enum 클래스 생성
Rank 상수 하나당 일치하는 번호 개수, 보너스 번호 일치 여부, 상금, 메세지 출력 양식을 저장시킴

링크

https://inor.tistory.com/12

[JDK] Stream API - 3

Rank enum 클래스에서 Stream API의 filter를 적극 활용하여 Rank 상수 감별

링크

723fe50

Joyykim · 2021-02-20T04:58:07Z

안녕하세요 현구막! 리뷰어는 아니지만 방어적 복사에 대해 참고할만한 링크 하나 드릴게여

https://johngrib.github.io/wiki/defensive-copy.md/

제 의견을 드리자면 현구막이 고민하고 있는 상황에서 리스트를 복사할 필요는 없다고 보고 외부에서 오는 LottoNumber를 복사할 것 같아요.
물론 LottoNumber가 불변객체라면 방어적 복사를 할 필요도 없겠네요ㅎㅎ

choihz

안녕하세요 현구막, 리뷰어 코니입니다 👋

로또 미션을 요구 사항에 맞게 잘 구현해 주셨어요 👍 꼼꼼한 커밋과 코멘트, 학습로그, 그리고 코드를 보면서 고민을 많이 하신 것이 느껴졌습니다.

질문에 먼저 답을 드릴게요.

저도 LottoGame의 존재는 좋다고 생각해요. 그런데 지금의 LottoGame은 현구막이 느끼시는 것처럼 너무 무겁고 복잡해요. 그러다 보니 테스트 코드를 작성하기도 어려워서 지금 중요한 로직의 테스트가 많이 빠져 있는 상황이네요. 책임을 다른 객체들에게 나눠 주고 테스트 코드도 작성해 보시면 좋을 것 같아요.
그리고 제 관점에서는 그려주신 두 가지 구조가 크게 다르지 않다고 느껴져서요, 언제 뭘 선택한다 이런 말씀을 드리기는 애매하네요 😅
일급 컬렉션은 사실 보편적(?)으로 인지되는 개념은 아닌 것 같고요, 따라서 자료도 거의 없다시피 하죠. 가장 좋은 건 아마 현구막도 보셨을 이동욱님의 글인데요, 여기서도 일급 컬렉션의 장점 중 하나로 상태와 행위를 한곳에서 관리를 언급하고 있어요. 일급 컬렉션이 그저 값만 담고 있고 밖에서는 getter로 값을 꺼내 쓰기만 한다면, 코드의 복잡성만 높일 뿐 아무 장점을 누리지 못하는 거라고 생각해요.
리뷰어마다 의견이 다를 수 있는데 저는 필요한 곳에서만 하면 된다고 생각하는 편이에요. 지금은 학습 단계이니 그 필요성과 사용법을 배우는 것을 목표로 사용해볼 수는 있겠지만, 후자의 경우는 좀 과하다고 생각합니다. 이건 코멘트로도 남겨 드렸어요.

추가적으로 궁금한 점이나 의견이 있다면 언제든 코멘트나 DM 주세요!

choihz · 2021-02-20T03:14:07Z

src/main/java/lottogame/domain/LottoGame.java

+    }
+
+    public void buyTickets(final Money money) {
+        this.initMoney = new Money(Integer.toString(money.getValue()));


이건 방어적 복사인 건가요? 여기에서 이렇게까지 할 필요는 없어 보여요 😅

choihz · 2021-02-20T03:16:05Z

src/main/java/lottogame/domain/Money.java

+    public Money(final String money) {
+        this.value = Integer.parseInt(money);


문자열 파싱 정도는 밖에서 해 준다면 도메인은 핵심 로직에 더 집중할 수 있을 것으로 보여요!

아!!!!!!!!!!!
매 미션마다 '사용자의 입력에 따른 정수 변환은 Integer.parseInt 가 걸리는데...' 라고 고민은 해놓고 저런 식으로 구현을 진행했었는데, 말씀해주신대로 컨트롤러나 LottoGame 쪽에서 파싱 후 넘겨주면 간단하게 해결이 가능했었네요..
"원시 값을 감싸는 이유는 믿고 편하게 사용하기 위함이다." 라는 생각에 갇혀서 문자열로 넘어오는걸 처리해주는게 당연한 역할이라고 생각해왔습니다. 머리가 탁 트인 기분이에요!!

추가적으로 궁금한게 있어요!
외부에서 정수로 파싱된 데이터를 넘겨주게 될텐데, 이 때 파싱이 불가능한 문자열 혹은 공백을 검증, 파싱하는 로직은 LottoGame이나 Controller 에서 진행하게 되는건가요? 아니면 View 단에서 진행하게 되는 것인가요?
LottoGame이나 Controller가 검증 로직을 갖기에는 안그래도 무거운 책임을 더해주는 것 같고,
View 단에는 검증, 파싱 ‘로직’을 가지면 안되는거 아닌가 고민이 돼요.

DM으로도 같은 질문을 주셨기에 답변 드렸습니다~

그런데 이건 제 의견일 뿐이라서 당장 다음 미션에서 다른 리뷰어님이 저와는 상충하는 의견을 주실 수도 있어요. 이런 정답이 없는 문제에서는 리뷰어의 의견을 참고로 해서 본인만의 의견을 만들어 가시는 게 좋을 것 같습니다.

다시 한 번 강조할게요. 꼭 제 의견을 받아들여 수정하셔야 하는 건 아니에요!

choihz · 2021-02-20T03:17:02Z

src/main/java/lottogame/domain/Money.java

+        }
+    }
+
+    public boolean isCanBuyAmount(final int amount) {


이름에서 is는 빠져도 될 것 같아요.

is가 boolean 타입을 반환하는 메서드 네이밍 컨벤션이 아닌가 했는데,
자세히 알아보니 can, has, should 등이 같은 의미를 가지는군요!! 감사합니다!!

이미 보셨을지 모르겠지만, Bool 변수 이름 제대로 짓기 위한 최소한의 영어 문법 추천드립니다 ㅎㅎ

choihz · 2021-02-20T08:30:23Z

src/main/java/lottogame/domain/ticket/LottoTicket.java

+
+    public LottoTicket() {
+        this.lottoNumbers = new LottoNumbers();
+        for (int number : issueLottoNumbers(new ArrayList<>())) {


빈 ArrayList가 여기서 생성되어 issueLottoNumbers(), initNumbers()까지 거쳐 들어가며 채워지고 있군요. 조금 더 단순해질 수도 있을 것 같아요. 그리고 제 생각엔 이렇게 로또 번호를 발권하는 역할은, 이름으로만 봐서는 LottoTicket보다 LottoTicketIssueMachine이 가지는 것이 더 자연스러울 것 같은데 현구막은 어떻게 생각하시나요?

로또 번호를 찍는 것이 로또 티켓이 가져야할 행동이라고 생각했어요. 그런데 코니의 피드백을 토대로 생각해보면

발급 행위는 "1~45 번호를 만들어서", "갖는 것"
만드는 행위를 LottoTicket이 해버리면 사실 LottoTicketIssueMachine이 존재할 이유가 크게 없다.

현실 세계에서 LottoTicket은 LottoNumber들의 묶음이다.
LottoNumbers == LottoTicket이므로, LottoTickets를 없앨 수 있다.
LottoTicket이 LottoNumber의 일급 컬렉션이 될 수 있다.

까지 생각이 정리되는 것 같아요.

choihz · 2021-02-20T08:39:16Z

src/main/java/lottogame/domain/ticket/LottoTickets.java

+import java.util.Collections;
+import java.util.List;
+
+public class LottoTickets {


이 일급 컬렉션은 하는 일이 별로 없네요. toList() 메서드에 의해 List<LottoTicket>이 리턴되고 있는데 여기부터 시작해서 일급 컬렉션에서 값을 직접 꺼내 쓰는 일이 연쇄적으로(LottoTickets → LottoTicket → LottoNumbers → LottoNumber) 일어나고 있어요. 값을 꺼내 쓰기보다 메시지를 보내는 방식으로 바꿔볼 수 있을까요? 그렇게 하면 어떤 장점이 있을까요?

객체간의 참조를 줄이고, 내부 구현을 드러내지 않을 수 있네요.
객체에 메세지를 보내라를 알고 있음에도 항상 응용하지 못하는건 생각의 시작점이 문제인 것 같습니다.

"출력뿐만 아니라 포괄적으로 사용가능한 toList()가 좋겠어!"
식으로 생각을 시작하면 안되고,

"출력을 위해 Controller가 로또 번호가 나열되는 리스트(혹은 문자열)를 달라고 요청시켜야겠군"
식으로 필요한 것만 최소한으로 구현하는 것으로 생각을 시작해야 하는걸까요?

출력을 위한 getter는 불가피한 면이 있다고 생각해요. 다만 도중에 굳이 값을 직접 꺼내지 않아도 되는데 꺼내는 경우(아마도 주로 연산이나 비교를 위해서?)가 있다면 메시지를 보내는 방향으로 개선해 볼 수 있을 것 같아요.

choihz · 2021-02-20T09:06:39Z

src/main/java/lottogame/domain/machine/LottoWinNumberIssueMachine.java

+    public static LottoNumbers issueWinNumbers(final String numbers) {
+        LottoNumbers lottoNumbers = new LottoNumbers();
+        for (String number : numbers.split(DELIMITER)) {
+            lottoNumbers.add(new LottoNumber(number));
+        }
+        return new LottoNumbers(lottoNumbers.toList());
+    }


여기도 방어적 복사인가요? 마찬가지로, 한 메서드 안에서 이렇게까지 할 필요는 없어 보여요 😳

choihz · 2021-02-20T09:23:11Z

src/main/java/lottogame/domain/number/LottoNumbers.java

+
+    private void validateDuplicate(final List<LottoNumber> lottoNumbers) {
+        if (new HashSet<>(lottoNumbers).size() != lottoNumbers.size()) {
+            throw new IllegalArgumentException("잘못된 당첨번호 입력입니다.");


이 예외가 발생할 일은 테스트 코드에서 말고는 없네요. 실제 어플리케이션에서 중복 당첨 번호가 들어오면 아래에 있는 add() 메서드에 의해 먼저 예외가 발생합니다.

choihz · 2021-02-20T09:25:33Z

src/main/java/lottogame/domain/number/LottoNumbers.java

+    public void add(final LottoNumber lottoNumber) {
+        if (contains(lottoNumber)) {
+            throw new IllegalArgumentException("중복된 당첨번호가 존재합니다.");
+        }
+        lottoNumbers.add(lottoNumber);
+    }


생성자의 validateDuplicate()에 의해 중복이 확인되어야 할 것 같은데, 위에서 말씀드린 것처럼 실제로는 여기서 예외가 발생하고 있어요. 이 설계에서의 문제점은 무엇일까요?

생성자만 사용하지 않고 이 메서드를 따로 만드신 이유가 무엇인가요?

첫 번째 문제로 생성자를 저렇게 만들었다면 이미 외부에서 완성된 List를 넘겨줬어야 했을 것 같고,
두 번째 문제로 List 앞에 final 키워드를 붙인 것은 "원소 불변을 원한다는 일종의 표식"으로 사용한 것인데, (실제론 List 재할당만 막을뿐 원소 불변을 지키진 못하니까) 정작 add 메서드를 제공해서 이를 지키지 못했습니다.

로또 티켓과 당첨번호 모두 번호 6자리를 사용하므로, LottoNumbers 객체를 재활용하려했습니다. 하지만 지나치게 복잡하기만 해졌네요... 😭

choihz · 2021-02-20T09:27:32Z

src/test/java/lottogame/domain/ticket/LottoTicketTest.java

+        assertThat(lottoTicket.getLottoNumbers().size()).isNotEqualTo(5);
+        assertThat(lottoTicket.getLottoNumbers().size()).isEqualTo(6);
+        assertThat(lottoTicket.getLottoNumbers().size()).isNotEqualTo(7);


Suggested change

assertThat(lottoTicket.getLottoNumbers().size()).isNotEqualTo(5);

assertThat(lottoTicket.getLottoNumbers().size()).isEqualTo(6);

assertThat(lottoTicket.getLottoNumbers().size()).isNotEqualTo(7);

assertThat(lottoTicket.getLottoNumbers().size()).isEqualTo(6);

이거면 충분하지 않나요?

'경계 값을 검사해야한다.' 는 생각에 저런 코드를 작성했는데, 사실 isEqualTo(6) 자체가 6 이외엔 모두 실패할테니 의미가 없는 라인들이었네요... 깊게 생각하지 않은게 뻔히 보이네요 😭

choihz · 2021-02-20T09:28:19Z

src/test/java/lottogame/domain/machine/LottoWinNumberIssueMachineTest.java

+public class LottoWinNumberIssueMachineTest {
+
+    @Test
+    @DisplayName("당첨 번호를 문자로 만들려고 할때")


테스트 이름은 "~ 할 때 ~ 한다"는 식으로 구체적으로 작성해 주세요!

add issueNumbers logic in LottoTicketIssueMachine add random lotto number create logic in LottoNumber add random number create class (RandomUtils)

delete LottoWinConfirmationMachine class delete LottoWinNumberIssueMachine class delete LottoNumbers class add contains method in LottoTicket add LottoWinningNumbers class

add parameter in constructor add validate min amount logic

LottoGame class focus only compare tickets and winning numbers

add get total invenstment money method add get total winning price method

Hyeon9mak · 2021-02-22T06:17:30Z

안녕하세요 코니! 리팩토링에 시간이 오래 걸렸습니다!
이번에도 질문이 있어요!

1. 객체간 결합도?

LottoTickets 클래스의 경우 테스트 코드 작성이 너무 벅차서 결국 포기했습니다. 숨이 턱 막히더라구요...
한 객체가 다른 객체를 필드멤버로 가지는 것이 자연스러운 모습이라고 생각했었는데, 가지고 있는 단계가 깊어질수록 테스트 코드를 작성할 때
고려해야할 단계도 깊어졌어요. 이 느낌이 객체간 결합도가 높다는 것일까요?
그렇다면 해법은 다른 객체를 필드멤버로 갖는 일을 최대한 줄이는 것이 될까요? 무언가 뜬구름 같아서 어려워요..

2. Map<Rank, Integer> 는 포장해도 Map<Rank, Integer>

코니가 리뷰해주셨던 연쇄적인 getter부분을 출력에 사용하기 위해 LottoGameResult 로 포장해보았지만,
결국 루프를 돌면서 출력을 하려면 Map 자료구조를 사용해야하더라구요.
결국 OutputView 메서드를 한 번 더 쪼개는 것으로 구현했습니다. 이렇게 쪼개도 되는지 잘 모르겠어요!

3. 이게 Controller가 할 일인지 LottoGame이 할 일인지...

제가 이번에 리팩토링한 LottoGameController에서 LottoTickets 객체와 LottoWinningNumbers 객체를 생성하고 있어요.
이렇게 구현한 이유는 LottoGame 객체는 발급한 티켓들과 승자번호를 비교하는 행동이 주역할이라고 생각했기 떄문이에요!

그런데 MVC 구조를 다시 생각해보니 "LottoTickets 객체와 LottoWinningNumbers 객체 모두 LottoGame에서 생성하는게 맞지 않나? Controller는 LottoGame과 InputView, OutputView가 통신만 하게 하고... 라는 생각이 드네요.

조금 더 확신을 가질 수 있는 기준이 있을까요?!
매번 아슬아슬 줄타기 하는 느낌이 싫어요!!!!!

4. LottoGameResult 존재 의의

Map<Rank, Integer> 자료구조를 포장하기 위해 LottoGameResult 객체를 새로 구현했어요!
그와 더불어 총 수익금과 투자금액 등을 계산하는 로직도 포함시켰구요.
그런데 총 수익금과 투자금액 모두 LottoGame에서도 해줄 수 있는 일들이 아닌가? 라는 찝찝한 기분이 남네요.

당장 LottoGame이 그리 복잡하지도 않고, LottoGameResult의 메서드들이 모두 LottoGame으로 넘어갈 수 있지 않은가, 그렇다면 LottoGameResult는 굳이 필요없지 않은가 하는 고민이 드네요.

코니가 보시기엔 어떠신가요? 😅 프로의 시선에서 어떻게 보이는지 궁금해요!

감사합니다!!

choihz

안녕하세요 현구막!

피드백을 정말 잘 반영해 주셨네요 👍 코드를 보니까 처음보다 훨씬 쉽고 읽기 편하다는 느낌을 받았는데, 현구막도 그렇게 느끼셨을까요? (저만 그렇게 느꼈다면... 리뷰 실패! 😱) 질문에 대한 답은 하단에 따로 코멘트로 남겼구요, 이번 단계는 여기서 마무리할게요. 2단계에서 만나요!

choihz · 2021-02-22T16:42:15Z

src/main/java/lottogame/controller/LottoGameController.java

+    private LottoWinningNumbers getWinningNumbers() {
+        Set<Integer> winningNumbers = InputView.inputWinningNumbers();
+        int bonusNumber = InputView.inputBonusNumber();
+        return new LottoWinningNumbers(winningNumbers, bonusNumber);
+    }
+
+    private LottoTickets getLottoTickets(Money money) {
+        LottoTicketIssueMachine lottoTicketIssueMachine =
+            new LottoTicketIssueMachine(new Money(money));
+        return lottoTicketIssueMachine.issueTickets();
+    }


play()에서 사용되는 순서대로 메서드를 배치한다면 더 읽기 좋아질 것 같아요.

choihz · 2021-02-22T16:46:02Z

src/main/java/lottogame/domain/LottoGame.java

+    public LottoGame(final LottoTickets lottoTickets,
+        final LottoWinningNumbers lottoWinningNumbers) {


Suggested change

public LottoGame(final LottoTickets lottoTickets,

final LottoWinningNumbers lottoWinningNumbers) {

public LottoGame(final LottoTickets lottoTickets, final LottoWinningNumbers lottoWinningNumbers) {

줄바꿈을 하신 특별한 이유가 있을까요? 🤔

가로로 너무 길어지는 것 같아서 개행을 했었어요!

choihz · 2021-02-22T16:46:25Z

src/main/java/lottogame/domain/LottoGame.java

+        return this.lottoTickets.getMatchingResult(this.lottoWinningNumbers, initMachingResults());
+    }
+
+    private Map<Rank, Integer> initMachingResults() {


choihz · 2021-02-22T16:51:39Z

src/main/java/lottogame/domain/ticket/LottoTickets.java

+
+    private Rank getRank(LottoWinningNumbers lottoWinningNumbers, LottoTicket lottoTicket) {
+        int matchCount = lottoWinningNumbers.countMatchedWinningNumber(lottoTicket);
+        boolean isBonusMatch = lottoWinningNumbers.isMatchBonusNumber(lottoTicket);


Suggested change

boolean isBonusMatch = lottoWinningNumbers.isMatchBonusNumber(lottoTicket);

boolean hasBonus = lottoWinningNumbers.hasBonusNumber(lottoTicket);

이건 어때요?

훨씬 자연스럽고 좋은거 같아요!

choihz · 2021-02-22T16:55:05Z

src/main/java/lottogame/domain/LottoGameResult.java

+    private int countBoughtTickets() {
+        return this.matchingResults.values()
+            .stream()
+            .reduce(0, Integer::sum);
+    }
+
+    public double totalWinningPrice() {
+        return matchingResults.entrySet()
+            .stream()


this 키워드를 쓸지 말지 결정해서 하나로 통일해 주세요! (다른 곳에서도요~)

choihz · 2021-02-22T17:00:53Z

src/test/java/lottogame/domain/RankTest.java

+    @DisplayName("맞춘 개수별 등수 확인")
+    void testGetPrice() {
+        assertThat(Rank.of(6, false)).isEqualTo(Rank.RANK1);
+        assertThat(Rank.of(5, true)).isEqualTo(Rank.RANK2);


테스트가 실패하고 있어요 😱

헉... 로직만 고치고 테스트 코드는 안돌려봤네요 ㅠㅠ 반성하겠습니다..

choihz · 2021-02-22T17:08:02Z

src/test/java/lottogame/domain/ticket/LottoTicketsTest.java

+        LottoTicketIssueMachine lottoTicketIssueMachine =
+            new LottoTicketIssueMachine(new Money(3000));
+        LottoTickets lottoTickets = lottoTicketIssueMachine.issueTickets();
+        assertThat(lottoTickets.getLottoTickets().size()).isEqualTo(3);


Suggested change

assertThat(lottoTickets.getLottoTickets().size()).isEqualTo(3);

assertThat(lottoTickets.getLottoTickets()).hasSize(3);

😎

👍👍👍 assertThat 테스트 메서드들을 공부해야겠네요!!

choihz · 2021-02-22T17:09:44Z

src/test/java/lottogame/domain/ticket/LottoTicketTest.java

+        Set<LottoNumber> lottoNumberSet = new HashSet<>();
+        lottoNumberSet.add(new LottoNumber(1));
+        lottoNumberSet.add(new LottoNumber(2));
+        lottoNumberSet.add(new LottoNumber(3));
+        lottoNumberSet.add(new LottoNumber(4));
+        lottoNumberSet.add(new LottoNumber(5));
+        lottoNumberSet.add(new LottoNumber(6));
+        this.lottoTicket = new LottoTicket(lottoNumberSet);


여기서도 LottoWinningNumbersTest에서처럼 스트림을 사용하면 더 좋을 것 같네요 👍

choihz · 2021-02-22T17:10:54Z

src/test/java/lottogame/domain/LottoGameResultTest.java

+    @Test
+    @DisplayName("총 구입 금액이 일치하는지 확인")
+    void testSameMatchCount() {
+        assertThat(lottoGameResult.totalInvestment()).isEqualTo(6*1000);


Suggested change

assertThat(lottoGameResult.totalInvestment()).isEqualTo(6*1000);

assertThat(lottoGameResult.totalInvestment()).isEqualTo(6 * 1000);

choihz · 2021-02-22T17:11:57Z

src/test/java/lottogame/utils/RandomUtilsTest.java

+        final int startNumber = 1;
+        final int endNumber = 45;
+
+        for (int i = 0; i < 999999; i++) {


choihz · 2021-02-23T00:52:09Z

1: LottoTickets 클래스를 아무리 봐도, 크게 테스트 코드를 작성하기 어려워 보이지는 않는데요...? ㅎㅎㅎ
다른 객체를 멤버로 갖는 건 물론 필요할 때만 하는 게 좋죠. 그런데 지금의 설계는 충분히 잘하신 거라고 생각해요. 그리고 원래 개발은 어렵습니다!! (완전!!!)

2, 4: 이미 코멘트로도 남겼던 것처럼 출력을 위한 getter는 불가피하죠. 지금의 LottoGameResult는 totalWinningPrice() 같은 메서드도 있고 해서 책임을 잘 나눈 거라고 생각합니다 👍 다만 LottoGame을 보면 하는 일이 LottoTickets와 LottoWinningNumbers를 받아 LottoGameResult를 리턴하는 게 전부이긴 해서, 둘은 합쳐도 될 것 같긴 하네요.
그리고 프로의 시선이라고 하셨는데... 저는 프로가 되려면 정말 하아아안참 멀었구요(겸손 아님! 😅) 그냥 여러분이 걸을 길을 아주 조금 앞서 걷고 있는 입장이라고만 생각해 주세요. 또한 아시다시피 개발에는 정답이 없어서요, 리뷰어들에게 정답을 구하려 하지 마시고 유연하게 생각하며 본인의 의견을 적극적으로 표현해 주시는 것이 좋습니다. "이렇게 쪼개도 되는지 잘 모르겠어요!" 이런 생각은 안 하셔도 돼요!

3: 지금의 LottoGame이 사실 하는 일이 딱히 없어 보여서, 말씀하신 것처럼 LottoTickets와 LottoWinningNumbers를 LottoGame이 생성하는 것도 좋을 것 같네요. 그런데 "...하는게 맞지 않나?" 이런 생각은 하지 마세요! 위에서 답변을 드린 것과 같은 이야기인데, 이건 맞고 틀리고의 문제가 아니에요. 그리고 확신을 가질 수 있는 기준을 말씀하셨는데, 개발하면서 확신을 가지고 뭔가를 하는 날이 올 수 있을까 저는 잘 모르겠네요. 그 아슬아슬한 줄타기 하는 느낌을 계속 안고 가야 하는게 이 직업 아닐까요? ㅎㅎㅎ 물론 제가 아직 경험이 일천한 초초초초주니어 개발자라 이렇게 생각하는 것일 수도 있겠지만요.

Hyeon9mak added 29 commits February 16, 2021 18:09

docs: 기능 목록 정리

1cd3882

feat(LottoNumber): 로또 넘버 검증 기능 구현

3a2c5ad

chore: add gitignore

07bf9a0

feat(LottoNumbers): 로또 번호 리스트 일급컬렉션화

45f2b3d

feat(LottoTicket): 중복되지 않는 로또번호 6개 생성하는 기능 구현

820e8e7

feat(Money): 구입 금액 검증 기능 구현

752ecb1

refactor: 패키지 구조 변경

8fb542a

feat(LottoTickets): 로또 티켓 리스트 일급컬렉션화

b771861

refactor(Money): 로또 가격과의 의존성 제거

efdc01b

feat(LottoTicketMachine): 금액에 따른 로또 티켓 발급 기능 구현

6007ddc

refactor : 메소드가 하던일을 생성자에서 처리하게 수정

4cfb7ad

feat(LottoDrawingMachine): 당첨 번호 생성 기능 구현

28b4352

refactor(LottoTicketMachineTest): 테스트 메소드 이름 변경

5688c0c

feat(LottoDrawingMachine): 보너스 번호 생성 기능 구현

3a3a7db

refactor: 패키지 구조 변경

58670df

feat(LottoNumbers): 보너스번호가 당첨 번호와 중복된 숫자인지 판별하는 기능 구현

2a5119a

feat(LottoWinningMachine): 로또 티켓의 번호가 몇개나 일치하는지 확인하는 기능 구현

eb598cb

feat(LottoWinningMachine): 보너스 번호 확인 기능 구현

3e36885

refactor(LottoWinningDrawingMachine): 객체명을 명시적으로 변경

ce46c74

feat(LottoTicketResult): 로또 티켓과 당첨 번호 비교 결과를 저장하는 클래스 구현

7ee6248

feat(LottoWinningMachine): 보너스 번호 일치 여부 확인 로직 변경

f40d221

style(LottoTicketMachine): 자바 컨벤션 적용

aafbb8a

feat(LottoGame): 당첨된 티켓 개수를 저장하는 기능 구현

c20d5cf

feat(Rank): 당첨 번호 개수별 등수 분류 구현

723fe50

refactor: 자바 컨벤션 적용

267a204

feat(LottoTicket): 구매한 로또 정렬 기능 구현

fe05034

feat: 로또 수익률 계산 기능 구현

1512aef

feat: view 구현

d6cad1f

feat: Controller 구현

a07d395

Hyeon9mak added 4 commits February 19, 2021 15:16

refactor: fix responsibility for buy lotto tickets

3d621be

refactor: add final keyword

9890de1

style: fix enterline

4a1632a

feat(LottoWinConfirmationMachine): add validate count logic

1acfa9f

choihz suggested changes Feb 20, 2021

View reviewed changes

Hyeon9mak added 18 commits February 21, 2021 13:19

refactor(Money): fix constructor parameter data type

d8ee898

refactor(InputView): delete constant value, add validate integer logic

9329f13

refactor: fix lotto ticket issue logic

55f11d3

add issueNumbers logic in LottoTicketIssueMachine add random lotto number create logic in LottoNumber add random number create class (RandomUtils)

refactor: add new constructor in Money

64560e0

refactor: fix logic to print lotto numbers

d4170a5

refactor(InputView): add transform String to Integer and Integer Set

3a41a00

refactor: fix winning number compare logic

f1fa083

delete LottoWinConfirmationMachine class delete LottoWinNumberIssueMachine class delete LottoNumbers class add contains method in LottoTicket add LottoWinningNumbers class

refactor: delete unused class

a488d9b

refactor(InputView): rename constant value for convention

b698003

refactor(Rank): delete print message format

7d40936

refactor(OutputView): divide print methods

9cc1222

feat(LottoNumber): add constant value in exception message

137f977

test(RandomUtilsTest): rename methods

d427e25

test(LottoTicketTest): add setUp method, refactor test methods

af08229

refactor(LottoTickets): add compare the winning numbers logic

f533c1b

refactor(LottoTicketIssueMachine): fix constructor

9c4ec45

add parameter in constructor add validate min amount logic

refactor(LottoGame): divide LottoGame logic with Controller

9dd4c20

LottoGame class focus only compare tickets and winning numbers

feat(LottoGameResult): add LottoGame result class

acf6725

add get total invenstment money method add get total winning price method

refactor(Rank): fix find rank logic

6bcc6d8

choihz approved these changes Feb 23, 2021

View reviewed changes

choihz merged commit 96d0fcf into woowacourse:hyeon9mak Feb 23, 2021

utterances-bot mentioned this pull request Mar 23, 2021

%EC%9A%B0%EC%95%84%ED%95%9C%ED%85%8C%ED%81%AC%EC%BD%94%EC%8A%A4-%EB%A1%9C%EB%98%90-%EB%AF%B8%EC%85%98-%ED%9A%8C%EA%B3%A0/ Hyeon9mak/Hyeon9mak.github.io#41

Closed

		public Money(final String money) {
		this.value = Integer.parseInt(money);

		public LottoGame(final LottoTickets lottoTickets,
		final LottoWinningNumbers lottoWinningNumbers) {

	boolean isBonusMatch = lottoWinningNumbers.isMatchBonusNumber(lottoTicket);
	boolean hasBonus = lottoWinningNumbers.hasBonusNumber(lottoTicket);

	assertThat(lottoTickets.getLottoTickets().size()).isEqualTo(3);
	assertThat(lottoTickets.getLottoTickets()).hasSize(3);

	assertThat(lottoGameResult.totalInvestment()).isEqualTo(6*1000);
	assertThat(lottoGameResult.totalInvestment()).isEqualTo(6 * 1000);

[1단계 - 로또 구현] 현구막(최현구) 미션 제출합니다. #275

[1단계 - 로또 구현] 현구막(최현구) 미션 제출합니다. #275

Conversation

Hyeon9mak commented Feb 18, 2021 • edited Loading

1. MVC에서 Model(domain)의 연결지점

2. 일급 컬렉션 클래스는 정확히 컬렉션 역할만?

3. 방어적 복사는 어느정도까지?

Hyeon9mak commented Feb 19, 2021

[TDD] 테스트 코드 우선 작성 - 5

내용

링크

[OOP] 원시값 포장 - 3

내용

링크

[OOP] 일급 컬렉션 - 4

내용

링크

[JDK] Enum - 5

내용

링크

[JDK] Stream API - 3

링크

Joyykim commented Feb 20, 2021 • edited Loading

choihz 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

choihz Feb 21, 2021 • edited Loading

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

Hyeon9mak commented Feb 22, 2021 • edited Loading

1. 객체간 결합도?

2. Map<Rank, Integer> 는 포장해도 Map<Rank, Integer>

3. 이게 Controller가 할 일인지 LottoGame이 할 일인지...

4. LottoGameResult 존재 의의

choihz 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

Choose a reason for hiding this comment

choihz commented Feb 23, 2021

Hyeon9mak commented Feb 18, 2021 •

edited

Loading

Joyykim commented Feb 20, 2021 •

edited

Loading

choihz Feb 21, 2021 •

edited

Loading

Hyeon9mak commented Feb 22, 2021 •

edited

Loading