Skip to content
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

Step2 - 블랙잭 미션 #815

Open
wants to merge 7 commits into
base: goodbyeyo
Choose a base branch
from

Conversation

goodbyeyo
Copy link

안녕하세요 🙂

블랙잭 미션 피드백 부탁드립니다. 🙏

기능을 구현하면서 BlackJackGame.handlePlayerDraw () 함수가
View 출력기능도 담당하고 있어서 분리해서 테스트 하기가 �까다로웠는데...
혹시 단위 테스트를 추가할 수 있는 좋은 방법이 있을까요?

Copy link
Member

@malibinYun malibinYun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2단계 미션 고생 많으셨어요!
생각해볼만한 곳에 피드백 남겨두었어요 :)
피드백 반영 후 다시 리뷰 요청 부탁드려요!

Comment on lines +5 to +6

class UserCards(private val cards: MutableList<Card>) : Collection<Card> by cards {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래 코드를 실행시키면 어떤 일이 발생할 수 있을까요?
방어적 복사와 읽기 전용 List 를 활용해보시면 좋겠어요!

val mutableList = mutableListOf()
val cards = UserCards(mutableList)

mutableList.clear()

Comment on lines +22 to +37
fun handlePlayerDraw(
player: Player,
gameCards: GameCards,
) {
val previousCardCount = player.cardSize()
InputView.enterIsContinueDrawCard(player)
if (player.isDrawContinue) {
player.receiveCard(gameCards.drawCard())
OutputView.printPlayerCard(player)
handlePlayerDraw(player, gameCards) // 재귀 호출
}

if (!player.isDrawContinue && player.cardSize() == previousCardCount) {
OutputView.printPlayerCard(player)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기능을 구현하면서 BlackJackGame.handlePlayerDraw () 함수가
View 출력기능도 담당하고 있어서 분리해서 테스트 하기가 �까다로웠는데...
혹시 단위 테스트를 추가할 수 있는 좋은 방법이 있을까요?

View에서 입력을 받는 부분을 람다로 처리해서 외부에서 주입받는 형태로 구성해보는 건 어떨까요?
람다의 활용이 어려우시다면, interface를 만들어서, ConsoleView에 대한 입력 구현체를 주입받는 형태로 구성해볼 수 있겠어요!

Comment on lines +2 to +3

data class Card(val rank: Ranks, val suits: Suits)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

카드의 종류는 총 몇가지가 존재할 수 있을까요?
똑같은 카드를 여러번 생성할 수도 있겠지만, 동일한 카드는 인스턴스를 재활용해보는 건 어떨까요?

Comment on lines +15 to +21
companion object {
fun create(): GameCards {
val allCards =
Suits.entries.flatMap { suit ->
Ranks.entries.map { rank -> Card(rank, suit) }
}
return GameCards(LinkedList(allCards.shuffled()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

랜덤 요소 때문에 테스트가 어려워보여요.
랜덤의 요소를 외부에서부터 주입받아보는 건 어떨까요?
그렇다면 정확히 어떤 카드들로 순서가 구성되어있는지 개발자가 제어하는 환경을 만들어 테스트 가능할 거라 생각해요 :)

Comment on lines +4 to +5

class Player(val name: String, var isDrawContinue: Boolean = true) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDrawContinue 변수를 외부에서도 변경 가능한 형태예요.
외부에서는 변경하지 못하도록 읽기 전용 프로퍼티로 만들어보는 건 어떨까요?

Comment on lines +10 to +12

fun create(playerNames: String): Players {
val players = playerNames.split(DELIMITER).map { playerName -> Player(playerName) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문자열을 파싱하는 행위는 View의 일이라고 생각해요.
뷰와 도메인 로직을 분리해보는 건 어떨까요?

Comment on lines +6 to +7
) {
ACE("A", listOf(1, 11)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACE의 기본 점수는 1이고, 특수한 상황에서 10점이 추가되는 것이니, 이것을 로직으로 표현해보는 건 어떨까요?
점수를 계산하는 곳에서 ACE에 10점을 추가할지 말지 결정해보면 좋겠어요!

Comment on lines +3 to +4
enum class Ranks(
val keyword: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

카드 끝수를 나타내는 이 문자열은 ConsoleView에 종속적인 값이라고 생각해요.
도메인에서 뷰 요소를 분리해보는 건 어떨까요?

Comment on lines +15 to +21
fun enterIsContinueDrawCard(player: Player) {
val userInput = getIsContinueDraw(player)
when (userInput.lowercase()) {
"n" -> player.stopCardDraw()
"y" -> player.continueCardDraw()
else -> println("잘못된 입력입니다. 다시 입력해주세요.")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

플레이어의 게임 진행 자체를 실행하는 로직은 도메인로직이라 생각해요.
View에서는 n 혹은 y 가 입력되었는지 받은 뒤, Boolean등을 반환해서, 도메인에서는 반환 받은 값을 가지고 player에게 메시지를 던지도록 구성해보는 건 어떨까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants