-
Notifications
You must be signed in to change notification settings - Fork 313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Step3: 블랙잭(딜러) #553
base: shlee4290
Are you sure you want to change the base?
Step3: 블랙잭(딜러) #553
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요!
블랙잭 3단계도 잘 구현해주셨습니다🙂
게임 진행을 Turn으로 구현해주신 부분이 인상 깊었어요.
몇가지 리뷰를 남겼는데 한 번 고민해보시고 다시 리뷰 요청 부탁드립니다👍
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
블랙잭 룰을 잘못 이해하고 있었네요.. 감사합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요!
리뷰한 부분들 잘 반영해주셨습니다👍👍
몇가지 리뷰를 조금 추가했는데 한 번 확인해보시고 다시 리뷰 요청 부탁드립니다🙂
@@ -0,0 +1,18 @@ | |||
package domain | |||
|
|||
object Score { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금은 Score과 관련된 기능을 수행하는 유틸 클래스가 되어버린 것 같아요.
구현하시는 블랙잭 시스템에 Score라는 타입을 새롭게 정의하는 건 어떨까요?
value class Score(private val value: Int) {
init {
// score 제약 조건
}
val isBlackJack: Boolean = this == BLACKJACK_SCORE
companion object {
}
}
val dealer: Dealer | ||
get() = list.filterIsInstance<Dealer>().first() | ||
|
||
val players: Players | ||
get() = Players(list.filterIsInstance<Player>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
게임에 참가하는 딜러는 1명이고 Player는 1명 이상이다
라는 제약을 초기화 단계에서 검증하면 어떨까요?
|
||
fun gamerToHit(): Gamer? { | ||
return list | ||
.firstOrNull { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
firstOrNull은 find와 동일한 기능을 수행해요.
특정 원소를 찾는다는 의미를 잘 드러내주는 find를 사용하면 어떨까요?
override val cards: Cards = Cards(), | ||
initialState: State = State.Hit, | ||
override val name: String = "딜러", | ||
var onHit: (() -> Unit)? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가변 프로퍼티를 외부에 드러내지 않고 클래스 내부에서만 변경가능하게 바꾸면 어떨까요?
추가로 기본값을 넣거나 지연 초기화를 사용하면 non-null 타입으로도 바꿀 수 있을 것 같아요.
fun winners(players: Players): Players { | ||
return players.playersWithMatchedScore(maxScore(players)) | ||
} | ||
|
||
fun losers(players: Players): Players { | ||
return players.playersWithMismatchedScore(maxScore(players)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
딜러와 비기는 경우는 어떻게 될까요?
지금 구조에서는 Dealer와 Players 모두 수정될 것 같아요.
승리/무승부/패배 각각을 함수로 제공하는 대신, 딜러와 플레이어간 승부 결과를 제공해주는 방식으로 바꿔보면 어떨까요?
fun matchWith(players: Players): MatchResults {
// 로직 생략
}
gamers: Gamers, | ||
cardDeck: CardDeck, | ||
changeState: (Turn) -> Unit, | ||
askPlayerWantToStay: ((String) -> Boolean)? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 파라미터들을 Turn의 프로퍼티로 가져가면 어떨까요?
추가로 Turn에 대한 테스트도 작성되면 좋을 것 같습니다🙂
gamers.gamerToHit()?.let { | ||
if (it is Player && askPlayerWantToStay?.invoke(it.name) == true) { | ||
proceedStay(it, changeState) | ||
return | ||
} | ||
proceedHit(it, cardDeck, changeState) | ||
} ?: run { | ||
changeState(FinalTurn) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
범위 지정 함수대신 엘비스 연산자를 사용해 null check를 하면 어떨까요?
gamers.gamerToHit()?.let { | |
if (it is Player && askPlayerWantToStay?.invoke(it.name) == true) { | |
proceedStay(it, changeState) | |
return | |
} | |
proceedHit(it, cardDeck, changeState) | |
} ?: run { | |
changeState(FinalTurn) | |
} | |
val hitGamer = gamers.gamerToHit() ?: run { | |
changeState(FinalTurn) | |
return | |
} | |
if (hitGamer is Player && askPlayerWantToStay?.invoke(hitGamer.name) == true) { | |
proceedStay(hitGamer, changeState) | |
return | |
} | |
proceedHit(hitGamer, cardDeck, changeState) |
안녕하세요
step3 구현했습니다. 리뷰 잘 부탁드립니다~