로또 1단계 미션 제출 by chae-heechan · Pull Request #6 · biforest/java-lotto

chae-heechan · 2021-04-02T15:50:16Z

No description provided.

oereo

정말 고생많으셨습니다!!!!! 코드를 보면서 제 코드가 고칠 부분이 많이 보이는 것 같아요!! 정말 고생하셨습니다!!

README.md

oereo · 2021-04-03T12:42:38Z

src/main/java/lotto/domain/MatchCount.java

+        this.isBonusMatch = isBonusMatch;
+    }
+    public boolean isUnderThree() {
+        return matchCount < 3;


전역상수로 빼는 방향은 어떠신가요??? 예를들어 로또의 규칙이 바뀌거나 범위가 1~45에서 바뀔 경우 상수로 빼는것도 좋을 것 같아요!!!

oereo · 2021-04-03T12:45:03Z

src/main/java/lotto/domain/dto/WinningResult.java

+public class WinningResult {
+    private final int numberOfWinners;
+    private final GameResult gameResult;
+    public WinningResult(int numberOfWinners, GameResult gameResult) {


numberOfWinners 라고 하니까 살짝 이해가 안될 수도 있을 것 같아요!!! 읽었을 때 우승자의 수?? 이렇게 해석이 될 수도 있지 않을까 하는 조심스러운 리뷰 달겠습니다!!! 저의 지극히 개인적인 생각이옵니다!

oereo · 2021-04-03T12:46:35Z

src/main/java/lotto/domain/lotto/LottoTicket.java

+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+public class LottoTicket {


살포시 띄어주세요!!!

oereo · 2021-04-03T12:47:52Z

src/main/java/lotto/domain/lotto/LottoTicket.java

+                .collect(Collectors.toList());
+        return lottoNumbers;
+    }
+    public boolean contains(LottoNumber lottoNumber) {


이건 제가 궁금해서 그런 부분인데 contains 라는 method가 기존에 있는데 같은 이름의 method 를 구현하는게 괜찮은 방향인지 모르겠어요!!!

oereo · 2021-04-03T12:49:14Z

src/main/java/lotto/domain/result/GameResult.java

+import java.util.List;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+public enum GameResult {


오오오오 enum 활용 좋습니다!!

oereo · 2021-04-03T12:54:33Z

src/main/java/lotto/domain/dto/WinningResult.java

+    public GameResult getGameResult() {
+        return gameResult;
+    }
+}


개행을 추가하면 될 거에요!!

oereo · 2021-04-03T13:02:44Z

src/main/java/lotto/domain/MatchCount.java

+        return matchCount == 5 && isBonusMatch;
+    }
+    public boolean isAllMatch() {
+        return matchCount == 6;


isAllMatch 와 matchCount == 6 으로 했을 때 나중에 수정을 할 상황이 올 것 같아요!!!

begaonnuri

구현 잘해주셨네요 💯
몇가지 변경사항 남겼으니 반영해주시고, 궁금한점 DM주세요 :)

begaonnuri · 2021-04-11T11:49:05Z

README.md

@@ -1,5 +1,47 @@
 # 🚀 로또 1단계 - 자동

+##  📋기능 구현 목록


기능 목록 👍🏻

begaonnuri · 2021-04-11T11:49:31Z

build.gradle


 version = '1.0.0'
-sourceCompatibility = 1.8
+sourceCompatibility = 14


프로젝트의 자바 버전을 올린 이유가 있을까요?

begaonnuri · 2021-04-11T11:50:16Z

src/main/java/lotto/LottoApplication.java

+        LottoApplication app = new LottoApplication(new Printer(), new Receiver(System.in));
+        app.run();


Application이 Application을 생성해서 실행시키는 것이 어색하게 느껴지네요.
run()은 애플리케이션의 흐름을 제어하면서 로직을 진행시키는 것 같은데 이것을 따로 객체로 분리해보면 어떨까요?
MVC 패턴과 Controller의 역할에 대해 학습해보면 좋을것같아요.

https://m.blog.naver.com/jhc9639/220967034588

begaonnuri · 2021-04-11T11:56:14Z

src/main/java/lotto/LottoApplication.java

+        if(manualTicketCount > purchaseAmount/LOTTO_TICKET_PRICE){
+            throw new IllegalArgumentException(LOTTO_TICKET_PRICE_RANGE_EXCEPTION_MESSAGE);
+        }


PurchaseAmount란 객체를 만들고 이 역할을 하도록 할 수 있지 않을까요?
main의 역할이 너무 큰 것 같아요.

begaonnuri · 2021-04-11T15:05:23Z

src/main/java/lotto/domain/lotto/LottoTicket.java

+    public LottoTicket() {
+        List<LottoNumber> lottoNumbers = LottoNumber.range();
+        lottoNumbers = generateLottoNumbers(lottoNumbers);
+        this.lottoNumbers = new ArrayList<>(lottoNumbers);
+    }
+    public LottoTicket(List<Integer> manualLottoNumbers){
+        List<LottoNumber> lottoNumbers = LottoNumber.setManualNumber(manualLottoNumbers);
+        this.lottoNumbers = new ArrayList<>(lottoNumbers);
+    }


이런 경우 사용자는 어떤 생성자가 수동 로또 생성인지, 자동 로또 생성인지 모를 것 같아요.
정적 팩토리 메소드로 구분해보는건 어떨까요?

begaonnuri · 2021-04-11T15:32:36Z

src/test/java/lotto/domain/MatchCountTest.java

+class MatchCountTest {
+
+    @Test
+    void isUnderThree() {
+    }
+
+    @Test
+    void isMatchThree() {
+    }
+
+    @Test
+    void isMatchFour() {
+    }
+
+    @Test
+    void isMatchFiveWithoutBonus() {
+    }
+
+    @Test
+    void isMatchFiveWithBonus() {
+    }
+
+    @Test
+    void isAllMatch() {
+    }
+}


내부를 구현해주세요!

begaonnuri · 2021-04-11T15:34:51Z

src/test/java/lotto/domain/StatisticsTest.java

+        assertThat(winningResults.get(0).getGameResult()).isEqualTo(GameResult.THREE_MATCHED);
+        assertThat(winningResults.get(0).getNumberOfWinners()).isEqualTo(3);
+
+        assertThat(winningResults.get(1).getGameResult()).isEqualTo(GameResult.FOUR_MATCHED);
+        assertThat(winningResults.get(1).getNumberOfWinners()).isEqualTo(1);
+
+        assertThat(winningResults.get(2).getGameResult()).isEqualTo(GameResult.FIVE_MATCHED_WITHOUT_BONUS);
+        assertThat(winningResults.get(2).getNumberOfWinners()).isEqualTo(0);
+
+        assertThat(winningResults.get(3).getGameResult()).isEqualTo(GameResult.FIVE_MATCHED_WITH_BONUS);
+        assertThat(winningResults.get(3).getNumberOfWinners()).isEqualTo(0);
+
+        assertThat(winningResults.get(4).getGameResult()).isEqualTo(GameResult.ALL_MATCHED);
+        assertThat(winningResults.get(4).getNumberOfWinners()).isEqualTo(1);


이렇게 테스트가 많을 경우 assertAll을 활용할 수 있어요.
assertThat이 1부터 10까지 있을때 assertAll을 사용하지 않은 경우 4에서 실패하면 그대로 테스트가 끝나지만,
assertAll을 사용하면 4에서 실패해도 5부터 10까지 계속 진행을 한답니다 :)

https://javacan.tistory.com/entry/JUnit-5-Intro

begaonnuri · 2021-04-11T15:35:13Z

src/main/java/lotto/domain/result/GameResult.java

+    FOUR_MATCHED(MatchCount::isMatchFour, "4개 일치", 50_000),
+    FIVE_MATCHED_WITHOUT_BONUS(MatchCount::isMatchFiveWithoutBonus, "5개 일치", 1_500_000),
+    FIVE_MATCHED_WITH_BONUS(MatchCount::isMatchFiveWithBonus, "5개 일치, 보너스 볼 일치", 30_000_000),
+    ALL_MATCHED(MatchCount::isAllMatch, "6개 일치", 2_000_000_000);


_ 구분 👍🏻

begaonnuri · 2021-04-11T15:38:21Z

src/test/java/lotto/domain/lotto/LottoNumberTest.java

+                .isThrownBy(() -> new LottoNumber(lottoNumber))
+                .withMessage("로또 번호는 1 이상, 45 이하여야 합니다.");
+    }
+    @DisplayName("해시코드 테스트 커버리지 커")


테스트는 어떤 목적일까요?
그리고 테스트 커버리지는 무슨 의미를 가질까요?
equals()처럼 누구나 아는 api를 테스트할 필요가 있을까요?

begaonnuri · 2021-04-11T15:38:36Z

src/test/java/lotto/domain/result/GameResultTest.java

+    @MethodSource(value = "provideEvaluate")
+    @ParameterizedTest(name = "matchCount = {0}, isBonusMatch = {1}, expected = {2}")


MethodSource 👍🏻

chae-heechan added 30 commits March 29, 2021 18:57

docs: 기능 구현 목록 작성

1756707

docs: 입력 금액 예외 사항 정리

db1647d

docs: 클래스 정리

673ac2e

feat: 클래스 생성

5e62790

feat: lotto 번호 생성 기능 추가

73db742

refactor: 난수 생성 메소드 stream으로 수정

b74d509

feat: 로또 발권 기능 추가

5863fc8

feat: 입력 타입 String으로 변경

3406e4a

feat: 로또 번호 출력 기능 추가

1318d7a

docs: 기능 구현 목록 변경

7fa1b41

docs: 예외 사항 목록 변경

380b00e

docs: 클래스 목록 변경

6aea022

refactor: LottoTicket 구조 변경

2e761cf

feat: LottoApplication 구현

40543c8

feat: Printer 구현

983c002

refactor: 생성자 구현

e1efbaf

fix: language level 변경

b0db442

feat: GameResult 구현

f5c9577

refactor: LottoTickets 구조 변경

4b04a8b

refactor: Issuer 구조 변경

c0d6281

feat: Statistics 구현

60fc2a0

feat: MatchCount 구현

a01c323

feat: LottoNumber 구현

77cddd7

feat: WinningNumber 구현

f2068d1

feat: WinningResult 구현

1e16e0d

feat: GameResults 구현

f13151d

test: GameResultTest 작성

0f8bc09

test: IssuerTest 작성

09c6fb4

test: LottoNumberTest 작성

eef113d

test: MatchCountTest 작성

f9638c7

chae-heechan added 4 commits April 2, 2021 23:54

test: StatisticsTest 작성

13f3aac

test: WinningNumberTest 작성

ac4f5f2

test: WinningResultTest작성

a87480f

feat: 코드 병합

7cfa859

chae-heechan changed the base branch from main to chae-heechan April 2, 2021 15:50

oereo reviewed Apr 3, 2021

View reviewed changes

feat: 수동 로또 생성 기능 추가

2e40f10

begaonnuri requested changes Apr 11, 2021

View reviewed changes

		@@ -1,5 +1,47 @@
		# 🚀 로또 1단계 - 자동

		## 📋기능 구현 목록

		LottoApplication app = new LottoApplication(new Printer(), new Receiver(System.in));
		app.run();

		@MethodSource(value = "provideEvaluate")
		@ParameterizedTest(name = "matchCount = {0}, isBonusMatch = {1}, expected = {2}")

Conversation

chae-heechan commented Apr 2, 2021

Uh oh!

oereo left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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

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!

3 participants