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

블랙잭 2단계 구현 #615

Open
wants to merge 15 commits into
base: bsgreentea
Choose a base branch
from
Open

Conversation

bsgreentea
Copy link

작업 내역)
1단계 리뷰 반영
블랙잭 게임 구현

리드미를 미리 작성하지 않아서 뒤늦게 기능 리스트를 셋업했습니다. 다음 단계에서는 미리 작성하고 커밋하겠습니다..ㅎㅎ

질문)
테스트 관련 질문이 있습니다!
'y/n으로 카드를 더 뽑을지에 대한 응답을 받는 부분'에 테스트가 있으면 좋을 것 같은데, 막상 테스트할 거리가 없는 것 같아 테스트 코드를 작성하지는 못했습니다. 좋은 테스트 방법이 있을까요? 아니면 테스트 작성이 부적절한 부분일까요?

- Skill 을 sealed interface로 만들고, 스킬 종류가 추가되면 이를 상속하도록 한다.
- DSL을 위한 실습 코드이므로 별도의 코드 분리는 하지 않았음.
- 참여자 입력 메서드 및 초기 카드 배분을 알리는 출력 메서드 구현
- 참여자와 카드를 다루기 위한 Participant, Card 클래스 생성
- 두 장씩 배분했는지 확인하는 테스트 작성
- 게임 시작하며 카드 풀을 먼저 생성하고 두 장씩의 카드를 배분하도록 구현
- 린트 포맷팅
- 한 장씩 나눠주는 케이스에 대한 테스트 코드 작성
- 초기에 모든 참여자에게 두 장씩 나눠주는 케이스는 init에서 처리하므로 접근 제한자를 private으로 변경
- Ace는 1 또는 11로 계산할 수 있다. 10으로 잘못되어 있던 부분을 11로 수정.
- 현재 보유한 카드의 합이 21을 넘지 않는다면 카드를 계속 뽑을 수 있는 상태로 본다.
- Ace는 1 또는 11로 계산할 수 있으나 카드 뽑기 선택을 더 넓게 하기 위해 checkCurrentScore()에서는 1로 처리한다.
- 깜빡해서 늦게나마 작성...
- (고민) 테스트 코드를 작성할 수 있는 부분인지 모르겠음.
- 상황에 따라 Ace의 점수를 1로 사용할지 11로 사용할지 결정하고, 21과 가장 가까운 점수로 계산하는 기능
- 보유한 각 Ace가 1이거나 11인 경우를 모두 확인하고, 가장 적절한 점수를 반환하도록 구현
- 총점이 21보다 작거나 같다면 그 중 가장 큰 값을, 21보다 크다면 그 중 가장 작은 값을 반환
- 참고로 21을 넘어가면 의미가 없음
Copy link
Member

@wisemuji wisemuji left a comment

Choose a reason for hiding this comment

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

안녕하세요 보성님!

2단계 구현을 잘 해주셨습니다 💯 앞으로의 미션을 진행하기 전 구조 상 변경이 필요해보여 피드백을 남겼습니다.
고민해보시면 좋을만한 코멘트를 달았으니 확인해주시면 감사하겠습니다.

궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요! 🙏 💪


