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

2단계 - 로또(자동) #563

Open
wants to merge 28 commits into
base: relkimm
Choose a base branch
from
Open

2단계 - 로또(자동) #563

wants to merge 28 commits into from

Conversation

relkimm
Copy link

@relkimm relkimm commented Dec 17, 2022

안녕하세요 길환님! 오랜만에 뵙게 되었네요😅
리뷰 요청 드립니다~🙇‍♂️

relkim-m and others added 28 commits December 16, 2022 15:52
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.

안녕하세요! 코드 잘 봤습니다.
리뷰 확인 부탁드려요!


override fun draw() {
println("지난 주 당첨 번호를 입력해 주세요.")
this.value = readLine()!!

Choose a reason for hiding this comment

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

Comment on lines +3 to +5
class WinningNumberInput(value: String = "") : UI {
var value: String = value
private set

Choose a reason for hiding this comment

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

변수를 제거해도 괜찮지 않을까요?
draw를 했을 때 함수 내부에서 getNumbers를 호출해서 반환해도 충분해보여요.

.filter { it.isNotBlank() }
.map {
val trimmed = it.trim()
trimmed.toIntOrNull() ?: throw IllegalStateException()

Choose a reason for hiding this comment

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

toInt만을 호출해도 좋을것 같아요. 안에서 더 좋은 Exception을 발생시킬거 같습니다!


import lotto.LottoTicket

class PurchaseCount(payment: Int) : UI {

Choose a reason for hiding this comment

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

생성자에서는 count를 받고, 팩터리 메서드를 활용해서 돈을 입력받아 count를 만든다는 함수를 만들어보는건 어떨까요?
핵심은 payment가 아닌데, 주 생성자에 오는게 아쉬워요!

Comment on lines +7 to +10
override fun draw() {
println("구입금액을 입력해 주세요.")
this.value = readLine()!!
}

Choose a reason for hiding this comment

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

다른 Input과 같은 리뷰입니다! 아마 UI라는것에 의존해서 그런것 같은데, generic을 이용하면 충분히 리턴값도 변경하면서 UI를 활용할 수 있을거 같아요.

Comment on lines +3 to +5
fun main() {
LottoGame.run()
}

Choose a reason for hiding this comment

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

여기서 입력/출력을 하고, 입력값을 LottoGame에 메서드를 정의해서 호출해보는건 어떨까요?

Comment on lines +13 to +18
fun from(matchCount: Int): LottoTicketStatus {
if (matchCount >= MATCH_COUNT_FOR_WIN) {
return WIN
}
return NOT_WIN
}

Choose a reason for hiding this comment

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

이 부분은 고민해볼까요?~ LottoTicketStatus의 각각의 값에는 MATCH_COUNT_FOR_WIN과 관련된 부분이 없어보여요! 이 메서드를 호출하는 부분이 알고 있으면 더 좋지 않을까요?

Comment on lines +19 to +21
.fold(0) { winningMoney, rank ->
winningMoney + rank.money
}

Choose a reason for hiding this comment

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

sum을 사용해봅시다!

this.tickets.forEach { it.match(winningNumber = winningNumber) }
}

fun getTickets(): List<LottoTicket> = this.tickets

Choose a reason for hiding this comment

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

get메서드를 따로 만드시지 않고, private으로 선언 된 변수 public으로 해제를 해제는 편이 더 낫습니다

}
}

fun getReturnRate(): Double {

Choose a reason for hiding this comment

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

수익률이라는 단어를 사용해볼까요? 또는 보상이라던지요!

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