Conversation
* InputView 생성
* 로또 1장의 가격은 1000원이다 * Lotto -> Lottery로 이름 변경
* 구매한 금액에 해당하는 로또 갯수 반환
* 구매 금액을 입력하면 구입 금액에 해당하는 로또를 발급해야 한다
pci2676
left a comment
pci2676
left a comment
There was a problem hiding this comment.
klintFormat을 한번 수행한 다음 커밋을 남겨주세요.
그리고 조금 더 개선해보면 좋을 것 같은 부분에 코드리뷰를 남겨두었습니다!
개선 후 다시 리뷰요청 부탁드리겠습니다.
| return readln().split(",").map { | ||
| it.trim().toInt() | ||
| } |
There was a problem hiding this comment.
메서드를 연속해서 호출하는 경우에는 행 구분을 해주세요!
https://kotlinlang.org/docs/coding-conventions.html#wrap-chained-calls
There was a problem hiding this comment.
kotlin convention을 다시 한 번 제대로 읽어봐야겠어요 감사합니다! 🙏🏼
| val lotteryTickets = mutableListOf<Lottery>() | ||
| repeat(count) { | ||
| lotteryTickets.add( | ||
| Lottery((MIN_LOTTERY_NUMBER..MAX_LOTTERY_NUMBER).shuffled().take(6).sorted()) | ||
| ) | ||
| } |
There was a problem hiding this comment.
가변적인 MutableList를 사용하는 것 보다 불변인 List로 생성하는 것이 더 안정적일 것 같습니다~!
아래와 같이 개선해보면 좋을것 같습니다.
| val lotteryTickets = mutableListOf<Lottery>() | |
| repeat(count) { | |
| lotteryTickets.add( | |
| Lottery((MIN_LOTTERY_NUMBER..MAX_LOTTERY_NUMBER).shuffled().take(6).sorted()) | |
| ) | |
| } | |
| val lotteryTickets = List(count) { | |
| Lottery((MIN_LOTTERY_NUMBER..MAX_LOTTERY_NUMBER).shuffled().take(6).sorted()) | |
| } |
There was a problem hiding this comment.
리턴할 때 UserLottery에 List 타입으로 선언해 추가해주었는데, buy 메소드 안에서도 가변적인 리스트보다 불변 리스트를 생성해 리턴하면 더 좋을 것 같네요!! 👍🏼
| fun getWinStatistic( | ||
| lotteryTickets: List<Lottery>, | ||
| lastWinnerNumbers: List<Int> | ||
| ): Pair<List<LotteryResult>, Double> { | ||
| val winResult = checkLotteryTickets(lotteryTickets, lastWinnerNumbers) | ||
|
|
||
| val lotteryResults = mutableListOf<LotteryResult>() | ||
| var sum = 0 | ||
| winResult.forEach { (count, matchCount) -> | ||
| val prize = WinnerPrize.getPrize(count).money | ||
| val message = "${count}개 일치 (${prize}원) - ${matchCount}개" | ||
| lotteryResults.add(LotteryResult(prize, matchCount, message)) | ||
| sum += (prize * matchCount) | ||
| } | ||
|
|
||
| return lotteryResults.toList() to sum.toDouble() |
There was a problem hiding this comment.
List<Lottery>를 가지고 있는 일급컬렉션에게 winningNumbers를 전달하여 위 책임을 위임할 수 있을것 같습니다.
그러면 전체적인 코드가 조금 더 간결해 질 수 있을것 같습니다.
| @@ -0,0 +1,18 @@ | |||
| package lotto | |||
|
|
|||
| enum class WinnerPrize(val money: Int) { | |||
| 3 -> LAST | ||
| 4 -> THIRD | ||
| 5 -> SECOND | ||
| else -> FIRST |
There was a problem hiding this comment.
WinnerPrize가 count에 대한 정보도 가지고 있을 수 있지 않을까요?
| @@ -0,0 +1,20 @@ | |||
| package lotto | |||
| ): List<Pair<Int, Int>> = | ||
| lotteryTickets.map { lottery -> | ||
| lottery.numbers.intersect( | ||
| lastWinnerNumbers.sorted().toSet() |
There was a problem hiding this comment.
다시 생각해보니 sort할 필요는 없는 것 같네요..! 🙏🏼
| 3 -> LAST | ||
| 4 -> THIRD | ||
| 5 -> SECOND | ||
| else -> FIRST |
| lottery.numbers.intersect( | ||
| lastWinnerNumbers.sorted().toSet() | ||
| ).count() | ||
| }.filter { it >= 3 } |
There was a problem hiding this comment.
3개가 넘게 일치하는지를 이곳에서 판별하지 않고
WinnerPrize에게 물어보는 형식으로 변경할 수 있을것 같습니다.
| private fun checkLotteryTickets( | ||
| lotteryTickets: List<Lottery>, | ||
| lastWinnerNumbers: List<Int> | ||
| ): List<Pair<Int, Int>> = |
There was a problem hiding this comment.
Pair로 Int와 Int가 쌍으로 나가는것보다 조금더 의미있는 객체로 내보내는것이 코드를 이해하기 더 수월할 것 같습니다.
2 participants
step2 로또(자동) 리뷰 잘 부탁드립니다! 🙏🏼