[2단계 - 로또(수동)] 이프(송인봉) 미션 제출합니다. #504
Merged
Conversation
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
choihz
approved these changes
Mar 7, 2022
choihz
left a comment
choihz
left a comment
There was a problem hiding this comment.
안녕하세요, 이프!
수정하신 내용 전부 확인했습니다 👍 수정 부분에서 특별히 피드백을 드릴 내용은 없는 것 같아 바로 머지할게요. 고생 많으셨어요~!
Comment on lines
+15
to
+30
| private Ball(final int number) { | ||
| BallValidator.validateBallNumber(number); | ||
| this.number = number; | ||
| } | ||
|
|
||
| public static Ball from(final int ballNumber) { | ||
| saveBallIfNotExist(ballNumber); | ||
| return balls.get(ballNumber); | ||
| } | ||
|
|
||
| private static void saveBallIfNotExist(final int ballNumber) { | ||
| if (balls.containsKey(ballNumber)) { | ||
| return; | ||
| } | ||
| balls.put(ballNumber, new Ball(ballNumber)); | ||
| } |
| - [x] `수동으로 구매할 로또의 개수` 입력 | ||
| - [x] `로또 개수`는 숫자여야 합니다. 아닌 경우, `IllegalArgumentException` 발생 | ||
| - [x] `로또 개수`는 양수여야 합니다. 아닌 경우, `IllegalArgumentException` 발생 | ||
| - [x] `로또 개수`는 음수가 아니어야 합니다. 아닌 경우, `IllegalArgumentException` 발생 |
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.
안녕하세요 이프입니다!
이전 PR을 Merge 해주셨는데, 아래의 3가지 변경사항이 생겨서 다시 PR 드립니다!
수동으로 구매할 로또의 개수로 0을 입력할 수 없는 문제 해결utils패키지 제거 및domain패키지 세분화 진행BallStorage클래스 제거 및 Ball 클래스 리팩토링기존의 캐싱 방식에 대해서
Ball생성자의 접근제어자가 열려 있기 때문에equals와 hashcode를 오버라이딩 하지 않겠다는 결론에 도달했습니다.
이부분에 대해 고민해본 결과
Ball생성자를 열어둔 이유가BallStorage에서 생성자를 호출하기 위함이라는 것에 주목했고BallStorage가Ball과 분리되어야 하는 것에 의구심을 갖게 되었습니다.결과적으로는,
Ball생성자를 private 접근제어자로 지정함으로써 외부에서 인스턴스를 생성하지 못하도록 차단했고BallStorage에서 행해지던 캐싱 과정을Ball내부로 이동시켰으며BallStorage를 제거했습니다.메서드 배치 순서는 스타일의 차이일 뿐, 정답이 없다고 생각합니다.
하지만 그렇다고 해서, 계속 제 방식을 고집하겠다는 것은 당연히 아닙니다.
이 부분은 앞으로 진행될 다양한 미션 과정에서 페어들과 충분히 얘기나눠보고
더 많은 경험을 쌓아가며 스타일을 확립하겠습니다.
메서드 배치 순서에 대해 좋은 얘기를 나눠주셔서 감사합니다!!