[2단계 - 로또 구현] 다니(이다은) 미션 제출합니다. #322
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 구현할 기능 목록 수정 - 확인할 프로그래밍 목록 수정
- 구현할 기능 목록 수정 - 확인할 프로그래밍 목록 수정
- 3등 결과도 2등 결과로 반환되는 버그 수정
- 수동으로 구매할 로또 수가 음수인 경우 예외를 발생시키는 기능 구현
- 구입금액으로 살 수 있는 최대 개수를 초과한 경우 예외 발생
- 숫자가 아닌 경우 예외 발생 - 공백인 경우 예외 발생
- 구분자가 콤마(,)가 아닌 경우 예외 발생
- 숫자가 아닌 경우 예외 발생 - 공백인 경우 예외 발생 - 1~45의 범위가 아닌 경우 예외 발생 - 6개보다 적거나 많은 숫자를 입력한 경우 예외 발생 - 중복되는 숫자를 입력한 경우 예외 발생
📚 다니의 학습 로그[Java] 문자열 - 5내용
링크[Java] stream findAny(), findFirst() 차이 - 3내용
링크[Java] HashMap vs EnumMap - 3내용
링크 |
choihz
approved these changes
Feb 27, 2021
There was a problem hiding this comment.
안녕하세요 다니!
2단계를 아주 깔끔하게 잘 구현해 주셨네요. 부족한 점이 많은 것 같다고 하셨는데, 자신감을 가지셔도 돼요! 👏
남겨주신 코멘트를 보고 앞으로는 더 꼼꼼히 리뷰해야겠다는 반성을 하게 되었어요.. 3등이 2등이라니... 😞 그래서 열심히 보았는데 특별히 코멘트를 남길 부분이 없어 보여요~ 2단계 구현보다는 1단계에서 그때는 왜 안 보였지 싶은 사소한 개선사항이 몇 개 있긴 한데 넘어가도 될 것 같습니다 ㅎㅎ 혹시 공부할 만한 게 더 필요하시다면(?) JUnit 5를 이용해 다양한 테스트 케이스로 테스트해봐도 좋을 것 같아요. (참고)
그럼 로또 미션은 여기서 마무리하도록 할게요! 그동안 정말 고생 많으셨고, 다음에 또 만나요~
| private static final String MESSAGE_STATISTICS = "당첨 통계\n--------\n"; | ||
| private static final String STATISTICS_FORMAT = "%d개 일치 (%d원)- %d개"; | ||
| private static final String STATISTICS_BONUS_FORMAT = "%d개 일치, 보너스 볼 일치(%d원) - %d개"; | ||
| private static final String MESSAGE_LOTTO_TICKET_COUNT_FORMAT = "수동으로 %d장, 자동으로 %d개를 구매했습니다."; |
| @@ -4,11 +4,17 @@ | |||
| import java.util.List; | |||
|
|
|||
| public class LottoMachine { | |||
There was a problem hiding this comment.
생성자를 private으로 닫아주면 불필요한 객체 생성을 막을 수 있겠네요~
|
|
||
| public class Statistics { | ||
| private static final Integer INITIAL_COUNT = 0; | ||
|
|
||
| private final Map<Rank, Integer> lottoStatistics = new HashMap<>(); | ||
| private final Map<Rank, Integer> lottoStatistics = new EnumMap<>(Rank.class); |
Comment on lines
+53
to
+60
| private List<LottoTicket> createManualLottoTickets(long manualCount) { | ||
| List<List<Integer>> manualNumbers = inputView.scanManualNumbers(manualCount); | ||
| return LottoMachine.buyManual(manualNumbers); | ||
| } | ||
|
|
||
| private List<LottoTicket> createAutoLottoTickets(long autoCount) { | ||
| return LottoMachine.buyAuto(autoCount); | ||
| } |
| System.out.println(MESSAGE_WINNING_NUMBER); | ||
| return deleteWhiteSpaces(scanner.nextLine()); | ||
| public List<List<Integer>> scanManualNumbers(long manualCount) { | ||
| System.out.println(MESSAGE_MANUAL_NUMBERS); |
| return deleteWhiteSpaces(scanner.nextLine()); | ||
| public List<List<Integer>> scanManualNumbers(long manualCount) { | ||
| System.out.println(MESSAGE_MANUAL_NUMBERS); | ||
| List<List<Integer>> manualNumbers = new ArrayList<>(); |
There was a problem hiding this comment.
여기선 List<List<Integer>>도 나쁘지 않다고 생각해요 👍 (너무 예스맨 같지만 ㅎㅎ)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요, 코니! 다니입니다 🤗
이번 단계도 코드 리뷰 잘 부탁드립니다!
이번 미션을 진행하면서 크게 변화가 있었던 내용들을 먼저 작성하겠습니다!
변경한 내용 1.
2단계를 시작하기 앞서 1단계 피드백과 코드를 살펴봤는데, 몇 개의 로또를 구매했는지 출력하는 기능을 깜빡했더라구요ㅠ 데드라인을 맞춰야겠다는 생각에 급하게 만들어서 이런 실수를 한 것 같습니다...! 그래서 이 부분 먼저 완성시키고 진행했습니다!
변경한 내용 2.
1단계에서는 로또 상금 테스트를 1등, 2등, 상금이 없는 경우만 했습니다. 그런데, 이번에 나머지 케이스도 테스트 해보니까 3등 결과가 2등 결과로 반환되는 버그가 있었습니다. if 문의 조건식을 수정하고 stream에 findAny() 대신 findFirst()로 변경해서 해결했습니다! 이 일로 꼼꼼한 테스트가 정말 중요하다는 걸 느끼게 되었습니다.
변경한 내용 3.
2단계의 요구사항인 로또 수동 구매 기능을 추가 구현했습니다. 수동, 자동 구매 개수 원시값을 포장하기 위해
Quantity객체를 생성했습니다. 코드를 작성하면서 이렇게 하는 게 맞는 건가..? 싶은 부분이 꽤 있었는데요. 제가 궁금했던 점은 바로 아래에 정리해서 질문 드리겠습니다!이어서 궁금했던 점들을 작성하겠습니다!
궁금한 점 1.
로또 수동 구매에서 수동으로 구매할 번호를 입력하는 기능을 구현할 때, View가 Domain에 의존하면 안 되니까 LottoTicket으로 먼저 포장하지 못하고 List 내에
List<Integer>로 각 입력을 저장해서 반환하게 작성했는데요. 이렇게List<List<Integer>>두 번 중첩해서 사용하는 게 괜찮은 방법인가요...?! 가독성이 떨어지는 거 같아 고민되는데.. 혹시 더 좋은 방법이 있을까요??궁금한 점 2.
Controller에서
createLottoTickets메소드를createManualLottoTickets,createAutoLottoTickets두 메소드로 분리해서 작성했습니다. 그런데, 각 메소드의 라인 수가 길지 않아 굳이 분리할 필요가 있었을까? 라는 의문이 생겼습니다. 만약 메소드를 분리하지 않고 함께 작성하면createLottoTickets가 수동 로또 만들기, 자동 로또 만들기 2개의 책임을 가진 것 같아 우선은 분리했는데요. 근데 이름만 보고 생각하면 로또 만들기라는 1개의 책임만 가진 거 같기도 하구요....! 지금처럼 분리한 상태로 두는 게 좋을까요, 아니면 하나로 합치는 게 더 나을까요?? 리뷰어님의 의견이 궁금합니다!!코드를 작성하면서 부족한 점이 너무 많은 것 같아 얼른 리뷰를 받고 싶었습니다!
이번에도 리팩토링이 필요하다 느껴지는 부분이 있다면 부담 없이 전부 지적해주세요!
바쁘신 와중에 귀한 시간 내주셔서 감사합니다 ❣