Skip to content
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

[Step2] 로또 자동 #797

Open
wants to merge 19 commits into
base: choihwan2
Choose a base branch
from
Open

[Step2] 로또 자동 #797

wants to merge 19 commits into from

Conversation

choihwan2
Copy link

마지막까지 잘부탁드립니다🙇🏻

Copy link

@ohgillwhan ohgillwhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요!
리뷰 조금 남겨봤어요. 확인 부탁드려요~

}
}

fun main() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maincontroller밖의 lottery 패키지에 있어도 되겠네요~

Comment on lines +19 to +20
LotteryGameView.printPurchaseMoneyView()
val money = readln().toInt()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputView를 만들어서 처리해도 괜찮겠네요~

Comment on lines +24 to +25
LotteryGameView.printWinnerLotteryNumber()
val numbers = readln().replace("\\s".toRegex(), "").split(",")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 InputView라는것을 만들어서 처리하면 될거 같네요. 그러면 다른 funOutputView를 만들어서 처리하면 될것 같아요.

return Lotteries.makeAutoLotteries(purchasePrice / Lottery.LOTTERY_PRICE)
}

fun start() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startpurchaseAutoLotteries 를 호출하니, start 밑에 purchaseAutoLotteries 가 있으면 좋겠네요.

import lottery.domain.WinningLottery
import lottery.view.LotteryGameView

class LotteryGame {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Controller라고 정의하신 LotteryGamepurchaseAutoLotteries 로직이 생긴것 같아요.
LotteryGameLotteryController로 바꾸고 로직을 가지고 있는 LotteryGamedomain 에 만들어볼까요?

@@ -0,0 +1,30 @@
package lottery.domain

class Lottery(numbers: Set<LotteryNumber>) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set 💯

companion object {
private val BASE_NUMBERS = (LotteryNumber.MIN_LOTTERY_NUMBER..LotteryNumber.MAX_LOTTERY_NUMBER).toSet()
const val LOTTERY_NUMBER_SIZE = 6
const val LOTTERY_PRICE = 1000

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

가격에 대한 정책은 게임에 더 가까울거 같아요!
만약 Lottery에서 사용하지 않는것이면 해당 객체와 어울리지 않는 상수일 수 있어요.

package lottery.domain

class LotteryRank {
val lotteriesRank = LotteryPrize.values().associateWith { RANK_DEFAULT_VALUE }.toMutableMap()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이름에서 타입을 알 수 없으면 타입을 명시해볼까요?

@@ -0,0 +1,20 @@
package lottery.domain

class LotteryRank {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨트롤러에서 Lotteries를 반복해서 해당 객체를 만드네요. Lotteries 에 메서드를 추가하여 LotteryRank를 만들게 하는건 어떨까요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그러면 팩터리메서드나 생성자를 통해 List<Lottery>를 받을 수 있을것이고, plusRankcalculateProfit 메서드도 사라질 수 있겠네요.

Comment on lines +3 to +4
class WinningLottery(numbers: Set<LotteryNumber>) {
val winningNumbers: Set<LotteryNumber>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 생성자에 val을 붙여서 정의할 수 있겠네요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants