Conversation
kilian-develop
left a comment
kilian-develop
left a comment
There was a problem hiding this comment.
코드 잘봤습니다~ 마지막 미션 이네용!!
민정님 수고 많으셨습니다...!
next-step 같이 화이팅 해요!
|
|
||
| public class Application { | ||
|
|
||
| public static void main(String[] args) { |
There was a problem hiding this comment.
main() 에 너무 많은 책임이 있는 것 같아요!
비즈니스 로직이 계속 추가 된다면 main이 너무 커질 것 같아요!
작은 단위로 어떻게 분리할 수 있을지 고민해보시면 좋을 것 같습니다.
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class GameStarter { |
There was a problem hiding this comment.
GameStarter라는 클래스에 플레이어 생성, 딜러 생성, 카드 생성과 같이 많은 책임이 있지 않나 생각이 듭니다.
블랙잭 게임 시작전에 과정이 많아지면 이 클래스도 점점 뚱뚱해지지 않을까 라는 생각이 있어요!
각 책임별로 클래스를 분리해서 설계해봐도 좋지 않을까 생각이 듭니다!
저와는 의견이 다르시다면 말씀해주세요!
|
|
||
| GameStatus gameStatus = new GameStatus(players, dealer, cardSet); | ||
|
|
||
| gameStarter.dealFirstTurn(gameStatus); |
There was a problem hiding this comment.
dealFirstTurn이 GameStatus를 리턴하는데 리턴한 객체를 아무데서도 받질 않고 있어요!
이러면 이 함수를 수행한 의미가 없어질 것 같아요!
| static int MAX_DEALER_CARD_NUMBER_SUM = 16; | ||
| static int GAME_STANDARD_WINNING_NUMBER = 21; |
There was a problem hiding this comment.
private static final int로 정의하는 게 좋아보여요!
관련 블로그 글 올려드려요!
블로그
| List<Player> players; | ||
| Dealer dealer; | ||
| CardSet cardSet; |
There was a problem hiding this comment.
혹시 private으로 선언하시지 않은 이유가 있으실까요??
| public Card(Denomination number, Suit suit) { | ||
| this.number = number; | ||
| this.suit = suit; | ||
| } |
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class Card { |
There was a problem hiding this comment.
도메인 객체들이 다 불변으로 되어있지 않은 것 같아요!
불변 객체의 장점과 관련된 블로그 링크 걸어드립니다.
혹시 일부러 불변으로 하시지 않았다면 알려주시면 감사하겠습니다.
| } | ||
|
|
||
| public List<Card> getCards() { | ||
| return cards; |
There was a problem hiding this comment.
list를 그대로 리턴 하면 원본 list가 변경되서 발생할 수 있는 문제들이 있을 것 같아요!
불변 리스트로 리턴 해주시거나 List.copyof로 복사본을 리턴해주시면 더 좋을 것 같아요!
| @@ -0,0 +1,9 @@ | |||
| public class WinningStatusPlayer { | |||
| Player player; | |||
| Boolean winner; | |||
There was a problem hiding this comment.
Boolean을 Wrapper class로 생성하신 이유가 있으실까요?
Wrapper 클래스로 사용하면 true, false 이외에 null 값이 들어올 수 있을 것 같아요
외에도 primitive 사용하면 좋은 이유 블로그 남겨드립니다.
생각이 다르다면 말씀해주세요!
| import java.util.List; | ||
|
|
||
| public abstract class CardHolder { | ||
| protected List<Card> cards; |
There was a problem hiding this comment.
이 부분도 일급컬렉션을 사용하면 더 좋지 않을까 생각듭니다!
4 participants
카드가 한 세트가 있다는 것을 모르고(?) 착각하고(?)
미션을 진행하다가 카드 뭉치가 필요하다는 것을 깨닫고 중간에 대대적으로 코드를 수정하였습니다.
수정한 것 이후부터 보시면 되겠습니다.
테스트는 리뷰 기간에 작성하도록 하겠습니다.