-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[로또] 버디 미션 제출합니다. #12
base: stopmin-main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
버디님 바쁘신 와중에 정말 수고 많으셨습니다~~!
리뷰 늦어서 죄송합니다...ㅎㅎ ㅠㅠ
너무 잘 짜놓으셔서 소소한 코멘트밖에 달 게 없었습니다! 👍
const val LOTTO_NUMBER_MIN = 1 | ||
const val LOTTO_NUMBER_MAX = 45 | ||
const val LOTTO_NUMBERS_PER_TICKET = 6 | ||
const val LOTTO_PRICE = 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상수 분리 👍👍👍
} while (purchasePrice == null) | ||
|
||
println() | ||
return Pair(purchasePrice!!, purchasePrice / LOTTO_PRICE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return Pair(purchasePrice!!, purchasePrice / LOTTO_PRICE) | |
return Pair(purchasePrice, purchasePrice / LOTTO_PRICE) |
!!
는 없어도 되네요~!
class Lotto(private val numbers: List<Int>) { | ||
class Lotto(val numbers: List<Number>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필드를 외부에 노출시키지 않도록, 캡슐화를 지키도록 코드 작성해보셔도 좋을 것 같습니다~!
THREE_MATCH(3, 5_000, "3개 일치 (5,000원)"), | ||
FOUR_MATCH(4, 50_000, "4개 일치 (50,000원)"), | ||
FIVE_MATCH(5, 1_500_000, "5개 일치 (1,500,000원)"), | ||
FIVE_PLUS_BONUS_MATCH(5, 30_000_000, "5개 일치, 보너스 볼 일치 (30,000,000원)", true), | ||
SIX_MATCH(6, 2_000_000_000, "6개 일치 (2,000,000,000원)"), | ||
LOSE(0, 0, "낙첨"); | ||
|
||
// TODO: 추가 기능 구현 | ||
companion object { | ||
fun findPrize(matchCount: Int, isBonusMatched: Boolean = false): LottoPrize { | ||
return entries.find { it.matchCount == matchCount && it.isBonusMatched == isBonusMatched } ?: LOSE | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
간결하게 잘 구현하신 것 같아요 👍
listOf( | ||
LottoPrize.THREE_MATCH, | ||
LottoPrize.FOUR_MATCH, | ||
LottoPrize.FIVE_MATCH, | ||
LottoPrize.FIVE_PLUS_BONUS_MATCH, | ||
LottoPrize.SIX_MATCH, | ||
).map { o -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리스트를 생성하지 않아도 LottoPrize.entries
를 사용하여 전체를 순회하는 방식으로 코드 작성할 수 있을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러코드를 분리하시려고 한 흔적이 보이네요!! 😆👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰가 늦었습니다! 죄송합니닷
버디의 실력은 어디까지인가..
간결한 코드 잘 봤습니다! 👍🏻
일하고 mvc강의 듣고 게임구현까지 너무 고생많으셨어요!
|
||
return bonusNumber!! | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
askWinningNumbers()에서는 require를 사용하여 유효성을 검사하고 있지만, askBonusNumber()에서는 try-catch 구문을 사용하는데, 유효성 검사 방식을 통일하는 것은 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하!!! 맞네요!! 유효성 검사하는 방식을 통일시키고 뭔가 캡슐화 하는 것도 나쁘지 않을 것 같다는 생각이 듭니닷...!! 다음 미션에서는 반영해보겠습니다!!
} | ||
|
||
fun run() { | ||
val (purchasePrice, lottoCount) = lottoStore.receivePaymentForLotto() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
component N 사용하니 좋네요! 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저번에 빅터가 쓴거 보고 따라해봤습니닷 ㅎㅎ
@@ -1,9 +1,28 @@ | |||
package lotto | |||
|
|||
class Lotto(private val numbers: List<Int>) { | |||
class Lotto(val numbers: List<Number>) { | |||
init { | |||
require(numbers.size == 6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init()에서 중복 검사하면 중복 테스트 케이스 통과 할 것 같아요!
numbers를 set으로 바꿔서 사이즈 체크를 한다던가 등 여러 방법이 있을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오~! 그렇군요 이거 그냥 예제코드에 있어서 그대로 썼는데 set도 꽤 좋은 것 같습니닷
} | ||
|
||
println("총 수익률은 ${returnRate}%입니다.") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
당첨 통계부분 간결하네요 ㄷㄷ
map으로 다루는 부분 배워갑니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엄 사실 map으로 사용한건 마음에 든다만 보름이 위에서 언급했듯이 굳이 리스트를 만든게 조금 문제였던 것 같아 좋은 코드인건 아닌 것 같네요 ㅠㅠㅠ
다음에 참고하실 때 보름님 말씀대로 LottoPrize.entries
에서 map을 쓴다면 더 간결하고 좋은 코드일 것 같습니다!!!
구현하면서 고민되었던 점
파일을 많이 나누는게 좋은건 아닌 것 같아서 그냥 적게 나눴습니다.
자꾸 예상치 못한 곳에서 테스트가 실패해서 제출이 조금 늦었습니다. ㅠ 힙 메모리에서 에러가 나네요ㅠㅠ
죄송한 점
뭔가 주말, 이번주 내내 정신이 없네요.. 헣 다른 분들도 바쁘실텐데 구현하시느라 정말 수고하셨습니닷...
테스트 코드 못짰고 아직 구조적으로 많이 부족한 것 같아서 (어제 새벽 3시에 짠 부분들이 많아서 솔직히 손볼게 넘나 많습니다..) 계속 수정해봐야할 것 같습니다. 특히나 예외처리하는 부분도 수정해야하고, 역할에 대해서도 다시 생각해서 수정해야할 것 같습니다.
리뷰 달아주시면 반영하면서 지금 수정하는 것도 마저 해보겠습니다.