Step2, 로또(자동) #1138
Open
Step2, 로또(자동) #1138
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BeokBeok
requested changes
Dec 23, 2024
| @@ -3,7 +3,7 @@ package calculator.model.input | |||
| object InputValidator { | |||
| fun validateNumbers(numbers: List<Int>) { | |||
| numbers.forEach { number -> | |||
| if (number < 0) throw IllegalArgumentException("음수는 계산할 수 없습니다.") | |||
| if (number == InputParser.NON_NUMERIC_DEFAULT_VALUE) throw IllegalArgumentException("음수는 계산할 수 없습니다.") | |||
Comment on lines
+13
to
+29
| monitor.displayLottoPurchaseAmount() | ||
| val totalPurchaseAmount = keyboard.inputLottoPrice() | ||
| val lottoCount = process.calculateLottoCount(totalPurchaseAmount) | ||
| monitor.displayLottoPurchasesCount(lottoCount) | ||
|
|
||
| val lottoTickets = process.generateLottoTickets(lottoCount) | ||
| monitor.displayIssuedLottoTickets(lottoTickets) | ||
|
|
||
| monitor.displayInputLastWeekLottoWinningNumbers() | ||
| val lastWeekNumbers = keyboard.inputLastWeekWinningNumbers() | ||
| val lottoStatistics = | ||
| process.calculateWinningStatistics( | ||
| lottoTickets, | ||
| lastWeekNumbers, | ||
| totalPurchaseAmount, | ||
| ) | ||
| monitor.displayLottoStatistics(lottoStatistics) |
There was a problem hiding this comment.
프로그래밍 요구사항에는 아래와 같은 내용이 있습니다.
함수(또는 메서드)의 길이가 15라인을 넘어가지 않도록 구현한다.
이 요구사항을 반영해보면 어떨까요?
| @@ -0,0 +1,15 @@ | |||
| package lotto.model.price | |||
|
|
|||
| class Lotto2024Price( | |||
There was a problem hiding this comment.
클래스명에 년도를 명시해야하는 이유가 있을까요?
그렇다면 해마다 클래스를 추가해야하는걸까요?
| override val price: Int = 1000, | ||
| ) : LottoPrice { | ||
| init { | ||
| require(price > 0) { |
Comment on lines
+25
to
+31
| val DEFAULT_RANK_PRICE = | ||
| mapOf( | ||
| 6 to 2_000_000_000, | ||
| 5 to 1_500_000, | ||
| 4 to 50_000, | ||
| 3 to 5_000, | ||
| ) |
There was a problem hiding this comment.
로또 번호가 일치하는 것에 대한 금액을 enum으로 관리해보면 어떨까요?
| @@ -0,0 +1,7 @@ | |||
| package lotto.view.keyboard | |||
|
|
|||
| interface Keyboard { | |||
There was a problem hiding this comment.
Keyboard 인터페이스를 구현하는 곳은 MarchineKeyboard 클래스만 존재하네요.
민수님은 언제 인터페이스 - 구현체 구조를 사용하시나요?
현재 구조에서는 Keyboard 인터페이스의 존재 의미가 적어보이는데 민재님은 어떻게 생각하시나요?
(Monitor도 동일)
Comment on lines
+51
to
+55
| if (profitRate > 1.0) { | ||
| println("총 수익률은 ${rate}입니다.") | ||
| } else { | ||
| println("총 수익률은 ${rate}입니다.(기준이 1이기 때문에 결과적으로 손해라는 의미임)") | ||
| } |
|
|
||
| @Test | ||
| fun `로또 번호는 1 ~ 45 사이여야 한다`() { | ||
| repeat(100) { |
There was a problem hiding this comment.
100번을 반복하는 것은 어떤 의미를 가질까요?
1~45 사이가 아닌 LottoNumbers를 생성하는 것에 대한 테스트를 작성해보는 것은 어떨까요?
| "3000,14000", | ||
| ], | ||
| ) | ||
| fun `구입금액, 구매가능 개수 테스트`(rawInput: String) { |
There was a problem hiding this comment.
3개부터 6개까지 일치했을 때의 얼마의 상금을 가져갈 수 있는지에 대해서 테스트 코드를 작성해보면 어떨까요?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR요청이 늦어져서 죄송합니다.
야근업무가 잦아져서 생각했던 시간만큼 과제를 진행하지 못한것 같습니다.
이번 PR요청에서 다급함에 선 테스트 코드 작성도 잘지켜지지 않아 아쉬움이 컸었으나,
시간이 너무 지체되는것 같아 우선 PR요청 드립니다.
이번 작업에서 아쉬웠던 부분은 LottoRank를 상속받는 다른 구현체로 대체될때,
DEFAULT_RANK_PRICE가 내부 속성으로 가져가고 싶었으나 지금은 방법이 떠오르지 않아 유지하였습니다.
LottoMachineProcess의 calculateWinningStatistics 메서드 내에서 LottoRank를 상속받아 구현한 DefaultLottoStatistics 객체를 생성하자니, 외부에서 쉽게 대체하긴 힘들겠다 생각되었습니다.
좋은 방법이 있으면 조언부탁드립니다.
PR 잘부탁드리겠습니다. 🙏