Step4 - 로또 (수동) #974
Step4 - 로또 (수동) #974
Conversation
chore: test클래스 패키지 이동, 코드 컨벤션
Refactor: 당첨 통계 역할 이동 (LottoMachine -> LottoCalculator)
catsbi
left a comment
catsbi
left a comment
There was a problem hiding this comment.
영재님 어느덧 마지막 4단계 미션까지 잘 구현해주셨어요.
매번, 여러 피드백들을 드렸음에도 꾸준히 잘 반영하시고 질문도 많이 해주셔서, 답변을 빨리드리진 못했지만 즐거웠던 시간이였던 것 같은데 영재님은 어떠신지 모르겠네요 😅
마지막 미션 몇 가지 피드백 남기긴했는데, 테스트 커버리지도 충분하고, 크게 문제가 될만한 요소는 없는 것 같아 머지드리고 마무리하도록 하겠습니다. 로또 미션 진행하시느라 고생많으셨어요!
| fun findLottoRanking(matchingNumberCnt: Int, hasBonusNumber: Boolean): LottoRanking { | ||
| return if (matchingNumberCnt == SecondPlace.matchingNumberCnt) { | ||
| determineRankingWithBonusNumber(hasBonusNumber) | ||
| } else { | ||
| LottoRanking.values().find { it.matchingNumberCnt == matchingNumberCnt } ?: None | ||
| } |
There was a problem hiding this comment.
정적 팩토리 함수에서 직접 2등인지 보너스 번호를 비교해야할까요? 이런 특별한 플래그들에 대한 조건들이 보너스 넘버가 끝이 아니라 계속 해서 추가되면 매번 이 함수에 조건들이 추가되야 할 것 같은데, 괜찮을까요?
| } | ||
|
|
||
| fun countMatchingNumbers(lotto: Lotto): Int { | ||
| return this.lotto.selectNumbers.intersect(lotto.selectNumbers).size |
There was a problem hiding this comment.
모든 로직을 작성할때 dot(.)이 몇 개인지 체크해보는것도 좋을 것 같아요. 개인적으로는 Stream API와 같이 중간 오퍼레이터들이 있는 지연 연산되는 특수한 경우를 제외하고는 2개 이상의 dot(.)이 있는경우 내가 지금 이 객체의 속살을 모두 꺼내서 요리를 하는게 아닌가? 고민해볼 수 있어요.
| return this.lotto.selectNumbers.intersect(lotto.selectNumbers).size | |
| return this.lotto.matchCount(targetLotto) |
|
|
||
| private const val GAME_COST = 1000 | ||
|
|
||
| fun calculateWinningRate(cash: Int, winningStatus: Map<LottoRanking, Int>): Float { |
There was a problem hiding this comment.
map자료구조에서 열거타입을 사용한다면 EnumMap을 고려해보면 좋을 것 같아요.
|
|
||
| object LottoCalculator { | ||
|
|
||
| private const val GAME_COST = 1000 |
There was a problem hiding this comment.
보통 상품의 가격은 상품에서 가지고 있죠.
로또의 가격은 로또 계산기에 있는게 맞을까요 로또가 가지고 있는게 맞을까요 ?!
|
|
||
| @Test | ||
| fun `당첨 번호와 일치하는 번호 수 반환`() { | ||
| // given : 당첨로또와 로또를 받는다. |
There was a problem hiding this comment.
주석에 행위에 대한 설명을 작성하는건 외부에 의한 요인으로 인한 변경점이 있어서 함수 시그니처로 나타내기 부족한 경우처럼 필수적인 경우가 아니라면 비권장드립니다.
컴파일시 체크할 수 있는 항목도 아니기에 기능변경이 일어나거나 관련 시그니처가 변경될때 해당 개발자가 직접 체크하지 않는한 놓치게 될 수 있고, 이로인해 시그니처와 주석이 달라지게되면서 차후 새로운 개발자는 혼동을 겪을 수 있습니다. 즉 가독성이 떨어지게 되는 것이죠.
2 participants
4단계 미션 전에 일전에 주셨던 step3 리뷰 반영한 것 리뷰를 부탁드리고자 합니다.
이전 리뷰 해주신 곳에 수정사항과 소량의 질문 댓글로 남겼습니다.