[STEP-2] 로또(자동) #1054
[STEP-2] 로또(자동) #1054
Conversation
There was a problem hiding this comment.
기능 목록을 잘 만들어주셨네요 👍
이렇게 작성하신 기능 목록이 결국에는 커밋 단위, 테스트 시나리오까지 이어지게 되는데요,
특히 TDD를 하시는 과정에서 객체 설계 및 “어떤 시나리오를 테스트해야 하지“에 대한 본인만의 답을 만들기에도 도움이 될거예요.
|
|
||
| lottoUser.checkLotteries(correctNumbers) | ||
| printLottoResult(lottoUser.calculateLottoCorrectCount()) | ||
| print수익률(lottoUser.수익률) |
There was a problem hiding this comment.
함수명이나 변수명으로 한글을 활용해주셨네요!
개발자들마다 한글 활용에 대한 여러 가지 시각이 있는 것 같아요.
저는 개인적으로 프로젝트 전체적으로 명확히 한글을 사용하는 상황에 대해 팀에서 합의만 된다면 크게 상관없다고 생각합니다 🙂
| data class CorrectNumbers( | ||
| val values: Set<Int>, | ||
| ) { | ||
| init { | ||
| require(values.size == CORRECT_NUMBER_COUNT) { | ||
| "[CorrectNumbers] 당첨 번호의 개수가 6개가 아닙니다. | 당첨번호: '$values'" |
There was a problem hiding this comment.
다른 개발자들도 CorrectNumbers 네이밍을 보고 "로또 당첨 번호"와 관련된 객체라고 이해할 수 있을까요? 어떻게 생각하시나요?
| } | ||
|
|
||
| companion object { | ||
| private const val CORRECT_NUMBER_COUNT = 6 |
There was a problem hiding this comment.
이 6이라는 숫자는 Lotto와 CorrectNumbers에서 모두 들고 있는데요,
중복을 의도하신 것일까요? 만약 중복되지 않아야 한다면 어디서 들고 있는 것이 적절하다고 생각하시나요?
| @@ -0,0 +1,26 @@ | |||
| package lotto.domain.enums | |||
|
|
|||
| private const val DEFAULT_COMPENSATION_UNIT = "원" | |||
There was a problem hiding this comment.
"원"이라는 값은 도메인보다는 뷰 요구사항에 적절하지 않을까요? (자동차 경주 미션 5단계 참고)
| private const val LOTTO_NUMBER_COUNT = 6 | ||
|
|
||
| private fun rollingRandom(): Set<Int> { | ||
| return (MIN_LOTTO_NUMBER..MAX_LOTTO_NUMBER) |
There was a problem hiding this comment.
rollingRandom을 새로 돌릴 때마다 (MIN_LOTTO_NUMBER..MAX_LOTTO_NUMBER) 구문으로 Range 객체가 매번 만들어지게 되는데요, 어떻게 개선할 수 있을까요?
| fun findByCorrectCount(correctCount: Int?): LottoCompensationStrategy? = | ||
| entries.find { it.correctCount == correctCount } | ||
|
|
||
| fun getCompensationByCorrectCount(correctCount: Int?): Long = | ||
| findByCorrectCount(correctCount)?.compensation | ||
| ?: DEFAULT_COMPENSATION |
There was a problem hiding this comment.
enum class에게도 메세지를 보내어 외부에서 조작하지 않고, 적절한 값을 받아올 수 있는 구조를 잘 만들어주신것 같아요 👍
다만 ~Strategy라는 네이밍을 보면 전략 패턴을 구현했다고 생각할 수 있을 것 같은데요, 혹시 특별한 의도가 있으셨을까요?
| } | ||
|
|
||
| @Test | ||
| fun `correctCount가 null이면 markedCorrectCount을 조회할 수 없다`() { |
There was a problem hiding this comment.
꼭 정확한 프로퍼티명을 테스트명에 언급하진 않으셔도 됩니다!
오히려 구체적인 구현(함수명, 클래스명 등)을 테스트명에 포함시키면 강하게 결합되어 나중에 유지보수하기 어려워집니다.
| fun `로또의 숫자들은 무작위로 섞여 있다`() { | ||
| val lotto = Lotto() | ||
|
|
||
| lotto.values.sorted() shouldNotBe lotto.values |
There was a problem hiding this comment.
랜덤 로직을 간접적으로라도 참조하고 있는 테스트는 매번 실행할때마다 어떤 결과가 나올지 보장할 수 없습니다.
특정 로또 번호에서만 버그가 발생한다고 가정한다면, 같은 코드여도 테스트가 간헐적으로 실패할 확률이 있다는 뜻입니다
| import org.junit.jupiter.params.ParameterizedTest | ||
| import org.junit.jupiter.params.provider.ValueSource | ||
|
|
||
| class LottoPurchaseAmountTest { |
There was a problem hiding this comment.
얼마의 금액으로 몇 장의 로또를 살 수 있었는지도 검증해보면 어떨까요~?
안녕하세요 리뷰어님!
상세한 리뷰 감사드립니다! 😄
이번 리뷰도 잘 부탁드려요ㅎㅎ 🙇🏻♂️