4단계 - 로또(수동) #589
Open
4단계 - 로또(수동) #589
Conversation
This file contains 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
Flamme1004K
requested changes
Jan 1, 2023
Comment on lines
+13
to
+20
| val resultMap = mutableMapOf<LottoTicketResult, Int>() | ||
| lottoTicketBundle.lottoTickets.groupingBy { ticket -> | ||
| val intersectNumbers = ticket.intersect(winningBallResult.winningBalls) | ||
| val isBonusBallMatched = winningBallResult.bonusBall in ticket | ||
| LottoTicketResult(intersectNumbers.size, isBonusBallMatched) | ||
| }.eachCountTo(resultMap) | ||
|
|
||
| return LottoWinningResult(resultMap.toSortedMap(getTicketResultComparator())) |
There was a problem hiding this comment.
Suggested change
| val resultMap = mutableMapOf<LottoTicketResult, Int>() | |
| lottoTicketBundle.lottoTickets.groupingBy { ticket -> | |
| val intersectNumbers = ticket.intersect(winningBallResult.winningBalls) | |
| val isBonusBallMatched = winningBallResult.bonusBall in ticket | |
| LottoTicketResult(intersectNumbers.size, isBonusBallMatched) | |
| }.eachCountTo(resultMap) | |
| return LottoWinningResult(resultMap.toSortedMap(getTicketResultComparator())) | |
| return lottoTicketBundle.lottoTickets.groupingBy { | |
| val intersectNumbers = it.numbers.intersect(winningBallResult.winningBalls) | |
| val isBonusBallMatched = winningBallResult.bonusBall in it.numbers | |
| LottoTicketResult(intersectNumbers.size, isBonusBallMatched) | |
| } | |
| .eachCount() | |
| .toSortedMap(getTicketResultComparator()) | |
| .run { | |
| LottoWinningResult(this) | |
| } |
이렇게도 표현이 가능해보이네요~ 😊
그리고 LottoTicketResult를 만드는 부분에 대해서 LottoMachine에서 모두 해결하는 것보다는, LottoTicketResult에게 LottoTicketBundle과 WinningBallResult를 넘겨서 좀 더 객체 협력을 만드는 게 좋지 않을까요?
| val numbers: Set<Int> = LottoNumberSelector.select(), | ||
| ) : Set<Int> by numbers | ||
| val numbers: Set<LottoNumber>, | ||
| ) : Set<LottoNumber> by numbers |
There was a problem hiding this comment.
저번 리뷰 때 kotlin delegate 패턴에 대한 단점을 물어봤었죠~
제가 생각하는 delegate 패턴의 단점은 바로 delegate에 대한 대상의 API를 모두 사용할 수 있다라고 생각해요.
일반적인 객체는 객체 캡슐화에 대해서 내부에서는 사용하고 외부에서는 필요한 메서드만 사용할 수 있지만, delegate를 해버리는 순간 의미없는, 사용하지 않는 메서드까지 모두 외부에서 사용할 수 있기 때문에 외부에서 내부적인 부분까지 사용할 수 있어 객체간의 협력이 깨질 수도 있다고 생각해요~
Comment on lines
+12
to
+17
| val lottoTickets: List<LottoTicket> = List(lottoTicketCount.autoTicketCount) { | ||
| val autoGenerateStrategy = lottoGenerateStrategyMap.getValue(TicketGenerateType.AUTO) | ||
| autoGenerateStrategy.generate() | ||
| } + List(lottoTicketCount.manualTicketCount) { | ||
| val manualGenerateStrategy = lottoGenerateStrategyMap.getValue(TicketGenerateType.MANUAL) | ||
| manualGenerateStrategy.generate() |
| @@ -1,18 +1,17 @@ | |||
| package lotto.domain | |||
|
|
|||
| import lotto.TicketResult | |||
| import lotto.common.LottoTicketPolicy | |||
|
|
|||
| class StatisticalResultExtractor( | |||
There was a problem hiding this comment.
해당 클래스는 LottoWinningResult 와 합쳐도 되는 부분이라고 생각되네요.
추가로 해당 부분은 LottoWinningResult 하나만으로도 모두 표현이 가능하지 않을까해요~
현재 의미가 중복되는 클래스가 너무 많아보이네요.
이 부분에 대해서는 충분한 설명이 필요하시다면 DM 부탁드립니다. 같이 이야기해보죠~
| class LottoManualGenerateStrategy : LottoGenerateStrategy { | ||
| override val ticketGenerateType = TicketGenerateType.MANUAL | ||
| override fun generate(): LottoTicket { | ||
| val manualLottoNumberStr = readln() |
Comment on lines
+18
to
+29
| val lottoWinning = LottoMachine.process( | ||
| LottoTicketBundle(lottoTicketCount, listOf(LottoAutoGenerateStrategy(), LottoManualGenerateStrategy())), | ||
| WinningBallResult( | ||
| WinningBalls( | ||
| setOf( | ||
| LottoNumber(1), LottoNumber(2), LottoNumber(3), | ||
| LottoNumber(4), LottoNumber(5), LottoNumber(6) | ||
| ) | ||
| ), | ||
| LottoNumber(7) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
상세한 테스트 좋습니다! 단, 아래 테스트와 플로우가 겹쳐 보이지 않나요~?
겹쳐 보이는 부분에 대해서는 메서드화할 수 있지 않을까 하네요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
안녕하세요 리뷰어님
이번에도 꼼꼼한 리뷰 부탁드려요 :)