companion object {
const val DEFAULT_CARD_COUNTS = 2
const val BEST_SCORE = 21
Copy link
Member

Choose a reason for hiding this comment

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

이 상수값을 들고 있는 책임이 Controller인게 적절할까요?

import blackjack.ui.InputView
import blackjack.ui.ResultView

class BlackJackGame(
Copy link
Member

Choose a reason for hiding this comment

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

Controller 역할을 하고 있는 BlackJackGame 내에서 분리해볼 수 있는 역할/책임이 있을까요?
다시 말해서, Controller를 만약 테스트하지 않는다고 하면(관련 코멘트는 따로 드릴게요) 테스트 범위에서 벗어나는 게임 요구사항이 있을까요?

Comment on lines +55 to +61
private fun pickRandomCards(count: Int): List<Card> {
val pickedCards = cardsPool.shuffled().take(count)
pickedCards.forEach { pickedCard ->
cardsPool.remove(pickedCard)
}
return pickedCards
}
Copy link
Member

Choose a reason for hiding this comment

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

테스트하기 어려운 로직을 어떻게 테스트할 수 있을까요?
혹시 자동차 경주/로또 미션에서는 이런 로직을 어떻게 테스트하셨나요?

enum class CardInfo(
val displayName: String,
val value1: Int,
val value2: Int = value1,
Copy link
Member

Choose a reason for hiding this comment

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

A를 1로 처리할지, 11로 처리할지 구분하는 로직 작성이 이번 미션의 핵심 중 하나입니다!
다른 개발자들도 CardInfo의 value1, value2를 보고 각각 어떤 값인지 이해할 수 있을까요?

enum class CardType(
val displayName: String,
) {
Diamond("다이아몬드"),
Copy link
Member

Choose a reason for hiding this comment

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

만약 뷰 요구사항이 수정되어 "Diamond"로 출력해야 한다고 가정해봅시다!
뷰 요구사항이 수정되었는데 Model이 변경되는게 어색하지 않으신가요?
자동차 경주 미션 5단계의 MVC 패턴을 참고하여 domain 로직과 view 로직의 의존 방향을 구분해보면 어떨까요?

val scoreWithoutAce = cards.filter { it.info != CardInfo.Ace }.sumOf { it.info.value1 }
val countOfAce = cards.count { it.info == CardInfo.Ace }
var bestScore = scoreWithoutAce + countOfAce * CardInfo.Ace.value1
(0 until countOfAce).forEach { countOfScoreOneAce ->
Copy link
Member

Choose a reason for hiding this comment

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

repeat을 사용해보면 어떨까요?

Comment on lines +99 to +102
data class SoftSkill(val name: String) : Skill
data class HardSkill(val name: String) : Skill
}
Copy link
Member

Choose a reason for hiding this comment

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

sealed interface를 잘 사용해주셨습니다! enum과 어떤 점이 다른지 눈치채셨나요?
참고: https://blog.kotlin-academy.com/enum-vs-sealed-class-which-one-to-choose-dc92ce7a4df5

앞으로 블랙잭 미션을 진행하면서 활용해볼 곳이 있는지 고민해보시면 좋을 것 같아요!

}
}

fun isPossibleToAllocation() = cardsPool.isNotEmpty()
Copy link
Member

Choose a reason for hiding this comment

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

테스트에서만 활용되는 함수네요.

테스트에서만 사용되는 로직을 위해 비즈니스 로직을 public으로 변경하는것은 좋지 않습니다. 가독성의 이유만으로 분리한 private 함수의 경우 public으로도 검증 가능하다고 여길 수 있으나 가독성 이상의 역할을 하는 경우 testable하게 구현하기 위해서는 클래스 분리를 할 시점은 아닐지 고민해볼 수 있습니다. 🙂

Copy link
Member

Choose a reason for hiding this comment

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

현재 블랙잭 게임에 컨트롤러 테스트만 존재하네요!

이러한 테스트를 각각 의미 있는 단위의 객체로 나누어 각자 테스트하면 어떨까요?
객체의 역할과 책임을 나누어 스스로 일하도록 해보세요!

팁: 단위 테스트의 핵심은 테스트하기 어려운 영역과 테스트하기 쉬운 영역을 분리하는 것입니다.

Copy link
Member

Choose a reason for hiding this comment

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

'y/n으로 카드를 더 뽑을지에 대한 응답을 받는 부분'에 테스트가 있으면 좋을 것 같은데, 막상 테스트할 거리가 없는 것 같아 테스트 코드를 작성하지는 못했습니다. 좋은 테스트 방법이 있을까요? 아니면 테스트 작성이 부적절한 부분일까요?

"y/n으로 카드를 더 뽑을지"는 뷰 요구사항이 포함된 것으로 보여집니다. 뷰 요구사항은 (미션에 의하면) 테스트 영역에서 제외됩니다. 만약 의미있는 단위로 객체를 나눠보면, 카드를 더 뽑을지 말지 결정하는 역할과 책임을 어떠한 객체에게 부여하고 싶어질거예요. 그 로직을 테스트하면 됩니다!

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.

None yet

2 participants