[정다훈] 연료 주입, 블랙잭 리뷰 요청드립니다. #33
Open
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
Rok93
requested changes
Feb 18, 2022
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class BlackJack { |
There was a problem hiding this comment.
BlackJack 클래스는 Controller의 용도로 사용하려는 것 같으나 비즈니스 로직이 상당수 존재하는 것 같아요. 🤔
Controller는 상태 값을 가지지 않아야해요. 👀
BlackJackController 클래스와 비즈니스 모델인 BlackJack 클래스로 분리해볼 수 있을 것 같아요. 🤗
Comment on lines
+33
to
+44
| private static void validateUserNames(String userNames) { | ||
| if (userNames == null || userNames.trim().isEmpty()) { | ||
| throw new IllegalArgumentException("[ERROR] 이름을 입력해 주세요"); | ||
| } | ||
| } | ||
|
|
||
| private static void validateDuplicate(List<String> splitUserNames) { | ||
| Set<String> removeDuplicate = new HashSet<>(splitUserNames); | ||
| if (removeDuplicate.size() != splitUserNames.size()) { | ||
| throw new IllegalArgumentException("[ERROR] 플레이어 이름은 중복될 수 없습니다"); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 유효성 검증 로직은 블랙잭 도메인 모델에 꼭 필요한 유효성 검증인 것 같아요. 👀
먼저 Player의 상태 값인 원시 값 name을 포장해보면 어떨까요? 🤗
| import blackjack.domain.report.GameResult; | ||
| import blackjack.domain.report.GameReport; | ||
|
|
||
| public class Dealer extends Player { |
There was a problem hiding this comment.
Dealer가 온전한 Player 클래스를 바로 상속받았기 때문에 어떤 메서드를 재정의해야 하는지 파악하기 어려운 것 같아요. 🤔
공통으로 사용하는 메서드는 그대로 사용할 수 있게 하되, 하위 구현체에 따라 달라지는 기능들은 추상 메서드로 정의해서 하위 구현체에서는 재정의를 강제하도록 하면 좋지 않을까요? 😃
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.
안녕하세요 리뷰어님!
페어 민상님(@minsang0850) 과 함께 페어프로그래밍으로 미션 수행해 과제 제출합니다.
생성자에 최대한 코드를 줄이고, 공통 부분 추출, 작게 만들기 등을 노력하려 했고,
또한, TDD를 적용하려 했습니다.
그 과정에서 작성하고 재작성하는 과정도 종종있었지만, 설계를 생각해보는 시간이었습니다.
그럼 리뷰 잘 부탁드립니다 👍