Open
Conversation
Hyeon9mak
requested changes
Apr 6, 2025
Member
Hyeon9mak
left a comment
Hyeon9mak
left a comment
There was a problem hiding this comment.
안녕하세요 유라님! 마지막 단계도 잘 진행해주셨습니다 👍
PR 에 구현해주신 내용을 이목요연하게 정리해주신 덕분에 코드리뷰가 매우 편리하네요 😄
동작 상 조금 더 보완해볼 부분이 보이는데요,
이미 구현역량이 뛰어나신 분이라 따로 피드백은 생략하겠습니다 😄
궁금하신 점 편하게 질문주세요!
Comment on lines
+33
to
36
| private void validateLottoNumbers() { | ||
| if (lottoNumbers.size() > LOTTO_NUM_COUNT) { | ||
| throw new IllegalArgumentException("로또 번호는 6개여야 합니다."); | ||
| } |
Member
There was a problem hiding this comment.
별건 아니지만! 이런 경우엔 lottoNumber.size() != LOTTO_NUM_COUNT 가 그대로 유지되어도 괜찮을거 같아요 😄
Comment on lines
-32
to
39
|
|
||
| Set<Integer> uniqueNumbers = new HashSet<>(numbers); | ||
| if (uniqueNumbers.size() != LOTTO_NUM_COUNT) { | ||
| if (lottoNumbers.size() < LOTTO_NUM_COUNT) { | ||
| throw new IllegalArgumentException("로또 번호는 중복될 수 없습니다."); |
Comment on lines
+6
to
+20
|
|
||
| public class LottoNumber { | ||
|
|
||
| static final int LOTTO_MIN_NUM = 1; | ||
| static final int LOTTO_MAX_NUM = 45; | ||
|
|
||
| public static final Map<Integer, LottoNumber> lottoNumberMap = IntStream.rangeClosed(LOTTO_MIN_NUM, LOTTO_MAX_NUM) | ||
| .boxed() | ||
| .collect(HashMap::new, (map, number) -> map.put(number, new LottoNumber(number)), HashMap::putAll); | ||
|
|
||
| private final int number; | ||
|
|
||
| private LottoNumber(int number) { | ||
| this.number = number; | ||
| } |
Member
There was a problem hiding this comment.
LottoNumber vo 가 생겼고, 이에 대해 캐싱해주셨군요 👍
보통 LottoNumber 에 대한 캐싱 값이 어딨는지 알고 싶을 땐 LottoNumber 클래스파일을 먼저 열어볼테니, 좋은 코드 배치라고 생각합니다 😄 👍
Comment on lines
+9
to
+10
| static final int LOTTO_MIN_NUM = 1; | ||
| static final int LOTTO_MAX_NUM = 45; |
Member
There was a problem hiding this comment.
접근 제한자를 default(package private) 을 선택해주셨네요!
요렇게 접근제한자를 선택하신 이유가 궁금해요!
Comment on lines
+9
to
+25
|
|
||
| @Test | ||
| void of_1에서_45사이_숫자면_같은_객체_반환한다() { | ||
| assertThat(LottoNumber.of(2)).isEqualTo(LottoNumber.of(2)); | ||
| } | ||
|
|
||
| @Test | ||
| void of_1미만일_경우_예외를_던진다() { | ||
| assertThatThrownBy(() -> LottoNumber.of(0)) | ||
| .isInstanceOf(IllegalArgumentException.class); | ||
| } | ||
|
|
||
| @Test | ||
| void of_45초과일_경우_예외를_던진다() { | ||
| assertThatThrownBy(() -> LottoNumber.of(46)) | ||
| .isInstanceOf(IllegalArgumentException.class); | ||
| } |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
안녕하세요.
수정 사항 요약
Lotto가 List 대신 Set을 가지도록 수정
ㄴ 2단계에서 받은 피드백을 반영
LottoNumber 클래스를 새로 추가
ㄴ 3단계까지는 자동 로또만 존재했기 때문에 1~45 숫자 캐싱은 LottoNumberGenerator 내에서만 관리했음.
ㄴ 그러나 수동 로또 입력이 도입되면서 숫자 유효성 검증과 관리가 분리되면 좋겠다고 판단해, LottoNumber 클래스를 별도로 분리하고 정적 캐싱 방식으로 구현
2등 당첨 로직 표현 방식을 개선
ㄴ 3단계에서 피드백 받은 2등 당첨의 표현방식이 다른 것을 적용
입력 버퍼 관련 문제를 해결하기 위해 readInt() 메서드를 추가
ㄴ 기존의 nextInt()는 숫자 뒤의 \n이 버퍼에 남아 있어 이후 입력에 영향을 주는 문제가 있었음.
ㄴ이를 해결하기 위해 readInt() 메서드를 만들어 nextLine()과 함께 처리.
감사합니다 :)