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 - 블랙젝 #672

Merged
merged 54 commits into from
Dec 8, 2023
Merged

Step2 - 블랙젝 #672

merged 54 commits into from
Dec 8, 2023

Conversation

oyj7677
Copy link

@oyj7677 oyj7677 commented Nov 27, 2023

안녕하세요. 너무 늦게 요청 드립니다....

진행하다보니 계속 추가 되고 변경되는 부분이 있어서 오래 걸리게 되었습니다.

input, output은 미완성 했지만 너무 늦어서 구조적인 부분이라도 우선 리뷰 요청드립니다.

메인에서 BlackjackGame을 선언합니다.

메인에서는 hit과 Stand만 선택하면 되는 구성입니다.

hit을 했을때는 현재 유저 카드 합이 21이상이어서 stand상태가 된다면 유저를 변경합니다.

그리고 실례가 안된다면 타 리뷰에 대해서 안드로이드 적으로 설명해주셨으면 합니다... ㅜㅜ

부족하지만... 계속해서 개선해 나가겠습니다.

Docs: 딜러 역할 삭제, 딜러 역할 분배
Refactor: Suit, CardNumber / class -> Enum으로 변경, 카드 무늬, 카드 번호 검증 로직 제거
chore: 카드 관련 클래스 패키지 생성 및 이동
Refactor: 카드팩 object -> class 변경, 사이드 이팩트 수정
Feat: 카드 인덱스 증가, 현재 인덱스 반환 기능 구현
Feat: 카드 인덱스 검증 로직 구현
…드 셔플 기능 역할 변경 (블렉젝 게임 -> 카드팩 )
Refactor: 사용자 이름 추가.

style: CardNumber 컨벤션 수정
Refactor: PlayingCard의 Ace계산 로직 제거
Refactor: 게임 인덱스의 파라미터 maxIndex ->. maxCardIndex로 변경
Docs: 블랙젝 게임 기능 목록 추가.
블랙젝 게임 역할 이동(-> PlayerGroup)
- 참여 인원 검증
- 플레이어 턴 넘김
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 4 to 16
object BlackJackCalculator {

private const val ADDITIONAL_SCORE = 10
private const val NO_ADDITIONAL_SCORE = 0
private const val ACE_ADDITIONAL_SCORE_THRESHOLD = 12

fun calculate(cardList: List<PlayingCard>): Int {
var result = cardList.sumOf { it.getPoint() }
cardList.filter { it.getCardNumber() == CardNumber.ACE }
.forEach { _ -> result += addCalculateAce(result) }

return result
}
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 3 to 10
class CardPack(val cardList: List<PlayingCard>) {

private var cardIndex = 0

fun hit(): PlayingCard {
return cardPack.cardList[cardIndex++]
}

Copy link
Member

Choose a reason for hiding this comment

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

어떤 의도를 담은 객체인지 알기가 어려웠어요 😥

companion object에 대부분의 메서드가 분리되어있는 건 어떤 이유이셨나요?
이 안에 있는 메서드들은 이 객체가 해야할 일이 맞는지 고민해보시면 좋겠어요 :)

또, 52번 카드를 뽑은 경우, out of index exception이 발생할 것으로 예상되는데요,
게임이 모두 끝나지 않은 상태에서 52장의 카드를 모두 소진해서 게임이 비정상 종료되는 것은 좋은 경험이란 생각이 들지 않아요.

또, 블랙잭 게임 룰 중에는 한 번에 여러 벌의 카드를 섞어 진행하기도 합니다.
이런 요구사항에 유연하게 대응할 수 있게 구성해보는 건 어떨까요?

Comment on lines 4 to 5

class PlayingCard(private val suit: Suit, private val cardNumber: CardNumber) {
Copy link
Member

Choose a reason for hiding this comment

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

52장의 각 카드의 인스턴스를 재활용해보는 건 어떨까요?
동일한 카드의 인스턴스를 새로 만들지 못하게 하고 기존에 만들어뒀던 인스턴스를 재활용할 수 있게 캐싱해봐도 좋겠어요 :)

Comment on lines 11 to 13
fun Suit.getName(): String {
return this.koName
}
Copy link
Member

Choose a reason for hiding this comment

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

getName 확장함수는 어떨 때 필요한걸까요?
멤버변수인 koName을 직접 접근하면 안되는 큰 이유가 있었을까요?_?

Comment on lines 14 to 15

fun getSuitList() = suitList
Copy link
Member

Choose a reason for hiding this comment

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

enum의 값을 모두 가져오기 위해서는 values() 메서드를 사용하시면 됩니다 :)

