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

step3: 로또(2등) 구현 #948

Open
wants to merge 13 commits into
base: kdh85
Choose a base branch
from
Open

step3: 로또(2등) 구현 #948

wants to merge 13 commits into from

Conversation

kdh85
Copy link

@kdh85 kdh85 commented Nov 15, 2023

안녕하세요, 윤혁님!

일전에 주신 리뷰 코맨드 최대한 이해한 기준으로 반영해보았습니다.!

제가 맞게 이해했는지 리뷰 부탁드려요.

항상 감사합니다.

kdh85 added 13 commits November 14, 2023 15:50
* 리뷰 머지시 나온 피드백들을 수용한 수정사항을 반영함.
* 요구사항을 도메인 기준으로 정리.
* 로또와 보너스번호로 구성된 당첨로또 객체를 정의함.
* 로또의 번호 일치수와 보너스번호 일치여부를 통해서 해당 로또의 당첨순위를 반환하는 책임을 가짐.
* 기존에 Record를 생성해서 반환하는 책임을 제거.
* 2등 보너스 등급을 추가.
* 등급별 일치하는 수와 보너스카운트 여부를 받아서 등급을 반환하는 책임 추가.
* 기존에 Record, LotteryReuslt의 책임을 재설계한 신규 등급관련 클래스를 정의함.
* 등급랭크와 랭크별 수량을 다루는 책임을 가지는 LottoRecord클래스를 구현.
* 등급랭크의 수량을 1씩 증가시키는 책임을 가짐.
* LottoRecord의 일급콜랙션 LottoRecords를 구현함.
* 기존 LotteryResult가 가지고 있던 당첨통계에 대한 책임을 그대로 이관함.
* 기존에 로또번들과 당첨로또를 주입해서 결과를 계산하던 부분을 계산결과를 받아 통계를 생성하도록 개선.
* 개선된 구조번경으로 인한 미사용 클래스 삭제.
* 로또안에 보너스 번호가 일치하는 경우가 있는지를 검증.
* 로또들의 당첨번호 수, 보너스번호 일치여부를 기준으로 당첨등급들을 찾아서 반환한다.
* 검증시 0을 포함하도록 수정.
* 수량을 증가량에 따라 한번에 증가시키도록 개선.
* 일치하는 랭크의 수만큼 수량을 집계하도록 수정.
* 보너스볼 입력을 추가함.
* 당첨통계 클래스 개선에 따라 출력문을 수정.
* 보너스볼 당첨내용이 추가됨에 따라 메세지 포멧 분기처리하도록 개선.
Copy link
Member

@malibinYun malibinYun left a comment

Choose a reason for hiding this comment

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

리뷰가 늦어져 죄송합니다 🥲
생각해볼만한 곳에 코멘트 남겨두었으니 확인해주세요!
피드백 반영 후 다시 리뷰 요청 부탁드립니다 :)

Comment on lines +24 to +35
fun matchCounts(bundle: List<Lotto>): List<Int> {
val counts = mutableListOf<Int>()
bundle.forEach { lotto ->
val countByNumbers = lotto.makeMatchCountByNumbers(this)//0~6개

val rank = Rank.values().firstOrNull {
it.matchCount == countByNumbers
} ?: Rank.NONE_RANK

counts.add(countByNumbers)
}
return counts
Copy link
Member

Choose a reason for hiding this comment

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

로또 티켓이 당첨인지에 대한 검증은 당첨 티켓이 갖게해보는 건 어떨까요?

현실 세계에서는 사람이 당첨번호를 보고, 티켓이 몇등인지 알아내지만,
당첨 티켓에게 비교할 로또를 주면서 어떤 결과인지 메시지를 던져 물어보는 건 컴퓨터의 세계에서 자연스러운 일이라 생각해요!

Comment on lines +38 to +39
fun isMatchBonus(bonusNumber: LottoNumber): Boolean {
return lottoNumbers.contains(bonusNumber)
Copy link
Member

Choose a reason for hiding this comment

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

"보너스 번호"를 가지고 있는지는 로또 티켓의 관심사일까요?
로또 티켓은 특정 번호를 가지고있는지 정도의 역할만 가지고 있어도 충분해보여요!
특별히 보너스 번호라는 관심을 갖지 않게, hasNumber 같은 이름으로 변경해보는 건 어떨까요?

Comment on lines +5 to +8
class LottoRecord(
val rank: Rank,
quantity: Int = 0,
) {
Copy link
Member

Choose a reason for hiding this comment

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

기록에 대한 객체를 만든뒤 거기에 계속 수를 더하는 가변 객체를 만들기 보다,
계산이 모두 끝난 뒤 마지막 snapshot을 저장하도록 불변 객체로 만들어보는 건 어떨까요?
기록 객체가 중간에 참조되어서 수정되는 것을 방지할 수 있겠어요 :)

Comment on lines +9 to +16
init {
require(quantity >= 0) {
"등급별 수량은 0이상 입니다."
}
}

var quantity = quantity
private set
Copy link
Member

Choose a reason for hiding this comment

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

멤버변수는 생성자 보다 위에 위치하는 것이 코틀린 컨벤션입니다 :)

Comment on lines 7 to 12
FOURTH_RANK(3, 5_000),
THIRD_RANK(4, 50_000),
SECOND_RANK(5, 1_500_000),
SECOND_BONUS_RANK(5, 3_000_000),
FIRST_RANK(6, 2_000_000_000),
NONE_RANK(0, 0),
Copy link
Member

Choose a reason for hiding this comment

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

5개 번호가 맞고, 보너스 번호가 맞은 것은 2등,
5개 번호가 맞고, 보너스 번호가 다른 것은 3등입니다.
도메인 규칙을 이름에 녹여내보는 건 어떨까요?

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.

2 participants