[알트] 블랙잭 미션 제출합니다. #2
Open
Conversation
This file contains 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
* NotFinishedState와 FinishedState로 구분 * 기존에 있던 Factory 클래스는 ReadyState로 대체
hongbin-dev
reviewed
Jul 3, 2020
Comment on lines
+47
to
+54
| List<Name> names = Name.fromComma(namesWithComma); | ||
| List<BettingMoney> bettingMonies = new ArrayList<>(); | ||
| for (final Name name : names) { | ||
| int money = inputView.inputBettingMoney(name); | ||
| BettingMoney bettingMoney = new BettingMoney(money); | ||
| bettingMonies.add(bettingMoney); | ||
| } | ||
| return playerService.createPlayers(names, bettingMonies); |
There was a problem hiding this comment.
List와 List를 매치시켜서 createPlayers() 해주고 있는데요. 각 List의 요소가 1:1로 매치되는데, Map자료구조를 활용하는건 어떨까요?
Comment on lines
+61
to
+65
| for (final Player player : players) { | ||
| drawCardToPlayer(cardDeck, player); | ||
| } | ||
| drawCardToDealer(cardDeck, dealer); | ||
| } |
There was a problem hiding this comment.
Suggested change
| for (final Player player : players) { | |
| drawCardToPlayer(cardDeck, player); | |
| } | |
| drawCardToDealer(cardDeck, dealer); | |
| } | |
| for (final Player player : players) { | |
| drawCardToPlayer(cardDeck, player); | |
| } | |
| } |
AllPlayers라는 객체가 있으면, 상속을 극대화하여 사용할 수 있을 것 같은데요
어려울까요..?
| } | ||
|
|
||
| private void drawCardToDealer(final CardDeck cardDeck, final Dealer dealer) { | ||
| while (!dealer.isFinished()) { |
There was a problem hiding this comment.
isFinished 메소드를 순수하게 사용하는 곳은 없는것 같은데요.
isNotFinished 로 변경해도 좋을 것 같아요.
Comment on lines
+16
to
+27
| public Name(String name) { | ||
| Objects.requireNonNull(name, "이름이 null입니다."); | ||
| name = name.trim(); | ||
| validateLength(name); | ||
| this.name = name; | ||
| } | ||
|
|
||
| public static List<Name> fromComma(final String names) { | ||
| return Stream.of(names.split(NAME_DELIMITER)) | ||
| .map(Name::new) | ||
| .collect(toList()); | ||
| } |
There was a problem hiding this comment.
딜러를 만들때만 생성자를 사용하는데요.
생성자의 접근지정자의 수준을 높히고, 딜러를 만드는 static 메서드를 만드는건 어떨까요?
Comment on lines
+21
to
+23
| if (cards.isEmpty()) { | ||
| throw new EmptyCardDeckException("카드를 모두 소모하여 더 뽑을 수 없습니다."); | ||
| } |
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.
블랙잭 끔찍하네요...
앞서 두 미션에서 dto를 만들어야 하는 명분을 찾지 못해 이번 미션에서는 만들지 않았습니다.
디미터 법칙은 하늘나라로 갔습니다👼
컨트롤러에 필드변수가 많은건 IoC 컨테이너가 없어서 그런것이니 너그러이 봐주시길 부탁드립니다...
service layer는 테스트 하지 않았습니다. 시간나면 추가하겠습니다...
커밋 주기가 엉망인건 제 탓입니다...
아무튼, 마지막 미션 리뷰도 잘 부탁드립니다🙏