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단계 - 로또(자동) #994

Open
wants to merge 2 commits into
base: dajeongda
Choose a base branch
from

Conversation

dajeongda
Copy link

pr 드립니다.

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 +25 to +29
private const val WINNER_REWARD = 2000000000
private const val SECOND_REWARD = 1500000
private const val THIRD_REWARD = 50000
private const val FORTH_REWARD = 5000
private const val LOTTO_PRICE = 1000
Copy link
Member

Choose a reason for hiding this comment

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

로또 등수를 도메인 객체로 표현해보는 건 어떨까요? enum 등으로 구현해보셔도 좋겠어요 :)

Comment on lines +7 to +8
fun match(
userLotto: Set<Int>,
Copy link
Member

Choose a reason for hiding this comment

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

Int에는 1~45 라는 로또 번호를 제한하기에는 너무 범용적인 자료형이라 생각해요.
로또 번호를 나타내는 도메인 객체를 만들어보는 건 어떨까요?

Comment on lines +3 to +4
class LottoShop {
fun buy(count: Int): List<Set<Int>> {
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 +8 to +76
@Test
fun `1등은 6개 일치` () {
val lottoGame = LottoGame()
val userLotto = setOf(1, 2, 3, 4, 5, 6)
val winningLotto = setOf(1, 2, 3, 4, 5, 6)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 1
}

@Test
fun `2등은 5개 일치`() {
val lottoGame = LottoGame()
val userLotto = setOf(1, 2, 3, 4, 5, 6)
val winningLotto = setOf(1, 2, 3, 4, 5, 7)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 2
}

@Test
fun `3등은 4개 일치`() {
val lottoGame = LottoGame()
val userLotto = setOf(1, 2, 3, 4, 5, 6)
val winningLotto = setOf(1, 2, 3, 4, 7, 8)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 3
}

@Test
fun `4등은 3개 일치`() {
val lottoGame = LottoGame()
val userLotto = setOf(1, 2, 3, 4, 5, 6)
val winningLotto = setOf(1, 2, 3, 7, 8, 9)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 4
}

@Test
fun `2개 경우 일치 하면 0`(userLotto: Set<Int>) {
val lottoGame = LottoGame()
val userLotto = setOf(1, 2, 12, 13, 14, 15)
val winningLotto = setOf(1, 2, 3, 7, 8, 9)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 4
}

@Test
fun `1개 경우 일치 하면 0`(userLotto: Set<Int>) {
val lottoGame = LottoGame()
val userLotto = setOf(1, 11, 12, 13, 14, 16)
val winningLotto = setOf(1, 2, 3, 7, 8, 9)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 4
}

@Test
fun `0개 경우 일치 하면 0`(userLotto: Set<Int>) {
val lottoGame = LottoGame()
val userLotto = setOf(10, 11, 12, 13, 14, 16)
val winningLotto = setOf(1, 2, 3, 7, 8, 9)
val actual = lottoGame.match(userLotto, winningLotto)

actual shouldBe 4
}
Copy link
Member

Choose a reason for hiding this comment

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

이런 형태의 테스트는 @ParameterizedTest를 활용해보는 건 어떨까요?

Comment on lines +8 to +14
@Test
fun `로또 n장 사기`() {
val lottoShop = LottoShop()
val count = 5
val lottos = lottoShop.buy(5)
lottos.size shouldBe count
}
Copy link
Member

Choose a reason for hiding this comment

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

몇 장을 샀는지 검증하는 것은 테스트에 큰 의미를 가지지 못한다고 생각해요.
개발자가 제어 가능한 형태로, 어떤 티켓이 반환되었는지까지 테스트할 수 있게 만들어보는 건 어떨까요?

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

3 participants