3단계 - 블랙잭(딜러) #566
3단계 - 블랙잭(딜러) #566
Conversation
딜러의 점수가 17점 이상이면 추가로 카드를 받을 수 없도록 구현
|
단계가 많이 남진 않아서 나머지까지 리뷰해드리겠습니다. 다만 리뷰는 이전처럼 빠르겐 어렵고 일주일에 한두번정도 봐드릴 수 있을 것 같습니다. |
| } | ||
|
|
||
| private fun dealOutAdditionalCards(dealer: Dealer, players: Players) { | ||
| private fun dealOutAdditionalCards(distributor: Distributor, players: Players) { | ||
| players.players.forEach { |
There was a problem hiding this comment.
일급 컬렉션 형태로 묶었을 때 players. players와 같이 사용되는 부분이 조금 어색하실텐데요.
불변 컬렉션을 사용한다면 Kotlin delegation을 적용하거나, 메서드를 추출하고 컬렉션 인터페이스의 메서드(forEach)를 숨기는 형태로 변경해보시면 좋겠습니다.
아마 지금은 dealOutAdditionalCard 같은 부분에 입출력 로직이 포함되어 있으니 delegation을 적용하시는 편이 조금 더 나을 수 있겠네요.
| val receiveNewCard = dealer.getScore() < Dealer.LIMIT_SCORE | ||
| if (receiveNewCard) { | ||
| distributor.dealOutCard(dealer) | ||
| } |
There was a problem hiding this comment.
점수를 가져온 뒤 BlackJackGame과 같은 입출력이 포함된 부분에서 비교 후 Boolean 값을 추출하기 보다는 해당 상태를 딜러 클래스에서 바로 가져올 수 있도록 구현하면 어떨까요?
| val receiveNewCard = dealer.getScore() < Dealer.LIMIT_SCORE | |
| if (receiveNewCard) { | |
| distributor.dealOutCard(dealer) | |
| } | |
| if (dealer.isReceiveNewCard) { | |
| distributor.dealOutCard(dealer) | |
| } |
| private fun takeAnotherCard(dealer: Distributor, player: Player) { | ||
| if (player.getScore() < MAX_SCORE) { |
There was a problem hiding this comment.
여기도 위에 남긴 코멘트와 동일한 의견입니다. 점수를 꺼내 비교하기보단 해당 행위 자체가 메서드로 추출되면 가독성 측면 등 여러 이점이 있을 것 같아요.
| override fun getGameResult(win: Boolean) { | ||
| gameResults.add(if (win) GameResult.WIN else GameResult.LOSE) | ||
| } |
There was a problem hiding this comment.
getxxx와 같은 메서드 네이밍은 호출 시 데이터를 반환할 것이라 기대할 것 같은데요. 지금은 getGameResult 메서드는 내부 가변 컬렉션에 데이터를 추가해주는 반환값이 없는 사이드 이펙트를 유발하는 메서드 형태로 보이네요.
이 부분과 관련해서 저는 아래 정도의 의견이 있습니다.
- 이 형태를 유지할거라면 메서드 이름을 변경한다.
- 가변 컬렉션을 활용하는게 아니라 게임 결과 연산을 매번 수행하고 내부 상태에 맞는 결과를 반환한다.
| object GameResultCalculator { | ||
| // 딜러와 각 플레이어의 점수를 비교해서 승패를 판별하고 기록함 | ||
| // 딜러가 21을 초과하면 그 시점까지 남아 있던 플레이어들은 가지고 있는 패에 상관 없이 승리한다 | ||
| fun getResult(dealer: Dealer, players: Players) { |
| import blackjack.domain.card.CardDeck | ||
| import kotlin.random.Random | ||
|
|
||
| class Distributor( |
There was a problem hiding this comment.
CardDeck에 테스트를 작성해주시긴 했지만 이 클래스를 호출한 뒤 dealer나 players에 카드가 정상적으로 분배 되었는지 등을 테스트해볼 수 있을 것 같습니다.
리뷰어님 안녕하세요, 과정 마감일은 지났지만 혹시 남은 두 스텝도 리뷰 해주신다면 감사하겠습니다..!!🙇🏻♂️ 완주하고싶어요..