val allSuits = Suit.values()

Comment on lines 23 to 24

fun getName() = name
Copy link
Member

Choose a reason for hiding this comment

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

코틀린에서는 프로퍼티를 활용해서, 따로 getter를 만드는 경우가 드물어요 :)
프로퍼티인 name의 접근 제어자를 공개로 변경해주시면 됩니다.

아래 kotlin property에 대한 공식 문서를 참고해주세요!
https://kotlinlang.org/docs/properties.html#declaring-properties

Comment on lines 8 to 10
private var _status = Status.PLAYING
val status: Status
get() = _status
Copy link
Member

Choose a reason for hiding this comment

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

외부에 공개해야하지만 외부에서는 수정하지 못하게하고, 내부적으로 가변을 유지하고싶을 때는 아래 처럼 작성하시면 됩니다 :)

var status: Status
    private set

koltin property 공식 문서를 꼭 읽어보세요!

Comment on lines 12 to 14
private var _cardList = mutableListOf<PlayingCard>()
val cardList: List<PlayingCard>
get() = _cardList
Copy link
Member

Choose a reason for hiding this comment

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

카드들을 일급 컬렉션으로 묶어보는 건 어떨까요?

@malibinYun
Copy link
Member

그리고 실례가 안된다면 https://github.com/next-step/kotlin-lotto/pull/985#discussion_r1404879317에 대해서 안드로이드 적으로 설명해주셨으면 합니다... ㅜㅜ

컨트롤러와 뷰의 역할을 메인에서 잘 분리해보라는 말씀인 것으로 이해했어요!
안드로이드에 대입한다면..
Activity에 모든 로직들이 다 담겨있는 형태이니, 이것을 View는 View의 일만 담당하도록 역할을 분리하고, 도메인은 도메인의 역할만을 담당하도록 분리해볼 수 있겠어요.
Activity에서는 View 계층과 도메인 계층을 연결해주는 딱 Controller의 일만 할 수 있게끔, 적절한 연결다리의 역할 계층을 만들어보라는 의미인 것 같아요.

말씀해주신 리뷰에서는 특정 프레임워크가 중요해보이지는 않아보여요!
프레임워크에 따라 다르게 이해될 수 있는 부분이 있을 거라 생각해요.
가장 중심을 두고 생각해봐야할 부분은, Main 함수가 컨트롤러와 UI역할이 섞여있으니, 컨트롤러의 영역을 분리해보는 것 이니 이곳에 초점을 맞춰서 잘 분리해보시면 좋겠어요!
어떤 기준으로 분리하셨는지에 대해 리뷰어분과 소통하시다보면 분명 잘 해내실 수 있으실 거고, 좋은 경험이 될 거라 믿어요 :)

- 클래스명 변경 CardNumber -> CardRank
- CardRank프로퍼티명 일부 변경 cardName -> symbol
- GameIndex, CardPack 제거,
- 사이드 이팩트 수정

