로또 미션 [SangBeom-Hahn] #2

SangBeom-Hahn · 2023-07-16T23:16:00Z

기능 단위로 커밋해 보았습니다. (완료 : 기능 구현/ 하는 중 : 뷰 분리, 예외처리, 리팩토링)

초기 엔터티를 구축, 테스트한다.

*feat: LottoGenerator 기능 및 테스트 구현 *feat: Controller 관련 기능 구현

*feat: Application main 메서드 구현 *feat: Controller 관련 기능 구현 *refactor: MoneyGenerator를 LottoGenerator에서 분리 *feat: MoneyGenerator 기능 및 테스트 구현 *test: LottoGenerator 검증 테스트 삭제

*feat: Lotto 기능 구현 *feat: Controller 관련 기능 구현

*feat: AnswerLotto 기능 구현 및 테스트 *feat: Controller 관련 기능 구현

*refactor: AnswerLotto bonusNumber 필드 추가 *refactor: Controller AnswerLotto 객체 생성

*feat: ResultService 기능 및 테스트 구현 *refactor: AnswerLotto bonusNumber 필드 타입 Integer에서 int로 변경 *refactor: AnswerLotto bonusNumber getter 추가 *refactor: Lotto numbers getter 추가

*feat: ResultService 기능 및 테스트 구현

*feat: ResultService 기능 및 테스트 구현 *feat: Controller 관련 기능 구현

sjiwon

저번에 디코에서 말한 부분이긴한데 전체적으로 매직넘버가 너무 많이 보이네요

그리고 자바 전체적인 tab 컨벤션은 4칸으로 맞춰서 구현해주세요
뷰 분리 관련해서는 앞으로 진행하실거라고 적어놓으셨기 때문에 따로 피드백 안했습니다

리뷰 다 지웠고 이따 클래스 단위로 다시 작성해드릴게요

sjiwon

기능 구현 완료라고 하셨는데 기능_테스트()는 통과하지 않네요

sjiwon · 2023-07-17T03:03:06Z

src/main/java/lotto/controller/Controller.java

