Oereo 과제제출합니다. #3

oereo · 2021-04-02T02:51:46Z

과제가 정말 전보다 어려워진 느낌인것 같아요... 아마 제약조건이 붙어서 더 그런 것 같습니다!
아직 테스트케이스와 예외처리가 다 되지 않아서 정말 죄송합니다.. 이제 회사를 퇴사했기 때문에 하루종일 자바과제를 보면서 공부하고 고치도록 하겠습니다!
코드리뷰는 쌍욕만 아니면 다 괜찮으니 많이많이 달아주세요!

…6개를 리스트로 반환하는 method 구현

…allResult 객체를 가져오는 method 구현

…toTicket 값을 파라미터로 넘기도록 수정

Jummi10

binary file들이 열 개 정도가 같이 올라와 있네요
제가 구현한 당첨자 통계를 결정하는 부분이 마음에 들지 않아 고민이었는데 세훈님 코드에는 Map<WinningStatus, Integer>으로 구현하여 깔끔하다고 느껴져서 좋았습니다. 노란 박스 뜨는 부분이 있던데 이 부분들과 변수, 메서드명, 필드과 메서드간 구분(띄어쓰기?)만 신경 써주시면 될 것 같습니다. 리뷰 단 부분 중에 이건 아니다라 느끼거나 이해 안 가는 부분 있으시면 댓글 달아주세요

수고하셨습니다~!

Jummi10 · 2021-04-02T06:23:45Z

src/main/java/lotto/ui/Printer.java

+    private static final String PRINT_FINAL_MATCHED_LOTTO_RESULT_MESSAGE = "%s개 일치 (%s원)- %s개";
+    private static final String PRINT_LOTTO_PROFIT_MESSAGE = "총 수익률은 %f입니다.";


오호 저는 "개 일치", "원", "개" 이런 식으로 다 나눠놨는데 이렇게 %s와 %f 로 하니까 너무 깔끔하고 좋네요!! 이 부분은 저도 참고해야겠네요!!

감사합니다!!!!!^^

Jummi10 · 2021-04-02T06:24:19Z

src/main/java/lotto/ui/Printer.java

+    private static final String PRINT_LOTTO_PROFIT_MESSAGE = "총 수익률은 %f입니다.";
+    public void requestPurchaseAmount() {
+        System.out.println(REQUEST_PURCHASE_AMOUNT_MESSAGE);
+    }
+    public void requestLastWeekLottoWinningNumber() {
+        System.out.println(REQUEST_LAST_WEEK_LOTTO_WINNING_NUMBER_MESSAGE);
+    }
+    public void requestLottoBonusBallNumber() {
+        System.out.println(REQUEST_LOTTO_BONUS_BALL_NUMBER_MESSAGE);
+    }
+    public void printAllLotto(Lottos lottos){
+        for (LottoTicket lotto : lottos.getLottos()) {
+            System.out.println(lotto.getLotto());
+        }
+    }


필드와 메서드 사이를 한 칸씩 띄어주면 더 읽기 좋을 것 같아요~

앗앗 그러네요!!! 정렬을 한번씩 잘 했어야 했는데 못한 것 같아요 ㅠㅠㅠ

Jummi10 · 2021-04-02T06:34:35Z

src/main/java/lotto/ui/Receiver.java

+        return dividedLastWeekLottoWinningNumbers.stream().map(Integer::parseInt).collect(Collectors.toList());
+


return dividedLastWeekLottoWinningNumbers.stream() .map(Integer::parseInt) .collect(Collectors.toList());

이렇게 줄바꿈 하는게 더 보기 좋을 것 같아요~

좋습니당!!!!

Jummi10 · 2021-04-02T06:40:09Z

src/main/java/lotto/domain/lottoticket/NumberOfLottoTicket.java

+    private final static int LOTTO_PRICE = 1000;
+    private final int tickets;
+    private final int money;


정적 필드와 인스턴스 필드를 한 줄 띄어서 구분해주면 좋을 것 같아요