Feat:
- Class Cards 추가  (PlayingCard기반 일급 컬랙션)
- CardPack의 카드 저장 역할 마이그래이션 CardPack -> Cards
- 카드덱 인터페이스화(게임 덱, 플레이어 덱)
- 테스트 코드 사이드 이팩트 수정
- 플레이어 상태 추가
- 플레이어 상태 업데이트 조건문 추가.
- BlackJackCalculator삭제
- PlayingCard class -> data class
- class Player 불필요한 함수 제거
- class Player 변수 변경 cardList -> playerDeck
chore: 플레이어 수 검증 예외 메시지 수정, InputView 매직 스트링 제거 및 함수 네이밍 변경
Refactor: BlackjackGameTest 삭제
@oyj7677
Copy link
Author

oyj7677 commented Dec 2, 2023

게임 진행의 로직이 이전과 많이 달라졌습니다.

BlackjackGame.kt에서 View와 비지니스 로직이 함께 있습니다. 이번 미션의 경우 지속적으로 input과 output이 있는 경우 클래스나 함수의 구성을 어떤 식으로 해야 테스트 코드와 함께 진행될 수 있나요?

deck클래스의 경우 게임 덱(배포 될 카드)과 플레이어 덱(받은 카드) PlayingCard의 컬렉션으로 구성되어 있어서 interface를 만들어 사용했습니다. 이러한 구성이 맞는지 자세히 리뷰 부탁드리겠습니다.

오랜 시간 걸렸지만... 부족함이 많은 것 같습니다. 리뷰로 혼내주시면 감사하겠습니다.

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.

다시 구현하시느라 고생 많으셨어요.
이번 피드백 뿐만 아니라 이전 피드백 내용들도 반영해보시고, 다시 리뷰 요청 부탁드려요!

Comment on lines 24 to 26
val response = InputView.askForHit(player.name)
handleForResponse(response, player)
OutputView.showPlayingCard(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.kt에서 View와 비지니스 로직이 함께 있습니다. 이번 미션의 경우 지속적으로 input과 output이 있는 경우 클래스나 함수의 구성을 어떤 식으로 해야 테스트 코드와 함께 진행될 수 있나요?

View에 대한 interface를 만들어서, console에 대한 구현체를 주입 받아 사용해볼 수 있겠어요 :)

Copy link
Author

@oyj7677 oyj7677 Dec 6, 2023

Choose a reason for hiding this comment

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

InputViewInterface, OutputViewInterface View interface, 생성 하여 BlackjackGame에 생성자로 전달하여 사용해봤습니다.

class BlackjackGame(
private val cardDeck: CardDeck,
val playerGroup: PlayerGroup,
private val inputView: InputViewInterface,
private val outputView: OutputViewInterface,
) 

Copy link
Author

Choose a reason for hiding this comment

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

InputViewInterface에선
fun askForHit(playerName: String): String

OutputViewInterface에선
fun showPlayingCard(player: Player)

Comment on lines 20 to 25
private val cardNumberList = listOf(
ACE, TWO, TREE, FOUR, FIVE, SIX, SEVEN,
EIGHT, NINE, TEN, JACK, QUEEN, KING,
)

fun getCardNumberList() = cardNumberList
Copy link
Member

Choose a reason for hiding this comment

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

#672 (comment)

이 피드백을 참고해볼 수 있겠네요 :)

Copy link
Author

@oyj7677 oyj7677 Dec 6, 2023

Choose a reason for hiding this comment

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

해당 리뷰 수정하였습니다. 사용하지 않는 함수 제거하였습니다.

init {
        for (suit in Suit.values()) {
            for (cardRank in CardRank.values()) {
                CARDS[toKey(suit, cardRank)] = PlayingCard(suit, cardRank)
            }
        }
    }

Comment on lines 2 to 3

data class PlayingCard(private val suit: Suit, private val cardRank: CardRank) {
Copy link
Member

Choose a reason for hiding this comment

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

#672 (comment)
여전히 유효한 피드백입니다 :)

Copy link
Author

@oyj7677 oyj7677 Dec 6, 2023

Choose a reason for hiding this comment

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

생성자의 내부로 들어가야하는 값은 메세지를 보내서 받고 직접 접근하는 건 접근제한자를 제거했습니다.