+import static lotto.domain.Lotto.LottoGenerator.generateLottos;
+import static lotto.domain.Lotto.MoneyGenerator.generateMoney;
+
+public class Controller {


tab 컨벤션 4칸

sjiwon · 2023-07-17T03:22:54Z

src/main/java/lotto/controller/Controller.java

+    List<Lotto> lottos = user.getLottos();
+    System.out.println(lottos.size() + "개를 구매했습니다.");
+    for (Lotto lotto : lottos) {
+      System.out.println(lotto);
+    }


발행한 로또 수량 및 번호를 출력한다. 로또 번호는 오름차순으로 정렬하여 보여준다.

이러한 요구사항이 있는데 위의 코드는 이 요구사항을 지킨걸까요?

sjiwon · 2023-07-17T03:23:13Z

src/main/java/lotto/controller/Controller.java

+    System.out.println("당첨 번호를 입력해 주세요.");
+    String answerNumbers = readLine();
+    System.out.println("보너스 번호를 입력해 주세요.");
+    int bonusLottoNumber = Integer.parseInt(readLine());
+    List<Integer> answerLottoNumbers = Stream.of(answerNumbers.split(","))
+      .map(Integer::valueOf).collect(Collectors.toList());
+    AnswerLotto answerLotto = new AnswerLotto(answerLottoNumbers, bonusLottoNumber);
+
+    Map<String, Integer> statistics = resultService.getStatistics(lottos, answerLotto.getAnswerNumbers(), answerLotto.getBonusNumber());
+    double returnRate = resultService.getReturnRate(money);
+
+    System.out.println("당첨 통계");
+    System.out.println("---");
+    System.out.println("3개 일치 (5,000원)" + "-" +  statistics.get("THREE") + "개");
+    System.out.println("4개 일치 (50,000원)" + "-" +  statistics.get("FOUR") + "개");
+    System.out.println("5개 일치 (1,500,000원)" + "-" +  statistics.get("FIVE") + "개");
+    System.out.println("5개 일치, 보너스 볼 일치 (30,000,000원)" + "-" +  statistics.get("FIVEPLUSBONUS") + "개");
+    System.out.println("6개 일치 (2,000,000,000원)" + "-" +  statistics.get("SIX") + "개");
+    System.out.println(String.format("총 수익률은 %.1f입니다.", returnRate));


리팩토링 하신다고 했으니까 이 부분은 피드백 생략할게요

sjiwon · 2023-07-17T03:25:26Z

src/main/java/lotto/domain/Lotto/AnswerLotto.java

+import java.util.List;
+
+public class AnswerLotto {
+  private List<Integer> answerNumbers;


List<Integer> answerNumbers가 아닌 Lotto answerNumber로 가져오면 중복된 검증을 없앨 수 있지 않을까요?

밑에 보이는 검증이 Lotto에서도 동일하게 보이네요. 차이가 있어서 이렇게 나눈건가요

sjiwon · 2023-07-17T03:27:57Z

src/main/java/lotto/domain/Lotto/AnswerLotto.java

+  public AnswerLotto(List<Integer> answerNumbers, Integer bonusNumber) {
+    validateLottoSize(answerNumbers);
+    validateLottoHasDuplicate(answerNumbers);
+    validateLottoNumbersRange(answerNumbers);
+    this.answerNumbers = answerNumbers;
+    this.bonusNumber = bonusNumber;
+  }


answerNumbers에 있는 숫자를 bonusNumber에서 중복해서 선택하면 어떻게 되나요?

sjiwon · 2023-07-17T03:33:54Z

src/test/java/lotto/LottoTest.java

+    @Test
+    @DisplayName("로또 번호에 1 ~ 45 외 범위의 숫자가 있으면 예외가 발생한다.")
+    void lottoNumberIsNotInRange() {
+        // then
+        assertThatThrownBy(() -> new Lotto(List.of(66, 1, 2)))
+          .isInstanceOf(IllegalArgumentException.class);
+    }


hasMessage까지 검증하는게 어떨까요

그리고 선택이긴 하지만 given - when - then Pattern에서 given / when으로 주어질 Context가 없으면 저는 then 주석을 달지 않습니다

sjiwon · 2023-07-17T03:35:18Z

src/test/java/lotto/domain/Lotto/AnswerLottoTest.java

+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+class AnswerLottoTest {


각자 스타일이긴한데 예외 테스트 케이스를 상단에 두는것을 선호합니다

일반적으로 어떤 도메인 객체를 구현하고 내부적인 검증이 이루어지는데

검증 -> 생성

생성 -> 검증

이 두가지 순서중에 어떤 순서가 맞을까요? 테스트 코드도 마찬가지라고 생각합니다

sjiwon · 2023-07-17T03:36:39Z

src/test/java/lotto/domain/Lotto/AnswerLottoTest.java

+  @Test
+  @DisplayName("당첨 번호 6자리와 보너스 번호 1자리를 입력하면 당첨 로또를 선언할 수 있다.")
+  void canCreateAnswerLotto() {
+    // given
+    List<Integer> answerNumberList = List.of(1, 2, 3, 4, 5, 6);
+    Integer bonusNumber = 1;
+    AnswerLotto answerLotto = new AnswerLotto(answerNumberList, bonusNumber);
+
+    // then
+    assertThat(answerLotto.getAnswerNumbers()).isEqualTo(answerNumberList);
+  }


given - when - then Pattern으로 테스트코드를 작성하신거 같네요

AnswerLotto answerLotto = new AnswerLotto(answerNumberList, bonusNumber);

이 코드가 given context에 들어가는게 맞을까요?

추가적으로 당첨 번호와 보너스 번호는 중복될 수 없습니다.
AnswerLotto에서 이와 같은 검증을 진행하지 않았기 때문에 위의 테스트 코드가 통과하네요

sjiwon · 2023-07-17T03:38:55Z

src/test/java/lotto/domain/Lotto/MoneyGeneratorTest.java

+class MoneyGeneratorTest {
+  private InputStream in;
+
+  @AfterEach
+  void afterEach() throws IOException {
+    in.close();
+  }
+


MoneyGenerator가 외부 I/O에 의존하는 컴포넌트라서 어쩔 수 없이 InputStream을 조작해서 테스트를 진행하셨네요

검증을 위한 Component를 따로 구현하고 해당 Component를 구현하는게 어떨까요

그러면 외부 I/O에 의존하지 않은 순수한 Unit Test가 되겠죠?

sjiwon · 2023-07-17T03:39:57Z

src/test/java/lotto/service/ResultServiceTest.java

+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertAll;
+
+class ResultServiceTest {


통계 메소드 테스트를 하나로 통일하고 내부적으로 통계에 대한 테스트 케이스 데이터를 생성해서 분리하는게 깔끔하겠네요

제 테스트 코드 LottoStatisticsTest를 참고하시면 도움이 될거같네요

sjiwon

AnswerLotto 테스트 추가 피드백

SangBeom-Hahn · 2023-07-17T12:42:55Z

매직넘버와 immutable 등 1주차에 감사히 피드백 주신 부분들은 모두 인지하고 있고 리팩토링 때 적용하려고 했습니다.
다음 미션부턴 완료되지 않은 리팩토링 항목들도 pr에 같이 작성하겠습니다.

SangBeom-Hahn added 12 commits July 14, 2023 12:58

docs(README): 요구사항과 기능 목록 작성

2df0cb9

feat: 초기 엔터티 구축

389c255

초기 엔터티를 구축, 테스트한다.

feat: 로또 발행 기능 구현

e94adef

*feat: LottoGenerator 기능 및 테스트 구현 *feat: Controller 관련 기능 구현

refactor(LottoGenerator): 구매금액 배수 검증 메서드 분리

a14f592

chore: Lotto 패키지 생성

4b3305b

feat: 구입금액 입력 기능 구현

ca99ae6

*feat: Application main 메서드 구현 *feat: Controller 관련 기능 구현 *refactor: MoneyGenerator를 LottoGenerator에서 분리 *feat: MoneyGenerator 기능 및 테스트 구현 *test: LottoGenerator 검증 테스트 삭제

feat: 발행한 로또 내역 출력 기능 구현

d8c2c32

*feat: Lotto 기능 구현 *feat: Controller 관련 기능 구현

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

5e30c1e

*feat: AnswerLotto 기능 구현 및 테스트 *feat: Controller 관련 기능 구현

refactor: 당첨 번호와 보너스 번호를 따로 저장하도록 변경

4e63004

*refactor: AnswerLotto bonusNumber 필드 추가 *refactor: Controller AnswerLotto 객체 생성

feat: 당첨 통계 기능 구현

6fe1833

*feat: ResultService 기능 및 테스트 구현 *refactor: AnswerLotto bonusNumber 필드 타입 Integer에서 int로 변경 *refactor: AnswerLotto bonusNumber getter 추가 *refactor: Lotto numbers getter 추가

feat: 수익률 계산 기능 구현

c06ea29

*feat: ResultService 기능 및 테스트 구현

feat: 당첨 통계와 수익률 출력 기능 구현

1e752fb

*feat: ResultService 기능 및 테스트 구현 *feat: Controller 관련 기능 구현

SangBeom-Hahn requested a review from sjiwon July 16, 2023 23:16

sjiwon changed the title ~~sangbeom~~ 로또 미션 [SangBeom-Hahn] Jul 17, 2023

sjiwon reviewed Jul 17, 2023

View reviewed changes

로또 미션 [SangBeom-Hahn] #2

Are you sure you want to change the base?

로또 미션 [SangBeom-Hahn] #2

Uh oh!

Conversation

SangBeom-Hahn commented Jul 16, 2023

Uh oh!

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

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

sjiwon Jul 17, 2023 • 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!

sjiwon Jul 17, 2023 • 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!

sjiwon left a comment

Choose a reason for hiding this comment

Uh oh!

SangBeom-Hahn commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sjiwon left a comment •

edited

Loading

sjiwon Jul 17, 2023 •

edited

Loading

sjiwon Jul 17, 2023 •

edited

Loading