[2단계] 로또 구현 #578
[2단계] 로또 구현 #578
Conversation
| sealed class ExceptionCode { | ||
| object NotAllowNullOrBlank : IllegalArgumentException("Input에 Null이나 빈 값이 있으면 안됩니다") | ||
| object NotMatchNumeric : IllegalArgumentException("Input이 숫자가 아닙니다") | ||
| object NotFindSeparator : IllegalArgumentException("Input에 구분자 ,가 없습니다") | ||
| object NotWinLotteryCount : IllegalArgumentException("당첨번호가 6자리가 아닙니다") | ||
| } |
There was a problem hiding this comment.
요건 개인 취향일 수 있는데 저는 가급적이면 커스텀 예외가 아니라 표준 라이브러리에 존재하는 예외(IllegalArgument, IllegalState 등)을 이용해 처리해보아도 좋을 것 같아요. 표준 라이브러리의 예외는 많은 개발자가 알고 있으므로 널리 알려진 요소를 재사용하면 다른 사람들이 API를 더 쉽게 배우고 이해할 수 있는 장점이 있습니다.
예외 메시지의 중복이 문제가 될 수 있는데 중복이 엄청 많은게 아니라면 그냥 그때그때 필요한 도메인에 예외 메시지를 남겨도 된다고 생각해요.
가급적 표준 예외를 다루라고 했지만 무조건적으로 표준 예외를 사용하지 않고 아래와 같은 경우는 커스텀 예외를 만드는게 더 좋을수도 있습니다.
- 컨트롤러 레이어에서 JPA의 Dataintegrityviolationexception 과 같은 저수준 예외를 활용하는 경우
- require나 check에서 발생시키는 예외가 이미 구현된 예외 처리 구문과 겹쳐 예외를 구분해야 하거나 하는 경우
| @@ -0,0 +1,3 @@ | |||
| package lotto.entity | |||
|
|
|||
| data class Lottery(val values: List<Int>) | |||
There was a problem hiding this comment.
지금은 항상 makeShuffleNumbers을 통해 Lottery가 생성되겠지만 이후에 다른 사람과 협업을 하는 과정에서 values에 listOf(1,2,3,4,5,6,7)이나 listOf(50,60,70,80,90,100)을 넣는 경우가 생길 수 있습니다.
당장 테스트만 봐도 간단한 실수로 제약이 깨질 수 있어보이네요.
val lotteries = listOf(Lottery(listOf(1, 2, 3, 11, 12, 13)), Lottery(listOf(1, 2, 3, 4, 5, 6)))input 쪽에서 제약 조건을 검증할수도 있겠지만 이런 도메인 클래스를 통해 예외를 검증해보시면 좋을 것 같아요.
| private const val rewardThree = 5000 | ||
| private const val rewardFour = 50000 | ||
| private const val rewardFive = 1500000 | ||
| private const val rewardSix = 2000000000 | ||
|
|
||
| data class WinLottery(var matchNum: Int = 0, val reward: Int, val count: Int = 0) | ||
| data class WinLotteryResult( | ||
| var matchThree: WinLottery = WinLottery(0, rewardThree), | ||
| var matchFour: WinLottery = WinLottery(0, rewardFour), | ||
| var matchFive: WinLottery = WinLottery(0, rewardFive), | ||
| var matchSix: WinLottery = WinLottery(0, rewardSix) | ||
| ) |
There was a problem hiding this comment.
enum 등을 활용해 1등, 2등, 3등 당첨 개수와 금액을 별도로 추출해 관리하면 어떨까요?
| val winLottery = input.split(",").map { it.toInt() } | ||
| if (winLottery.size != 6) { |
There was a problem hiding this comment.
당첨 번호도 일반 로또 게임 하나와 동일하게 6개의 숫자, 1-46까지의 숫자라는 동일한 제약 조건을 가집니다.
input에서 검증하더라도 이후 로직에 따라 로또 도메인 객체에 7개의 숫자, 46이 넘는 숫자, 음수 등이 들어가 제약 조건이 깨질 수 있습니다. input단에서 검증하지 않고 로또 도메인 객체를 통해 검증해보세요.
| when (num) { | ||
| 3 -> winLotteryResult.matchThree.matchNum += 1 | ||
| 4 -> winLotteryResult.matchFour.matchNum += 1 | ||
| 5 -> winLotteryResult.matchFive.matchNum += 1 | ||
| 6 -> winLotteryResult.matchSix.matchNum += 1 |
There was a problem hiding this comment.
로또의 요구사항이 변경될 일은 거의 없겠지만 여기에 당첨 숫자가 늘어났을 때 when 구문에 코드를 작성하는걸 잊으면 버그가 발생할 수 있겠네요. enum 등을 활용해 코드를 개선해보세요.
| fun calculateWin(winNumbers: List<Int>, lotteries: List<Lottery>): WinLotteryResult { | ||
| val winLotteryResult = WinLotteryResult() | ||
| winLotteryResult.apply { | ||
| lotteries.forEach { checkWin(it.values.intersect(winNumbers.toSet()).size, this) } | ||
| } | ||
| return winLotteryResult | ||
| } | ||
|
|
||
| private fun checkWin(num: Int, winLotteryResult: WinLotteryResult) { | ||
| when (num) { | ||
| 3 -> winLotteryResult.matchThree.matchNum += 1 | ||
| 4 -> winLotteryResult.matchFour.matchNum += 1 | ||
| 5 -> winLotteryResult.matchFive.matchNum += 1 | ||
| 6 -> winLotteryResult.matchSix.matchNum += 1 |
There was a problem hiding this comment.
만약 enum 등을 활용해 당첨 결과(1등, 2등, 3등 등)을 분류하고 Map<Enum, Int>와 같은 자료구조를 활용하신다고 하면 코틀린 stdlib에서 제공하는 groupingby와 유사한 함수들을 활용해 결과 추출을 불변으로 처리해볼 수 있을 것 같아요.
안녕하세요 리뷰어님
항상 소중한 리뷰 남겨주셔서 감사합니다
이번에도 잘 부탁드립니다
질문사항
당첨 로또를 계산하는 부분을 처리하기 위해 entitiy를 WinLottery에 두개의 data class로 만들었는데 이렇게 만드는 게 최선인지 궁금합니다
이렇게 3,4,5,6을 나눠버리면 확장성에 문제가 있어보이는데, 그렇다고 하나의 클래스에 3,4,5,6을 val로 넣어버리기엔 계산할 때 각각 3,4,5,6을 찾아서 매핑하기가 어렵지 않나 했습니다
랜덤으로 뽑는 함수가 테스트가 어려워서 최대한 상위로 빼려고 하였지만, 로직부분 테스트코드가 너무 빈약하다고 생각합니다 어떻게 하면 좋을까요?
늦었지만 메리 크리스마스입니다