Skip to content

Comments

🚀 4단계 - 로또(수동)#1137

Open
Aimbe wants to merge 13 commits intonext-step:aimbefrom
Aimbe:step4
Open

🚀 4단계 - 로또(수동)#1137
Aimbe wants to merge 13 commits intonext-step:aimbefrom
Aimbe:step4

Conversation

@Aimbe
Copy link

@Aimbe Aimbe commented Dec 20, 2024

안녕하세요 경록님!마지막 4단계 제출합니다!
항상 피드백 감사합니다!🙇‍♂️

Copy link

@Rok93 Rok93 left a comment

Choose a reason for hiding this comment

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

정호님 4단계 미션 잘 구현해주셨습니다. 👍
이제 로또 미션 종료도 얼마 남지 않았군요 😎👏

조금 더 짚고넘어가면 좋을 부분들이 보여서 코멘트 남겨두었습니다!
확인해서 반영 부탁드릴게요. 😁
이제 정말 거의 끝나가니 마지막까지 화이팅입니다. 💪💪💪

val manualCount = InputView().readManualLottoCount()
val manualLottos = InputView().readManualLottoNumbers(manualCount)

val lottos = LottoService(LottoPurchaseManager()).purchase(purchaseAmount, manualLottos)
Copy link

Choose a reason for hiding this comment

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

현재 LottoService 객체를 두번 생성해서 사용하고있는데요.
LottoService 객체는 한번 생성한 뒤에 재사용하면 어떨까요?

val purchaseAmount = LottoPrice(InputView().readPurchaseAmount())

val manualCount = InputView().readManualLottoCount()
val manualLottos = InputView().readManualLottoNumbers(manualCount)
Copy link

Choose a reason for hiding this comment

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

View가 Model에 의존하고있는 것 같아요. 🤔
View 단에서는 입력 값으로 List<List<Int>>의 결과를 가져오고, 이 결과는 Controller에서 변환해주도록 해주면 좋을 것 같습니다.

Copy link

Choose a reason for hiding this comment

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

추가적으로 이 수동(으로 입력 받은) 로또를 생성해주는 역할은 어떤 객체에게 위임할지도 고민해보시면 좋을 것 같아요. 😃


import Lottos

class LottoPurchaseManager {
Copy link

Choose a reason for hiding this comment

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

LottoPurchaseManager의 경우 별도의 상태 값을 가지지 않는데, object class로 선언해도 되지 않을까요? 🤔

Comment on lines +27 to +29
operator fun plus(lotto: Lottos): List<Lotto> {
return tickets + lotto.tickets
}
Copy link

Choose a reason for hiding this comment

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

아래와 같이 리턴 타입을 Lottos로 변경해보면 어떨까요? 😃

Suggested change
operator fun plus(lotto: Lottos): List<Lotto> {
return tickets + lotto.tickets
}
operator fun plus(lotto: Lottos): Lottos {
return Lottos(tickets + lotto.tickets)
}

Comment on lines +42 to +50
val manualLottos =
(1..count).map {
val numbers =
readln().split(",")
.map { it.trim().toInt() }
.map { LottoNumber(it) }
Lotto(numbers)
}
return Lottos.from(manualLottos)
Copy link

Choose a reason for hiding this comment

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

(이미 위에서 언급했었지만) 실제로 입력 받은 값을 Domain Model인 Lotto 그리고 Lottos로 변경하는 로직은 Controller에 위치시키면 좋을 것 같습니다. 😉

Comment on lines +37 to +46
@Test
fun `2개 이하 맞추면 미당첨이다`() {
val rank2 = Rank.from(matchCount = 2, matchBonus = false)
val rank1 = Rank.from(matchCount = 1, matchBonus = false)
val rank0 = Rank.from(matchCount = 0, matchBonus = false)

assertThat(rank2).isEqualTo(Rank.NONE)
assertThat(rank1).isEqualTo(Rank.NONE)
assertThat(rank0).isEqualTo(Rank.NONE)
}
Copy link

Choose a reason for hiding this comment

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

하나의 테스트 케이스지만 다양한 인자에 대한 테스트는 Parameterized Test를 활용해서 조금 더 깔끔하게 변경해볼 수 있을 것 같아요. 😃

val purchasedLottos = LottoService().purchase(lottoPrice)
val manualLottos =
List(manualCount) {
Lotto(createLottoNumbers(1, 2, 3, 4, 5, 6))
Copy link

Choose a reason for hiding this comment

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

createLottoNumbers 가 아니라 처음부터 createLotto 메서드를 만들면 일일이 Lotto로 랩핑하는 로직들을 줄일 수 있을 것 같아요. 😃

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