Conversation
malibinYun
requested changes
Nov 28, 2023
Member
malibinYun
left a comment
malibinYun
left a comment
There was a problem hiding this comment.
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 |
Member
There was a problem hiding this comment.
로또 등수를 도메인 객체로 표현해보는 건 어떨까요? enum 등으로 구현해보셔도 좋겠어요 :)
Comment on lines
+7
to
+8
| fun match( | ||
| userLotto: Set<Int>, |
Member
There was a problem hiding this comment.
Int에는 1~45 라는 로또 번호를 제한하기에는 너무 범용적인 자료형이라 생각해요.
로또 번호를 나타내는 도메인 객체를 만들어보는 건 어떨까요?
Comment on lines
+3
to
+4
| class LottoShop { | ||
| fun buy(count: Int): List<Set<Int>> { |
Member
There was a problem hiding this comment.
컬렉션의 자료형 타입이 겹쳐져있으면 어떤 값을 의미하는지 쉽게 알아보기 어려워진다고 생각해요.
한 컬렉션을 객체로 묶어 의미를 담아보는 건 어떨까요?
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 | ||
| } |
Member
There was a problem hiding this comment.
이런 형태의 테스트는 @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 | ||
| } |
Member
There was a problem hiding this comment.
몇 장을 샀는지 검증하는 것은 테스트에 큰 의미를 가지지 못한다고 생각해요.
개발자가 제어 가능한 형태로, 어떤 티켓이 반환되었는지까지 테스트할 수 있게 만들어보는 건 어떨까요?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pr 드립니다.