data class PlayingCard(val suit: Suit, val cardRank: CardRank) {
    fun getPoint(): Int {
        return cardRank.point
    }

    fun getSuitName(): String {
        return suit.koName
    }
.....
}

윤혁님만의 직접 접근과 메시지를 통한 접근에 대한 기준점을 알려주시면 감사드리겠습니다.

Copy link
Member

Choose a reason for hiding this comment

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

여러 팩의 카드를 현실세계에서 사용한다면, 스페이드 ACE라는 카드는 팩의 수 만큼 존재하겠지요 :)
컴퓨터의 세계에서도 동일할까요?
여러 팩의 카드를 쓰더라도, 동일한 인스턴스를 가리킨다면, 스페이드 ACE라는 카드(인스턴스)는 메모리 상에 단 한 장만 존재해도 됩니다.

하지만 현재 스페이드 ACE 라는 카드는 생성자를 통해 얼마든지 만들어낼 수 있는 구조이지요 :)
그렇기에 생성자를 private으로 변경하고, 팩터리 메서드를 통해 캐싱된 카드를 반환하도록 만들어보라는 의미었습니다.

자바에서는 Integer라는 class에서 자주 사용하는 숫자를 캐싱해두고 사용해요.
이 부분의 구현을 참고해보셔도 좋겠어요 :)

Comment on lines 12 to 14
fun getCardRank(): CardRank {
return cardRank
}
Copy link
Member

Choose a reason for hiding this comment

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

#672 (comment)

코틀린 프로퍼티에 아직 익숙하지 않으신 것 같네요!
의식적인 연습을 하시면 충분히 익숙해지실 거라 생각해요 :)

Copy link
Author

Choose a reason for hiding this comment

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

생성자 프로퍼티가 public이 되면서 삭제하였습니다.

