Open
Conversation
MyStoryG
requested changes
Dec 2, 2023
MyStoryG
left a comment
MyStoryG
left a comment
There was a problem hiding this comment.
승화님 안녕하세요.
3단계 구현 하시느라 수고하셨습니다.
몇 가지 코멘트 남겼으니 확인 부탁드립니다.
질문 주신 내용은 코멘트로 남긴 내용을 반영 하시다 보면 해결될 것으로 보여요!
객체의 역할에 대해 조금 더 고민해 보시고 테스트 케이스도 좀 더 추가해 보면 좋을 것 같습니다.
| @JvmInline | ||
| value class LottoAnswer( | ||
| private val answer: List<Int> | ||
| data class LottoAnswer( |
There was a problem hiding this comment.
LottoAnswer라고 하니 어떤 역할인지 떠오르지 않았습니다.
저희는 이 도메인에 대해 지속적으로 고민하니 사용자에게 입력받은 값이라는 것을 압니다.
하지만 당첨번호를 의미하는 말로 바꾸면 좀 더 명확할 것 같다는 생각이 드네요.
Comment on lines
+4
to
+5
| private val answer: List<Int>, | ||
| private val bonusNumber: Int |
There was a problem hiding this comment.
입력받은 값(당첨번호)은 이렇게 별도로 관리할 필요는 없을 것 같습니다.
현재 Lotto 클래스가 대체할 수 있지 않을까요?
당첨번호나 로또나 똑같이 6개의 번호를 가지는 동일한 형태니까요.
현재 객체들의 역할에 대해 조금 더 고민하시면서 변경해 보면 좋을 것 같아요.
| println("총 수익률은 ${result.earningRate}입니다.") | ||
| } | ||
|
|
||
| private fun outputLottoResultSeperate(matchCount: MatchCount, amount: Int, earnResult: Map<MatchCount, Int>): String { |
Comment on lines
+17
to
+23
| val earningStrategy = mapOf( | ||
| MatchCount.THREE to 5000, | ||
| MatchCount.FOUR to 50000, | ||
| MatchCount.FIVE to 1500000, | ||
| MatchCount.FIVE_WITH_BONUS to 30000000, | ||
| MatchCount.SIX to 2000000000 | ||
| ) |
There was a problem hiding this comment.
이렇게 매핑을 하기보다 힌트에도 있듯이 Enum으로 관리해 보면 어떨까요?
그리고 지금처럼 컨트롤러에 이 로직이 있는 것이 적합한지도 고민해 보시면 좋을 것 같아요.
| println("${lottoCount}개를 구매했습니다.") | ||
| lotteries.forEach { | ||
| println(it.numbers) | ||
| println(it.numbers.sorted()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
step3도 잘 부탁드립니다
고민사항
기존에는 LottoAnswer가 일급 컬렉션 이였는데 bonusNumber가 추가되게 되어서 일급컬렉션에서 벗어나게 되었는데 별도의 일급컬렉션을 만들어야 할까요?