Conversation
wisemuji
left a comment
wisemuji
left a comment
There was a problem hiding this comment.
안녕하세요 창환님!
3단계 구현을 잘 해주셨습니다 💯 깔끔한 구현이 좋았습니다
몇 가지 고민해보시면 좋을만한 코멘트를 달았으니 확인해주시면 감사하겠습니다.
궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요!
| MATCHED_FIVE(1_500_000, 5, BonusMatchResult.NOT_MATCH), | ||
| MATCHED_FIVE_WITH_BONUS(30_000_000, 5, BonusMatchResult.MATCH), | ||
| MATCHED_SIX(2_000_000_000, 6, BonusMatchResult.NO_EFFECT), ; | ||
|
|
||
| companion object { | ||
| private const val MATCH_NUMBER_TRANSFER_ERROR_MESSAGE = "로또 결과 이넘값 변환 오류가 발생하였습니다." | ||
| private val matchNumberToMatchResultMap = entries.associateBy { it.matchNumber } | ||
| private const val RELATED_BONUS_MATCH_NUMBER = 5 |
There was a problem hiding this comment.
5라는 숫자가 이렇게 3번 선언될 필요가 있을까요? 정답은 없을 것 같은데 어떻게 생각하시나요?
| enum class MatchingResult(val prizeAmount: Int, val matchNumber: Int, val bonusMatchResult: BonusMatchResult) { | ||
| MATCHED_THREE(5_000, 3, BonusMatchResult.NO_EFFECT), | ||
| MATCHED_FOUR(50_000, 4, BonusMatchResult.NO_EFFECT), | ||
| MATCHED_FIVE(1_500_000, 5, BonusMatchResult.NOT_MATCH), |
There was a problem hiding this comment.
NOT_MATCH와 NO_EFFECT가 의미있게 구분되어야 하는 이유가 있을까요?
| purchaseAmount: Int, | ||
| ): Double { | ||
| val matchResults: Map<MatchingResult, Int> = getMatchLottoResult(winningNumbers) | ||
| val matchResults: Map<MatchingResult, Int> = getMatchLottoResult(winningNumbers, bonusNumber) |
There was a problem hiding this comment.
다른 개발자들도 어떤 의미인지 한 눈에 이해할 수 있도록 Kotlin의 named arguments를 활용해보시면 어떨까요~?
| private fun getByAuto(): Set<LottoNumber> { | ||
| lottoNumberGenerator ?: return setOf() | ||
| return buildSet { while (size < 6) add(LottoNumber.get(lottoNumberGenerator.generateLottoNumber())) } | ||
| private fun getByAuto(): Pair<Set<LottoNumber>, LottoNumber> { |
There was a problem hiding this comment.
다른 개발자들도 Pair<Set<LottoNumber>, LottoNumber>을 보고 어떤 의미를 갖는 자료형인지 한눈에 이해할 수 있을까요?
| private companion object { | ||
| fun getLottoBunch(numbers: List<List<Int>>): LottoBunch = | ||
| LottoBunch(numbers.map { Lotto(RandomGenerator).apply { setLottoByManual(*it.toIntArray()) } }) | ||
| fun getLottoBunch(numbers: List<Pair<List<Int>, Int>>): LottoBunch = |
There was a problem hiding this comment.
다른 개발자들도 List<Pair<List<Int>, Int>>을 보고 어떤 의미를 갖는 자료형인지 한눈에 이해할 수 있을까요?
| fun match( | ||
| winningNumber: List<LottoNumber>, | ||
| bonusNumber: LottoNumber, | ||
| ): MatchingResult? { | ||
| return MatchingResult.getResult(lottoNumbers.intersect(winningNumber).size, bonusNumber == this.bonusNumber) | ||
| } |
There was a problem hiding this comment.
데이터를 꺼내어(get) 조작하지 않고, 메세지를 던져 객체가 일하도록 구조를 잘 만들어주셨네요!
| } | ||
| } | ||
|
|
||
| fun setLottoByManual( |
There was a problem hiding this comment.
저는 개인적으로 private 함수보다는 public 함수를 먼저 위치시키는 것을 선호합니다. 다른 개발자들이 이 클래스의 역할을 파악할 때 public으로 외부에 드러난 함수를 먼저 보고, 그 다음에 숨겨진 함수를 보는 것이 자연스럽다고 느껴질 것 같아요.
(이 내용을 정리한 레퍼런스가 있었는데 못찾겠네요 😂
| class Lotto(private val lottoNumberGenerator: LottoNumberGenerator) { | ||
| var lottoNumbers: Set<LottoNumber> = getByAuto() | ||
| var lottoNumbers: Set<LottoNumber> | ||
| private set | ||
| var bonusNumber: LottoNumber | ||
| private set |
There was a problem hiding this comment.
우리가 일반적으로 로또를 구매할 때 보너스 번호를 같이 들고 있나요?
wisemuji
left a comment
wisemuji
left a comment
There was a problem hiding this comment.
안녕하세요 창환님!
3단계 구현을 잘 해주셨습니다 💯 깔끔한 구현이 좋았습니다
몇 가지 고민해보시면 좋을만한 코멘트를 달았으니 확인해주시면 감사하겠습니다.
궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요!
2 participants
안녕하세요 리뷰어님
로또 보너스 번호 구현해보았습니다.
리뷰 부탁드립니다. 감사합니다.