2주차 - Step3 로또(2등) 리뷰 부탁드립니다. #249
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
young891221
approved these changes
Jun 27, 2019
| public static ResultSheet getResult(List<LottoTicket> lottoTickets, LottoTicket luckyNumber, int bonusNumber) { | ||
| Map<LottoRank, Integer> lottoResultMap = new HashMap<>(); | ||
|
|
||
| for (LottoTicket ticket : lottoTickets) { |
There was a problem hiding this comment.
ResultSheet를 잘 빼주셨네요ㅎㅎ
뭔가 지저분한 느낌은 List lottoTickets도 일급 객체화 시키면 많이 해소될 듯 보입니다.
| if (rank.getPrizeMoney() != SECOND_PLACE_PRIZE) { | ||
| printResultContent(result, rank, MESSAGE_FOR_LOTTO_RESULT); | ||
| } | ||
| //TODO: Q. else 구문을 안쓰면서도 이 부분을 어떻게 개선할 수 있을지 궁금합니다..! |
There was a problem hiding this comment.
가장 쉬운 분리는 메서드 분리입니다. 하지만 depth만 줄이는 방법일듯 합니다.
그래서 클린코드에 자료구조 + 인터페이스로 분리하는 방식이 있는데요.
https://donnaknew.tistory.com/1 여기는 참고글
https://www.youtube.com/watch?v=GYNT7O3rLhU&list=PLuLb6MC4SOvXCRePHrb4e-EYadjZ9KHyH&index=3
여기는 백명석님이 실제로 리펙토링하는 방식을 다룬 동영상입니다. 참고하시면 좋을듯 해요ㅎㅎ
| } | ||
|
|
||
| private boolean isOutOfRange(int number) { | ||
| return number < MINIMUM_LOTTO_NUMBER || number > MAXIMUM_LOTTO_NUMBER; |
There was a problem hiding this comment.
범위를 벗어난 경우에 true를 반환하고 있어서 OR 조건을 사용했습니닷
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.
안녕하세요!
2단계에서 피드백 주신 내용을 반영해서
로또 결과를 ResultSheet이라는 객체로 빼서 구현했습니다.
그런데 로또 2등 구현 로직이 추가되면서
ResultSheet 객체를 생성하는 정적팩토리메서드가 많이 지저분해졌는데,
제대로 개선하지 못한 것 같아 아쉽습니다..
여전히 문제가 많다고 생각됩니다..! 많은 지적과 조언 부탁드리겠습니닷!ㅎㅎ