Conversation
Rok93
left a comment
Rok93
left a comment
There was a problem hiding this comment.
다훈님 안녕하세요. 블랙잭 미션을 함께하게 된 김경록입니다. 🙇♂️
미션 빠르게 잘 구현해주셨네요! 👍👍
점수 합산에 관련된 로직들 인상 깊게 잘 봤어요. 👍 (여기에 관련해서 피드백 남겨두었어요! 😊)
몇몇 코멘트들 남겨두었으니 확인해서 반영해주세요! 🙏
잘 이해가 안 되거나 어려운 부분은 언제든지 DM 주세요! 😉
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class BlackJack { |
There was a problem hiding this comment.
BlackJack 클래스는 Controller의 용도로 사용하려는 것 같으나 비즈니스 로직이 상당수 존재하는 것 같아요. 🤔
Controller는 상태 값을 가지지 않아야해요. 👀
BlackJackController 클래스와 비즈니스 모델인 BlackJack 클래스로 분리해볼 수 있을 것 같아요. 🤗
| 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 클래스를 바로 상속받았기 때문에 어떤 메서드를 재정의해야 하는지 파악하기 어려운 것 같아요. 🤔
공통으로 사용하는 메서드는 그대로 사용할 수 있게 하되, 하위 구현체에 따라 달라지는 기능들은 추상 메서드로 정의해서 하위 구현체에서는 재정의를 강제하도록 하면 좋지 않을까요? 😃
2 participants
안녕하세요 리뷰어님!
페어 민상님(@minsang0850) 과 함께 페어프로그래밍으로 미션 수행해 과제 제출합니다.
생성자에 최대한 코드를 줄이고, 공통 부분 추출, 작게 만들기 등을 노력하려 했고,
또한, TDD를 적용하려 했습니다.
그 과정에서 작성하고 재작성하는 과정도 종종있었지만, 설계를 생각해보는 시간이었습니다.
그럼 리뷰 잘 부탁드립니다 👍