Comment on lines 5 to 19
class GameDeck(override val cardDeck: MutableList<PlayingCard>) : CardDeck {

init {
cardDeck.shuffle()
}
private var index = 0

fun getCardWithIncrease(): PlayingCard {
return cardDeck[index++]
}

fun addCards(cards: List<PlayingCard>) {
this.cardDeck.addAll(cards)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

index가 51을 넘어가면 out of index 예외가 발생할 것으로 우려되는데요,

블랙잭 게임 룰 중에는 한 번에 여러 벌의 카드를 섞어 진행하기도 합니다.
이런 요구사항에 유연하게 대응할 수 있게 구성해보는 건 어떨까요?

Copy link
Author

@oyj7677 oyj7677 Dec 6, 2023

Choose a reason for hiding this comment

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

카드의 인덱스가 카드덱 사이즈와 같아지면 인덱스를 0으로 변경하고 카드 셔플을 진행하도록 변경하였습니다.

fun getCardWithIncrease(): PlayingCard {
    if (isMaxIndexOfCard()) resetCard()
    return cardList[index++]
}

private fun isMaxIndexOfCard(): Boolean {
    return index == cardList.size
}

private fun resetCard() {
    cardList.shuffle()
    index = 0
}

Comment on lines 5 to 7

class PlayerDeck(override val cardDeck: MutableList<PlayingCard> = mutableListOf()) : CardDeck {

Copy link
Member

Choose a reason for hiding this comment

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

deck클래스의 경우 게임 덱(배포 될 카드)과 플레이어 덱(받은 카드) PlayingCard의 컬렉션으로 구성되어 있어서 interface를 만들어 사용했습니다. 이러한 구성이 맞는지 자세히 리뷰 부탁드리겠습니다.

개발에 맞고 틀림은 없다고 생각해요. 관점과 방향이 달라 정답이 없는 분야라고 생각해요 :)

다만, PlayerDeck이라고 표현하기에는, 플레이어가 가지게 될 몇 장 정도의 카드를 "덱" 이라고 부르는 것은 조금 부자연스러워보여요.
#672 (comment)
이전에 남긴 피드백이었던, 카드들을 일급 컬렉션으로 만들어보는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

PlayerDeck 대신 Hands로 변경하였습니다.

class Hands(val cardList: MutableList<PlayingCard> = mutableListOf())

해당 일급 컬랩션은 Player가 사용합니다.

Comment on lines 8 to 12
var status = Status.START
private set

var playerDeck = PlayerDeck()
private set
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
Author

@oyj7677 oyj7677 Dec 6, 2023

Choose a reason for hiding this comment

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

객체지향 생활 체조 원칙
7. 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.

위 원칙을 생성자에도 적용해야되는 줄 알고 아래로 뺐었습니다.
DM으로 질문 드린 구조 적용하였습니다.

class Player(
    val name: String,
    status: Status = Status.START,
    hands: Hands = Hands(),
) {

    var status = status
        private set

    var playerDeck = hands
        private set

Copy link
Member

Choose a reason for hiding this comment

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

3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다 에서는,
필드 즉, 멤버변수가 인스턴스 변수임을 의미해요.

다만, 코틀린 생성자에서는 파라미터와 함께 멤버변수를 동시에 정의할 수 있기에, 파라미터로 들어오는 것 또한 인스턴스 변수라고 헷갈릴 수 있을 것 같아요.

인스턴스 변수가 많다면, 비슷한 책임을 지닌 변수들을 하나의 모델로 묶어서 표현한다면, 2개로 줄여볼 수 있겠네요 :)

Comment on lines 12 to 29
fun `트럼프 카드를 생성하고, 해당 카드의 포인트를 요청할 때, 포인트를 반환한다`() {
// given : 트럼프 카드를 생성한다.
val playingCard = PlayingCard(Suit.SPADE, CardRank.TREE)
val playingCard2 = PlayingCard(Suit.HEART, CardRank.TWO)
val playingCard3 = PlayingCard(Suit.HEART, CardRank.ACE)
val playingCard4 = PlayingCard(Suit.HEART, CardRank.KING)

// when : 해당 카드의 포인트를 요청한다.
val actual = playingCard.getPoint()
val actual2 = playingCard2.getPoint()
val actual3 = playingCard3.getPoint()
val actual4 = playingCard4.getPoint()

// then : 카드별 포인트를 반환한다
assertThat(actual).isEqualTo(3)
assertThat(actual2).isEqualTo(2)
assertThat(actual3).isEqualTo(1)
assertThat(actual4).isEqualTo(10)
Copy link
Member

Choose a reason for hiding this comment

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

완전히 동일한 테스트를 여러번 수행하고 있다고 생각해요. 1개를 검증하는 것만으로도 충분하다 생각해요.
정말로 필요하다 생각이 드신다면, ParameterizedTest를 활용해보는 건 어떨까요?

Comment on lines 11 to 27
fun `GameDeck을 생성하고, 카드 반환 요청을 했을 떄, 카드 반환과 카드 인덱스가 증가한다`() {
// given : Cards를 생성한다.
val gameDeck = GameDeck(CardPack.cards.toMutableList())

// when : 카드 한장 반환을 요청한다.
val actual = gameDeck.getCardWithIncrease()

// then : 첫번째 카드를 반환하고 카드 인덱스가 1 증가한다.

assertThat(actual).isEqualTo(gameDeck.cardDeck[0])

// when : 카드 한장 반환을 요청한다.
val actual2 = gameDeck.getCardWithIncrease()

// then : 두번째 카드를 반환하고 카드 인덱스가 1 증가한다.
assertThat(actual2).isEqualTo(gameDeck.cardDeck[1])
}
Copy link
Member

Choose a reason for hiding this comment

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

테스트 코드의 검증부는 하드 코딩하는 것이 바람직합니다.

도메인 로직을 활용한 테스트의 방식은, 결국 프로덕션 코드를 copy & paste 한 결과와 다르지 않습니다.

가장 문제가 되는 점은, 해당 도메인 로직 자체에 버그가 존재하더라도 해당 테스트 코드는 통과하게 됩니다. 도메인 로직을 그대로 사용해서 비교하기 때문이에요.

GameDeck에는 개발자가 원하는 카드 목록만 넣어서 생성한 뒤, 검증문에서는 해당 카드가 나왔는지 하드코딩으로 비교해보는 건 어떨까요?

검증부 하드 코딩에 대한 좋은 글을 공유드립니다. 꼭 읽어 보시길 바라요 :)
https://jojoldu.tistory.com/615?category=1036934

Comment on lines 23 to 30

@Test
fun `카드 점수 반환`() {
// given : 플레이어 덱을 생성하고 카드를 추가한다.
val playerDeck = PlayerDeck()
playerDeck.addCard(PlayingCard(Suit.DIAMOND, CardRank.ACE))
playerDeck.addCard(PlayingCard(Suit.DIAMOND, CardRank.TEN))
playerDeck.addCard(PlayingCard(Suit.DIAMOND, CardRank.TEN))
Copy link
Member

Choose a reason for hiding this comment

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

ACE가 10점으로 계산되는 경우도 테스트 코드를 추가해보는 건 어떨까요?

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.

피드백 반영 고생 많으셨어요 :)
다음 미션 진행을 위해 머지하겠습니다.
피드백은 다음 미션에 반영해주세요!

Comment on lines +35 to +36
private fun handleForResponse(response: String, player: Player) {
when (response.uppercase()) {
Copy link
Member

Choose a reason for hiding this comment

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

response가 String으로 들어오는 것은 Console View의 의존성이 여전히 묻어나보여요.
입력 뷰가 조금 바뀌어서 y, n가 아닌, YES, NO 혹은 예,아니오로 변경된다면, 이 모든 String에서 검증해주어야할까요?

view에서는 예 혹은 아니오에 대한 도메인 객체를 반환하도록 만들어보는 건 어떨까요?

Comment on lines +43 to +45
else -> {
println(TEXT_RETRY_INPUT)
}
Copy link
Member

Choose a reason for hiding this comment

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

이 부분도 여전히 Console View의 의존관계를 가지고있습니다 :)
output View에 대한 추상화를 만드셨으니 이쪽으로 옮겨주세요!

Comment on lines +27 to +33
init {
for (suit in Suit.values()) {
for (cardRank in CardRank.values()) {
CARDS[toKey(suit, cardRank)] = PlayingCard(suit, cardRank)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

코틀린 확장함수를 활용해보는 건 어떨까요?
람다로 표현된 식은 바로 멤벼변수에 할당해서, 멤버변수를 mutable로 유지하지 않을 수 있겠어요 :)

Comment on lines +4 to +5

class CardDeck(private val cardList: MutableList<PlayingCard>) {
Copy link
Member

Choose a reason for hiding this comment

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

생성자 파라미터로 Mutable Collection을 받는 것은 지양하는 것이 좋습니다 :)
외부에 남아있는 참조를 실수로 조작한다면 이 객체가 문제가 생길 수 있는데요,
아래 코드를 실행하면 어떤 일이 발생할까요?

val mutableList = mutableListOf()
val cardDeck = CardDeck(mutableList)

mutableList.clear()

Comment on lines +10 to +12

private var index = 0

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 +2 to +3

interface InputViewInterface {
Copy link
Member

Choose a reason for hiding this comment

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

View를 interface로 주입할 수 있게 변경되었네요 👍

다만, interface에 interface라는 이름을 붙이는 건 바람직하지 않아보여요.
다른 class에 ~~class 라는 접미사를 붙이지 않는 것 처럼요 :)

InputView가 interface가 되고, 하위 구현체가 구체적인 어떤 InputView인지 이름으로 나타내보는 건 어떨까요?

@malibinYun malibinYun merged commit 320b91c into next-step:oyj7677 Dec 8, 2023
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