Jummi10 · 2021-04-02T07:00:34Z

src/main/java/lotto/domain/strategy/RandomLottoNumberStrategy.java

+    public List<Integer> getRandomLottoNumbers(){
+        List<Integer> lottoNumberRange = setLottoNumberRange();
+        return getLottoNumbers(lottoNumberRange);
+    }
+
+    private List<Integer> setLottoNumberRange(){
+        ArrayList<Integer> lottoNumberIndex = new ArrayList<>();
+        for (int lottoNumber = LOWER_BOUND; lottoNumber < UPPER_BOUND; lottoNumber++) {
+            lottoNumberIndex.add(lottoNumber);
+        }
+        return lottoNumberIndex;
+    }


lottoNumberRange를 Application에서 for문을 도는 동안 매번 생성을 해주어야 할까요??

죄송해요 ㅠㅠ 저 이해를 못한 것 같아요!!! 1~45 까지의 로또에서 찍을 수 있는 번호들을 하나하나 리스트에서 담아주는 역할인데 매번 생성이 된다는 게 어떤건지 다시 설명해주실수 있으신가요??

private Lottos makeLottos(NumberOfLottoTicket numberOfLottoTicket) { ArrayList<LottoTicket> lottoDummy = new ArrayList<>(); for (int i = 0; i < numberOfLottoTicket.getNumberOfLottoTicket(); i++) { LottoTicket lotto = new LottoTicket(randomLottoNumberStrategy.getRandomLottoNumbers()); lottoDummy.add(lotto); } // ~~ }

Application.makeLottos()에서 randomLottoNumberStrategy.getRandomLottoNumbers()를 로또 티켓 개수만큼 for문을 돌면, 로또 티켓 하나를 만들 때마다 또 for문을 돌면서 1부터 45까지의 번호인 lottoNumberRange를 생성하는 방식이잖아요! 그리고 이렇게 로또 티켓을 만들 때마다 매번 생성된 lottoNumberRange를 셔플시키고 6개씩 잘라서 반환해주면 로또 티켓 하나가 만들어지구요.

제 말은 RandomLottoStrategy가 생성될 때 1~45의 lottoNumberRange를 맨 처음 한 번만 필드로 만들어두고
ApplicationmakeLottos()에서 randomLottoNumberStrategy.getLottoNumbers()만 해도 되지 않을까 라는 말이었습니다..!

글로 쓰려니 전달이 어렵긴 하네요 😥 혹시 이해하기 어려우시다면 다시 물어봐주세요!!

아아아아아아아아 진짜 이해했습니다!!! 아아아 감사합니다!!!

이렇게 설명은 썼지만 사실 세훈님이 원래 짜신대로 해도 무방할것 같습니다!! 그냥 코드 보다가 생각이 나서 말씀드린거였습니다!

아 근데 확실히 정미님 말씀이 맞는 것 같아요! 굳이 계속 생성할 필요가 없을것 같아요!!

Jummi10 · 2021-04-02T08:12:01Z

src/main/java/lotto/domain/EnumWinningStatus.java

+import java.util.HashMap;
+import java.util.Map;
+
+public class EnumWinningStatus {


클래스 이름에 Enum이 들어가네요??

Jummi10 · 2021-04-02T08:19:35Z

src/main/java/lotto/domain/lottowinningresult/LottoWinningResult.java

+        return this.lottoWinning;
+


this.를 쓰지 않아도 충분히 표현이 잘 될 것 같고 13번 라인같이 불필요한 라인은 삭제하는게 깔끔할 것 같아요

Jummi10 · 2021-04-02T08:26:34Z

src/main/java/lotto/domain/EnumWinningStatus.java

+            Boolean lottoBonusBallNumber = lottoWinningBonusBallResult.getLottoWinningBonusBallResult().get(i);
+            getWinningLottoTicketPrices(lottoMatchedNumber, lottoBonusBallNumber);
+        }
+        printer.printAllMatchedLottoResult(getMappingLottoWithBonusBall());


비즈니스 로직 사이에 뷰 로직이 들어갔네요!

Jummi10 · 2021-04-02T08:41:20Z

src/main/java/lotto/domain/EnumWinningStatus.java

+    private void makeWinningLottoTicket(
+            int lottoMatchedNumber,
+            int matchedWinningNumberCount,
+            boolean isMatchedBonusBallCount,
+            boolean lottoBonusBallNumber,
+            WinningStatus winningStatus
+    ){
+        if((lottoMatchedNumber == matchedWinningNumberCount) && (isMatchedBonusBallCount == lottoBonusBallNumber)){
+            lottoPrices.add(winningStatus);
+        }
+    }
+
+    private Map<WinningStatus, Integer> getMappingLottoWithBonusBall() {
+        for (WinningStatus key: lottoPrices
+             ) {
+            mappingLottoWithBonusBall.put(key, mappingLottoWithBonusBall.getOrDefault(key, 0)+1);
+        }
+        return mappingLottoWithBonusBall;
+    }


오호 이런 방식으로 if 분기를 구구절절 쓰지 않고 당첨 통계를 낼 수도 있군요 아주 깔끔하네요!!!👍
(parameter와 for문 줄바꿈만 바꿔주면 될 것 같습니다 ㅎㅎ)

저도 이 방식으로 구현하다가 의문이 생겨서 댓글 답니다!

WinningStatus를 보면

SIX_MATCH(6, false, 2000000000), FIVE_MATCH_WITH_BONUS_BALL(5, true, 30000000), FIVE_MATCH(5, false, 1500000), FOUR_MATCH(4, false, 50000), THREE_MATCH(3, false, 5000);

이런 식으로 돼있는데요
혹시 lottoMatchedNumber = 3, isMatchedBonusBallCount = true인 경우에는 어떻게 하나요??
이 경우는 THREE_MATCH가 lottoPrices에 add가 안 되지 않나요??! 🤔

앗 이 부분 다시 한번 해볼게요 ㅠㅠ 제가 지금 다시 엎고 정미님이 리뷰해주신 것을 바탕으로 짜고 있어서요!!!! 빠르게 오늘 해보고 공유를 드릴게요!!

이번에 새롭게 구조를 다시 짜면서 이 부분을 봤는데 체크로직을 따로 두어야 할 것 같아요 ㅠㅠㅠㅠ

Jummi10 · 2021-04-02T08:42:30Z

src/test/java/lotto/domain/LottoTest.java

+    @Test
+    void 로또_객체를_생성한다(){
+        //given
+        List<Integer> numbers = List.of(6,7,8,9,10,11);


Suggested change

List<Integer> numbers = List.of(6,7,8,9,10,11);

List<Integer> numbers = new ArrayList<>(Arrays.asList(6,7,8,9,10,11));

빨간 줄이 뜨네용

oereo added 22 commits April 1, 2021 16:17

docs(guide): 구현할 기능 목록과 예외 처리 목록 작성

d9441fe

feat: Lotto 객체 생성 및 테스트 케이스 작성, NotValidLottoLengthException 예외처리 추가

16b3952

feat: 로또 넘버를 랜덤으로 생성해주는 RandomLottoNumberStrategy class 추가

44d3e1a

feat: RandomLottoNumberStrategy class 생성 및 shuffle을 통해서 중복되지 않는 랜덤숫자 …

a6ef619

…6개를 리스트로 반환하는 method 구현

feat: add business logic

4a1c382

delete: LottoFactory class 에서 사용하지 않는 class 삭제

b6e6587

refactor: Discriminator method에 numberOfLottoTicket 파라미터를 받을 수 있도록 수정

271acd7

feat: LottoFactory class 에서 LottoWinningResult 객체와 LottoWinningBonusB…

c4bfc6e

…allResult 객체를 가져오는 method 구현

refactor: numberOfLottoTicket 이 로또ㅗㄹ 맞았는지에 대한 여부를 체크하기 위해 numberOfLot…

76a2518

…toTicket 값을 파라미터로 넘기도록 수정

feat: 맞춘 로또 티켓의 당첨금액을 리스트로 만드는 WinningLottoTicketPrices method 구현

7c5412b

feat: 메시지를 출력하는 Printer class 생성

ab9d320

feat: receiveLastWeekLottoWinningNumbers method 구현

bdf3cfd

feat: receiverLottoBonusBallNumber method 구현

8190d21

feat: requestLottoBonusBallNumber method 와 printAllLotto method 구현

6d45ba1

refactor: LottoMachine class 에서 convention 에 위배되는 사항들 수정

883d44a

refactor: run() method 구조 변경 및 makeLottos method 분리

114b929

feat: 로또 당첨을 통해 계산되는 이익률을 담당하는 Profile class 추가

2f01632

feat: NumberOfLottoTicket class 안에 getMoney method 구현

8573f20

refactor: 사용하지 않는 모듈 삭제와 구조 변경

be95b6a

feat: printAllMatchedLottoResult method 구현 및 printLottoProfit method 구현

2b89a23

refactor: 기존 Integer type 에서 WinningStatus type 으로 수정하여 map 구성

9e8a76f

docs(guide): class 구조와 method 대한 부분 수정

53ba8db

Jummi10 suggested changes Apr 2, 2021

View reviewed changes

oereo added 7 commits April 3, 2021 22:14

delete: binary 파일 삭제

8fb68d5

refactor: Printer class 안 재정렬

0666ef4

refactor: Receiver class 안 재정렬

121c28a

feat: profit이 1보다 큰지 작은지 여부 파악 method 구현

7db2cfb

docs(refactor): 불필요한 리스트 제거

b71e88a

refactor: 로또 최종 결과값 출력 method 수정 및 Printer class 수정

758faba

feat: out file 제외

0036133

oereo added 2 commits April 4, 2021 00:49

refactor: profit 객체 관련 로직 수정

d94f4d6

refactor: 코드리뷰를 통한 구조 수정

307ecf1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Oereo 과제제출합니다. #3

Oereo 과제제출합니다. #3

oereo commented Apr 2, 2021

Jummi10 left a comment •

edited

Loading

Jummi10 Apr 2, 2021

oereo Apr 3, 2021

Jummi10 Apr 2, 2021

oereo Apr 3, 2021

Jummi10 Apr 2, 2021

oereo Apr 3, 2021

Jummi10 Apr 2, 2021

oereo Apr 3, 2021

Jummi10 Apr 2, 2021

oereo Apr 3, 2021

Jummi10 Apr 3, 2021

oereo Apr 3, 2021

Jummi10 Apr 3, 2021

oereo Apr 3, 2021

Jummi10 Apr 2, 2021

Jummi10 Apr 2, 2021

Jummi10 Apr 2, 2021

Jummi10 Apr 2, 2021

Jummi10 Apr 6, 2021

oereo Apr 8, 2021

oereo Apr 8, 2021

Jummi10 Apr 2, 2021

		private static final String PRINT_FINAL_MATCHED_LOTTO_RESULT_MESSAGE = "%s개 일치 (%s원)- %s개";
		private static final String PRINT_LOTTO_PROFIT_MESSAGE = "총 수익률은 %f입니다.";

		return dividedLastWeekLottoWinningNumbers.stream().map(Integer::parseInt).collect(Collectors.toList());

	List<Integer> numbers = List.of(6,7,8,9,10,11);
	List<Integer> numbers = new ArrayList<>(Arrays.asList(6,7,8,9,10,11));

Oereo 과제제출합니다. #3

Are you sure you want to change the base?

Oereo 과제제출합니다. #3

Conversation

oereo commented Apr 2, 2021

Jummi10 left a comment • 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

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

Jummi10 left a comment •

edited

Loading