feat: 4단계 로또 수동 #2816
Merged
feat: 4단계 로또 수동 #2816
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
JunHoPark93
requested changes
Nov 14, 2022
src/main/java/constant/Rank.java
Outdated
Comment on lines
17
to
32
| public static Rank findRank(int countOfMatchedNumber) { | ||
| switch (countOfMatchedNumber) { | ||
| case 6: | ||
| return Rank.FIRST; | ||
| case 5: | ||
| return Rank.SECOND; | ||
| case 7: | ||
| return Rank.SECOND_WITH_BONUS; | ||
| case 4: | ||
| return Rank.THIRD; | ||
| case 3: | ||
| return Rank.FOURTH; | ||
| default: | ||
| return Rank.UNKNOWN; | ||
| } | ||
| } |
There was a problem hiding this comment.
enum을 활용해주셨는데요, case문을 없앨수있도록 만들어보면 어떨까요?
|
|
||
| SIEVE.addAll(ticketNumbers); | ||
|
|
||
| if (SIEVE.size() != ticketNumbers.size()) throw new IllegalArgumentException("복권번호가 중복됩니다."); |
Comment on lines
9
to
10
| private final List<LotteryNumber> ticketNumbers; | ||
| private final Set<LotteryNumber> SIEVE = new HashSet<>(); |
There was a problem hiding this comment.
둘 다 유지될 필요가 있을까요? 불필요한 인스턴스 변수를 제거해보아요!
💡 규칙 8: 일급 콜렉션을 쓴다.
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class LotteryNumber { |
There was a problem hiding this comment.
도메인들을 잘 추가해주셨어요! 추가로 테스트는 꼭 만들어주셔요 🙏
| import java.util.Objects; | ||
|
|
||
| public class LotteryNumber { | ||
| private final int MIN_NUMBER = 1; |
There was a problem hiding this comment.
Suggested change
| private final int MIN_NUMBER = 1; | |
| private static final int MIN_NUMBER = 1; |
Comment on lines
24
to
41
| firstPrizeLotteryTicket = LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(1), LotteryNumber.of(2), LotteryNumber.of(3), | ||
| LotteryNumber.of(4), LotteryNumber.of(5), LotteryNumber.of(6))); | ||
| secondPrizeLotteryTicket = LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(1), LotteryNumber.of(2), LotteryNumber.of(3), | ||
| LotteryNumber.of(4), LotteryNumber.of(6), LotteryNumber.of(10))); | ||
| secondPrizeWithBonusLotteryTicket = LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(1), LotteryNumber.of(2), LotteryNumber.of(3), | ||
| LotteryNumber.of(4), LotteryNumber.of(6), LotteryNumber.of(11))); | ||
| noPrizeLotteryTicket = LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(11), LotteryNumber.of(12), LotteryNumber.of(13), | ||
| LotteryNumber.of(14), LotteryNumber.of(15), LotteryNumber.of(17))); | ||
| matchedWinnerLotteryTicket = new WinnerLotteryTicket(LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(1), LotteryNumber.of(2), LotteryNumber.of(3), | ||
| LotteryNumber.of(4), LotteryNumber.of(5), LotteryNumber.of(6))), LotteryNumber.of(11)); | ||
| unmatchedWinnerLotteryTicket = new WinnerLotteryTicket(LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(1), LotteryNumber.of(2), LotteryNumber.of(3), | ||
| LotteryNumber.of(4), LotteryNumber.of(5), LotteryNumber.of(6))), LotteryNumber.of(7)); |
JunHoPark93
approved these changes
Nov 17, 2022
| this.reward = reward; | ||
| } | ||
|
|
||
| public static Rank findRank(int countOfMatchedNumber) { |
Comment on lines
+8
to
+10
| private final List<LotteryNumber> ticketNumbers; | ||
|
|
||
| public LotteryTicket(List<Integer> ticketNumbers) { | ||
| private static final int COUNT_OF_LOTTERY_NUMBERS = 6; |
There was a problem hiding this comment.
불필요한 변수를 잘 제거해주셨습니다!
다만 8,10라인 선언 위치만 변경부탁드려요 :)
Comment on lines
+73
to
+75
| LotteryTicket secondPrizeLotteryTicket = LotteryTicket.of(Arrays.asList( | ||
| LotteryNumber.of(1), LotteryNumber.of(2), LotteryNumber.of(3), | ||
| LotteryNumber.of(4), LotteryNumber.of(6), LotteryNumber.of(10))); |
There was a problem hiding this comment.
더 나아가면 숫자만 전달하면 LotteryTicket이 나오는 fixture메서드를 만들 수도 있겠네요!
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.
#2810 이슈를 해결하고자 작업하였습니다.
❗3단계 PR에서 남겨주신 코멘트도 반영하였습니다. 한번 확인 부탁드려요 🙇♀️