Step4 - Lotto (Manual) #1158
Step4 - Lotto (Manual) #1158
Conversation
There was a problem hiding this comment.
Hi Mingyum, how are you? 🤗
You also did a great job on this step!
The code is already well-structured and tested, but since this is the final stage, it would be great to do a bit of cleanup before wrapping up.
I've left a few comments with suggestions for improvement.
You're almost there — keep it up and finish strong! 💪
| val matches = ticket.checkMatches(winningNumbers) | ||
| val bonusMatches = ticket.checkBonusMatches(winningNumbers) | ||
| statistics.add(matches, bonusMatches) |
There was a problem hiding this comment.
Continue to #1156 (comment):
Oh, my mistake 🥲 I explained it inappropriately — sorry for the confusion.
What I actually meant to ask was: "Why do we check both matches and bonus matches separately?"
Since both are needed to determine the prize, wouldn’t it make more sense to treat them as a single match result?
Is there a way we could express this in a more cohesive?
| @@ -1,12 +1,24 @@ | |||
| package lotto.domain | |||
|
|
|||
| import lotto.controller.WinningNumbers | |||
|
|
|||
| class Ticket(val lottoNumber: Set<LottoNumber>) { | |||
| constructor() : this(generateLottoNumber()) | |||
There was a problem hiding this comment.
It's a nice approach to distinguish between generating a ticket randomly and manually.
However, I think the current constructor doesn't clearly convey that intent.
How about using a factory function instead?
Here’s a reference you might find helpful:
| val tickets = LottoShop().purchase(purchaseAmount) | ||
| resultView.printTickets(tickets) | ||
| val manualTickets = inputView.enterManualTickets() | ||
| val tickets = LottoShop().purchase(purchaseAmount, manualTickets) |
There was a problem hiding this comment.
When we purchase the tickets, a new instance of LottoShop is created each time.
Do you think that's necessary? How else could we handle that?
Here’s a reference you might find helpful:
(+) Similarly, LottoNumber is a value object with a fixed range of values.
It might be worth thinking about how instances are managed.
I recommend revisiting what we covered in the last feedback session!
| throw IllegalArgumentException("Please enter $AMOUNT_OF_NUMBER_FOR_TICKET numbers") | ||
| } | ||
| } | ||
|
|
||
| constructor(input: List<Int>) : this(input.map { LottoNumber(it) }.toSet()) |
There was a problem hiding this comment.
Before merging, it might be good to double-check if the class layout follows the recommended order across the code.
Hello, Brie. How are you 😆
I completed to add new feature in Lotto mission, and apply your comment in previous PR.
I hope you have great weekend.
Thank you 😄