[둘리] 1단계 로또 제출합니다. #17
Conversation
src/main/kotlin/view/OutputView.kt
Outdated
| fun outputResult(lottoResult: LottoResult) { | ||
| println("\n당첨 통계") | ||
| println("---------") | ||
| println("3개 일치 (${Rank.FIFTH.winningMoney})원 - ${lottoResult[Rank.FIFTH]}개") |
src/main/kotlin/domain/Rank.kt
Outdated
| require(countOfMatch in LOTTO_COUNT_MIN..LOTTO_COUNT_MAX) { ERROR_COUNT_OF_MATCH_RANGE.format(countOfMatch) } | ||
| require(countOfMatch != LOTTO_COUNT_MAX || matchBonus.not()) { ERROR_IMPOSSIBLE_CASE } | ||
|
|
||
| return when (countOfMatch) { |
There was a problem hiding this comment.
values() 를 활용하면 간결하게 표현이 가능하지 않을까요?
| } | ||
|
|
||
| fun getRateOfReturn(): Double = | ||
| floor( |
There was a problem hiding this comment.
각 연산에 대한 부분을 함수로 나누거나 변수로 단계를 나눠주면 가독성이 좀 더 확보될 것 같아요.
| const val NUMBER_SIZE = 6 | ||
| private const val ERROR_NUMBER_SIZE = "로또 번호의 개수는 6개여야 합니다. \n 잘못된 값 : %d" | ||
| private const val ERROR_NUMBER_DUPLICATED = "로또 번호는 중복되어선 안된다. \n잘못된 값: %s" | ||
| fun create(numbers: List<Int>): Lotto = Lotto(numbers.map { LottoNumber(it) }) |
There was a problem hiding this comment.
LottoNumber 는 숫자가 같으면 같은 클래스 일거라, 매번 생성하기보다 재사용을 해볼 수도 있을 것 같아요!
src/main/kotlin/domain/LottoStore.kt
Outdated
| class LottoStore { | ||
|
|
||
| fun buyLotto(amount: Int): List<Lotto> { | ||
| require(amount in MINIMUM_AMOUNT..MAXIMUM_AMOUNT) { ERROR_CREATE_COUNT.format(amount) } |
There was a problem hiding this comment.
Amount 에 대한 원시값도 포장�해보고, 거기로 검증 로직을 위임해보는 것은 어떨까요?
There was a problem hiding this comment.
MINIMUM_AMOUNT와 MAXIMUM_AMOUNT로 1000에서 100000까지의 범위로 설정했는데 추가로 음수 예외처리가 필요한 이유에 대해 설명해주실 수 있으실까요?
There was a problem hiding this comment.
앗 그러네요!
제가 다른 분 리뷰를 동시에 진행하다가 헷갈렸던 거 같습니다 😭
얘기주신 것처럼 이미 존재하는 테스트에서 커버하고 있는 영역이라 별도로 음수에 대한 예외처리는 필요없을 것 같습니다~!
src/main/kotlin/domain/Rank.kt
Outdated
|
|
||
| return when (countOfMatch) { | ||
| 6 -> FIRST | ||
| 5 -> if (matchBonus) SECOND else THIRD |
There was a problem hiding this comment.
특정 등수에 대해 조건문을 두지 않고, 일관된 방식으로 등수를 구하도록 리펙토링 해볼 수 있을까요?
현재와 같은 구조는 보너스번호를 가지는 등수가 추가될 때 마다 if 문이 계속 추가되어야하는 구조가 될 거 같아요.
| ; | ||
|
|
||
| companion object { | ||
| fun valueOf(countOfMatch: Int, matchBonus: Boolean): Rank { |
There was a problem hiding this comment.
보너스 번호 요구사항 변경이 발생해도 코드의 큰 변경없이 대응이 가능해지겠네요 👍
안녕하세요, 리뷰어님! 바쁘신 와중에 코드를 리뷰해주셔서 감사합니다.
열심히 배우겠습니다!