[블랙잭 step1] 신은정 미션 제출합니다. #5
Conversation
| final CardDeck cardDeck = CardDeck.create(); | ||
| cardDeck.shuffle(); |
There was a problem hiding this comment.
카드덱을 셔플하는 책임을 컨트롤러가 가져야할지는 의문이에요.
CardDeck.ofShuffled()처럼 이미 셔플된 상태의 카드덱을 가져와도 괜찮을 것 같습니다 :)
| import blackjack.view.InputView; | ||
| import blackjack.view.ResultView; |
There was a problem hiding this comment.
도메인 객체가 뷰를 의존하도록 설계한 의도가 궁금합니다!
(뷰를 컨트롤러가 아닌 도메인 레벨에서 의존하도록 한 이유가 있을까요?)
|
|
||
| import java.util.List; | ||
|
|
||
| public class InputView { |
There was a problem hiding this comment.
InputView를 객체가 아닌 정적 메서드만을 갖는 클래스로 설계하신 이유가 궁금합니다 :)
| public static Suit of(final String name) { | ||
| return Arrays.stream(Suit.values()) |
There was a problem hiding this comment.
지금처럼 팩토리 메서드 네이밍을 관례로 사용할 경우,
파라미터가 하나만 있을 때는 of 가 아닌 from을 사용해요 :)
| public String showCardInfo() { | ||
| return rank.getName() + suit.getName(); | ||
| } |
There was a problem hiding this comment.
"카드 숫자 + 카드 문양"을 나타내는 것은 UI로직에 해당해요.
즉, 현재 도메인 객체가 UI로직에 의존하고 있어요.
도메인이 뷰(UI)를 의존할 경우 어떤 추가 비용이 발생할 수 있을지 고민해보시면 좋을 것 같습니다 :)
| public int calculateScore() { | ||
| return cards.calculateScore(); | ||
| } | ||
| public abstract boolean canDrawCard(); |
There was a problem hiding this comment.
(취향 존중합니다.)
추상 클래스의 추상 메서드의 경우 다른 구현부가 있는 메서드보다 위에 위치시키는게 가독성이 더 좋다고 느껴서,
저는 추상 클래스를 설계할 때 인스턴스 변수 -> 생성자 -> 추상 메서드 -> 구현부가 있는 메서드처럼 작성하는 편이에요 :)
| private Name(final String name) { | ||
| validate(name); | ||
| this.name = name; | ||
| } | ||
|
|
||
| public static Name of(final String name) { | ||
| return new Name(name); | ||
| } |
There was a problem hiding this comment.
(팩토리 메서드는 더 생길 수 있으므로) validation을 팩토리가 메서드가 아닌 생성자에 위치시킨 점이 좋네요 👍
| private static void validate(final String name) { | ||
| if (name == null || name.isEmpty()) { | ||
| throw new IllegalArgumentException("이름은 null 이거나 빈 문자열일 수 없습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 메서드는 생성자에서만 호출되므로 static일 필요는 없는 것 같아요!
| @Override | ||
| public boolean canDrawCard() { | ||
| return cards.calculateScore() < PLAYER_DRAW_THRESHOLD; | ||
| } |
There was a problem hiding this comment.
카드를 추가적으로 뽑을 수 있는지에 대한 로직을 다형성을 이용해서 구현한 점이 좋네요 👍👍👍
|
|
||
| private void calculateResultCount(final List<PlayerResult> playerResults) { | ||
| for (final PlayerResult playerResult : playerResults) { | ||
| addCount(Result.reverse(playerResult.getResult())); |
There was a problem hiding this comment.
| addCount(Result.reverse(playerResult.getResult())); | |
| addCount(playerResult.reversed()); | |
| class PlayerResult { | |
| ... | |
| public Result reversed() { | |
| return this.result.reverse() | |
| } | |
| ... | |
| } |
Result.reverse() static 메서드를 인스턴스 메서드로 만들고, DealerResult 객체가 PlayerResult 객체에게 메시지를 보내서 반전된 결과를 가져오도록 하면 어떨까요?
결과를 반전시키기 위해서 Result에 static 메서드가 있음을 DealerResult가 알아야 하는 것과, 알고 있는 playerResult에게 메시지를 보내서 객체간 협력하게 만드는 것은 의미있는 차이가 있다고 봅니다 :)
No description provided.