-
Notifications
You must be signed in to change notification settings - Fork 107
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
[로또] 이홍진 미션 제출합니다. #84
base: main
Are you sure you want to change the base?
Conversation
1. 프로젝트 개요 2. 프로젝트 흐름 3. 기능 요구사항 4. 프로그래밍 요구사항 5. 기능 목록 6. 오류 처리 7. 테스트
1. 사용자에게 입력을 받는 InputView 생성 - 반복되는 Console.readLine()과 println()을 기능별로 나눔 2. LottoFactory를 통해 로또를 생성 - 투입한 금액에 대한 정보 제공 - 생성될 로또의 개수에 대한 정보 제공 - 로또 생성 - 투입 금액에 대한 검증
기능 목록 1. 입력 : 숫자로 입력 2. 로또 발행 전체 완료
1. Lotto class를 이용해 Lotto 번호, 로또 당첨 등수, 로또 숫자에 대한 유효성 검증를 제공 2. 로또 번호가 5개 일치할 경우 보너스 숫자 포함 여부에 따라 2등과 3등을 구분
1. 로또 당첨번호와 보너스 번호 입력 UI 추가
1. 기존 로또 발행시 정렬이 안되있었는데 오름차순으로 정렬
1. 발행된 로또를 통해 당첨된 로또가 몇개인지, 총 당첨액이 얼마인지, 로또의 총 수익률이 몇 퍼센트인지 계산해주는 LottoResultCalculator 추가 2. 로또 한장의 결과를 저장하는 LottoResult 추가
1. LottoResultCalculator를 통해 계산된 로또 당첨 통계 표기 2. 구매금액 대비 수익에 대한 수익률을 소수점 둘째자리에서 반올림하여 표기
1. 완료한 기능 목록 체크 - 입력: 로또번호, 보너스 번호 - 당첨 통계: 전체 - 수익률: 전체 2. 새 기능목록 추가 - 로또 발행: 로또의 정렬 - 출력
1. 메서드명 변경 - 로또의 순이익을 표기하는 net profit으로 변경 2. yield를 계산할때 netProfit이 정수여서 purchaseAmount가 더 큰 값이면 0으로 계산되는 오류 수정 - netProfit 타입을 float로 변경
1. 메서드명 변경 - 로또의 순이익을 표기하는 net profit에서 총 수익인 total earnings로 변경
1. 검증 함수를 object로 Validator로 옮겨서 사용
1. 검증 함수를 object로 Validator로 옮겨서 사용 2. 검증된 입력을 사용하므로 init 제거 3. purchaseAmount 제공하는 메서드 추가
1. 사용하지 않는 import 제거
1. 프로그램을 실행하는 함수들이 있는 컨트롤러 추가 - payment: 구매금액을 받고 로또를 생성 - giveLotteries: 생성된 로또의 갯수, 로또 정보를 보여줌 - inputWinningNumber: 해당 주차 당첨 로또의 당첨 번호를 입력, console 종료 - showResult: 당첨 로또의 갯수 및 총 수익률을 보여줌 2. controller의 run함수로 main에서 주 프로그램 실행
1. constant 패키지 생성으로 const를 InputView에서 분리해 관리
1. LottoNumberValidator: 로또 숫자 입력에 대한 Validator 2. PurchaseAmountValidator: 구매금액에 대한 Validator 3. BonusNumberValidator: 보너스 숫자 입력에 대한 Validator
1. ErrorMessages: 에러메세지에 대한 표기 2. LottoRules: 로또에 대한 규칙 상수 - 검증에 대한 다양한 제한값 - 승리 조건에 대한 고정값 - 승리 결과에 대한 고정값 3. OutputConst: 로또 결과를 표기
1. 상수값을 분리 2. LottoResultCalculator - 상금에 대한 이름을 변경: price -> prize
1. LOTTO_NUMBER_NUMERIC - 구입 금액은 숫자여야 합니다. -> 로또 번호는 자연수여야 합니다. 2. 각 에러에 맞게 공백을 나눔
1. 오류 처리: 10억원을 넘길 경우 - 구입 금액은 1억원 이하여야 합니다. -> 구입 금액은 10억원 이하여야 합니다. 2. 총 수익률 계산 공식 - (수익/투자금) * 100
1. LottoRank - 등수가 갖는 정보를 포함, 로또 등수, 상금 금액, output에서 보여줄 해당 등급의 표기
1. Lotto - 직접 계산하던 로또 등수를 LottoRank에서 수행후 반환 2. LottoResultCalculator - LottoRank의 추가로 당첨 횟수를 계산하는 로직들이 LottoRank를 이용 - 상금계산도 enum class를 이용 - getLottoYield 위치 변경 3. OutputView - LottoRank를 이용해 불필요한 when을 사용하지 않음
1. LottoRules - Count format을 추가: 1,000단위로 쉼표제공 2. OutputConst - 입력 파라미터값 String으로 변경 3. OutputView - count 포멧 변경값을 표기
1. 기존 기능목록에 대해 테스트 일정 작성 -> class 별 메소드에 대한 테스트로 변경
1. 초기화 검증 테스트 - 로또 개수 테스트, 초과일 경우, 미만일 경우 - 로또 범위 테스트, 1미만일 경우, 45초과일 경우 - 중복 숫자 테스트 2. 불변성 테스트 - 객체 생성후 객체 내부값은 불변 확인 3. 로또 등수 테스트 - 당첨일 경우 - 꽝일 경우
1. class나 object에 프로퍼티가 없을 경우 class 명 밑에 공백 한줄 추가
1. class나 object에 프로퍼티가 없을 경우 class 명 밑에 공백 한줄 추가 2. 주석 제거
1. 구매 금액 불변성 테스트 2. 로또 발행 테스트 - 발행 로또 갯수는 금액에 따라 결정 - 최대 금액에 대해 빌드 가능한지 테스트
1. 로또 결과 테스트 - 1등부터 5등까지 당첨 갯수 확인 - 당첨이 안되면 0으로 표기됨을 확인 2. 로또 수익률 테스트 - 결과 예시에서 나온 계산방식이 맞는지 확인 - 1000단위 구분과 소수점 계산 확인 3. 테스트 결과 Float 타입은 소수점 계산에 오류가 생김 -> Double타입으로 변경
1. 빈 값 입력 테스트 - 에러 메세지 확인 2. 숫자가 아닌 입력 테스트 3. 구입 금액의 천 단위 테스트 4. 금액 범위를 넘어간 경우 - 기존 10억만 검사하던 기존과 달리 1000~10억으로 변경
1. ~ 예외가 발생한다
1. 입력 문자열 테스트 - 구분자 테스트 - 자연수가 아닌경우 테스트 2. 로또 번호 테스트 - 로또 번호의 개수 테스트 - 로또 번호 범위 테스트 - 로또 번호 중복 숫자 테스트 3. 구분자 추가 - 쉼표와 쉼표뒤 공백 추가
1. 구분자 삭제 - 쉼표와 쉼표뒤 다시 삭제
1. 구분자 삭제 - 쉼표와 쉼표뒤 다시 삭제
1. 보너스 번호 입력에 대한 테스트 추가 - 입력값이 정수가 아닌 경우 - 번호 범위 예외 - 중복일 경우
1. 테스트 코드 작성 완료로 인한 체크
1. model 패키지로 이동
1. 기능 테스트 2개 추가 2. 예외 테스트는 나눠서 실행 했으므로 추가하지 않음
1. 기존 forEach를 통해 각각 계산해 넣는 방식과 달리 sumOf를 이용해 더 간단히 각각 계산
1. 프로젝트 구조 추가
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상수들을 엄청 꼼꼼하게 나눠두신 부분이 인상깊네요!
코드 잘 보고 갑니다 3주차도 고생하셨습니다 :)
- 숫자로 입력 | ||
- 숫자가 아닐 경우: "[ERROR] 구입 금액은 숫자여야 합니다." | ||
- 1,000으로 나눠 떨어지지 않을 경우: "[ERROR] 구입 금액은 1,000원 단위여야 합니다." | ||
- 10억원을 넘길 경우: "[ERROR] 구입 금액은 10억원 이하여야 합니다." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
최대 구매 금액을 10억원으로 설정하신 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
처음엔 10만원으로 제한을 두려했다가, '로또 발행을 많이 했을때 처리속도가 얼마나 될까?' 라는 단순한 생각으로 21억을 안넘는 이쁜숫자 10억으로 설정했어요!
class LottoFactory(private val purchaseAmount: Int) { | ||
val lottoQuantity: Int = purchaseAmount / LottoRules.AMOUNT_UNIT | ||
|
||
fun getPurchaseAmount(): Int = purchaseAmount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
purchaseAmount 매개변수를 private val로 설정해두고 아래에서 따로 public 함수인 getPurchaseAmount()
으로 만들어서 반환하도록 만드신 이유가 있을까요?
매개변수를 public으로 두면 굳이 함수를 만들지 않아도 되지않을까 싶어서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와 그렇네요? 처음에 Lotto
클래스와 비슷하게 만들다 보니까 private
를 건들면 안된다고 생각한거 같습니다..
} | ||
|
||
private fun showResult() { | ||
val lottoResults = lottoResultCalculator.getLottoResults(winningNumbers, bonusNumber!!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bonusNumber가 bonusNumber: Int? = null
로 nullable하게 정의되어 있는데 여기에선 !!로 설정을 해두셔서 처음부터 null을 허용하지 않도록 만드는 게 더 안전할 것 같습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Model 설정 및 활용도가 돋보였던거 같습니다!
3주차도 수고하셨습니다 ! 남은 4주차도 화이팅입니다 ㅎㅎ
#41 리뷰 한번 부탁드립니다 ㅎㅎ
const val THREE_MATCHED = "3개 일치 (5,000원)" | ||
const val FOUR_MATCHED = "4개 일치 (50,000원)" | ||
const val FIVE_MATCHED = "5개 일치 (1,500,000원)" | ||
const val FIVE_MATCHED_WITH_BONUS = "5개 일치, 보너스 볼 일치 (30,000,000원)" | ||
const val SIX_MATCHED = "6개 일치 (2,000,000,000원)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rank의 Prize를 활용하여 %d 형식으로 범용적인 스트링으로 만드는 것도 좋을거 같아요!
|
||
class InputView { | ||
|
||
private fun getInput(message: String): String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
각 상황에 따라 메시지 적용 할 수 있어서 좋네요 !
); | ||
|
||
companion object { | ||
fun fromMatchedCount(matchedCount: Int, bonusMatched: Boolean = false): LottoRank { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 다른 클래스에서도 fromMatchedCount를 접근하기 위해서 companion object로 선언한걸까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 LottoRank를 사용하는 곳에서 사용하기 위해 선언했습니다
private lateinit var lottoFactory: LottoFactory | ||
private lateinit var lottoResultCalculator: LottoResultCalculator | ||
private lateinit var winningNumbers: List<Int> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 아이들인 일부러 사용하시는 함수에서만 사용하시려구 지연 초기화 하신걸까요?
너무 좋은 방법이네요 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이거 좋은 것 같아요! 제 코드에서는 맨날 초기화하라구 떴었는데 ㅎㅎ 배워갑니다
const val AMOUNT_IS_NOT_EMPTY = "$ERROR_HEADER 구입 금액은 빈 값이 될 수 없습니다." | ||
const val AMOUNT_IS_NUMERIC = "$ERROR_HEADER 구입 금액은 숫자여야 합니다." | ||
const val AMOUNT_IS_MULTIPLE_OF_THOUSAND = "$ERROR_HEADER 구입 금액은 1,000원 단위여야 합니다." | ||
const val OUT_OF_AMOUNT_RANGE = "$ERROR_HEADER 구입 금액은 1,000원 이상 10억원 이하여야 합니다." | ||
|
||
const val LOTTO_NUMBER_NUMERIC = "$ERROR_HEADER 로또 번호는 자연수여야 합니다." | ||
const val LOTTO_NUMBER_DELIMITER = "$ERROR_HEADER 로또 번호는 쉼표로 구분해야 합니다." | ||
const val LOTTO_NUMBER_COUNT = "$ERROR_HEADER 로또 번호는 6개여야 합니다." | ||
const val LOTTO_NUMBER_RANGE = "$ERROR_HEADER 로또 번호는 1부터 45 사이의 숫자여야 합니다." | ||
const val LOTTO_NUMBER_UNIQUE = "$ERROR_HEADER 로또 번호는 중복될 수 없습니다." | ||
|
||
const val BONUS_NUMBER_NUMERIC = "$ERROR_HEADER 보너스 번호는 자연수여야 합니다." | ||
const val BONUS_NUMBER_RANGE = "$ERROR_HEADER 보너스 번호는 1부터 45 사이의 숫자여야 합니다." | ||
const val BONUS_NUMBER_UNIQUE = "$ERROR_HEADER 보너스 번호는 로또 번호와 중복될 수 없습니다." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기도 enum class를 사용하면 어떨까 싶어요! 문제 요구사항이 아무래도 enum class를 사용하는걸 많이 선호하는 것 같아서요 !
const val PURCHASE_AMOUNT_INPUT = "구입금액을 입력해 주세요." | ||
const val WINNING_NUMBER_INPUT = "\n당첨 번호를 입력해 주세요." | ||
const val BONUS_NUMBER_INPUT = "\n보너스 번호를 입력해 주세요." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기두 물론 상수로 관리해도 좋지만 이왕 요구사항 조금 더 따른다는 의미로 enum class로 관리하면 좋을 것 같네요 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵! 다른분들 코드를 보다 보니 enum class로 관리하니깐 훨씬 좋아보이더라고요! 이번주차에는 그렇게 도전하려 합니다!
감사합니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 잘하셔서 크게 의견 남길게 없네요 ㅎㅎ
상수들만 enum class로 수정하면 더욱 좋을 것 같은 의견입니다
마지막 주차도 화이팅 해요 ! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3주차도 고생 많으셨습니다~
마지막까지 같이 힘내봐요!
class OutputView { | ||
|
||
fun showErrorMessage(errorMessages: String) { | ||
println(errorMessages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun showErrorMessage(errorMessages: String) = println(errorMessages)
개인적인 의견인데, 출력만 하는 간단한 함수는 위에 처럼 표현식을 사용하면 엄청 깔끔하더라구요!
} | ||
|
||
private fun validateMaxAmount(amount: Int) { | ||
require(amount in LottoRules.AMOUNT_RANGE) { ErrorMessages.OUT_OF_AMOUNT_RANGE } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 코드가 최소한의 기능 단위로 분할 한다는 것을 잘 표현한 코드 같아요!
참고하기 좋은 코드 같습니다. 잘 배우고 갑니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔한코드 잘봤습니다. 에러처리 부분이 인상적이네요! 한 주 동안 고생하셨습니다.
private lateinit var lottoFactory: LottoFactory | ||
private lateinit var lottoResultCalculator: LottoResultCalculator | ||
private lateinit var winningNumbers: List<Int> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이거 좋은 것 같아요! 제 코드에서는 맨날 초기화하라구 떴었는데 ㅎㅎ 배워갑니다
) | ||
lottoFactory = LottoFactory(purchaseAmount) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
give lottaries가 무슨 뜻일까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로또의 복수형을 검색해보다가 lotteries라고 표현하는글을 참고해서 lotteries라고 표기했습니다!
const val SIX_MATCHED = "6개 일치 (2,000,000,000원)" | ||
|
||
private const val LOTTO_COUNT = "- %s개" | ||
fun matchedLotteries(count: String): String = LOTTO_COUNT.format(count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상수만 모아둔 곳인 것 같은데 함수가 들어가야하는 이유가 있을까요?
다른 분 코드 보니까 그냥 LOTTO_COUNT출력하는 부분에서 바로 .format(~~) 이렇게 사용해버리더라구요. 참고하셔도 좋을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상수에 대한 함수여서 사용했습니다.
좋은 제안 감사합니다~!!
지금보니 enum class로 사용하는 것도 좋을 것 같네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각 못했던 방법을 많이 얻어갈 수 있는 코드 리뷰였습니다. 특히 input 검증 과정에서 컨트롤러에 private 함수 하나 만들어두고 어떤 input이냐에 따라 다른 validate 함수를 인자로 주입하는 방식에서 정말 많이 배울 수 있었습니다. 수고 많으셨습니다!!
class BonusNumberValidatorTest { | ||
private val winningNumberTest = listOf(1, 2, 3, 4, 5, 6) | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = ["a", "#", "", "0.1"]) | ||
fun `입력 값이 정수가 아닌 경우 예외가 발생한다`(input: String) { | ||
val exception = assertThrows<IllegalArgumentException> { BonusNumberValidator.getValidatedBonusNumber(input, winningNumberTest) } | ||
|
||
assertThat(exception).hasMessage(ErrorMessages.BONUS_NUMBER_NUMERIC) | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = ["0", "46"]) | ||
fun `보너스 번호가 범위를 넘어가면 예외가 발생한다`(input: String) { | ||
val exception = assertThrows<IllegalArgumentException> { BonusNumberValidator.getValidatedBonusNumber(input, winningNumberTest) } | ||
|
||
assertThat(exception).hasMessage(ErrorMessages.BONUS_NUMBER_RANGE) | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = ["1", "2", "3", "4", "5", "6"]) | ||
fun `보너스 번호와 당첨 번호에 중복된 숫자가 있으면 예외가 발생한다`(input: String) { | ||
val exception = assertThrows<IllegalArgumentException> { BonusNumberValidator.getValidatedBonusNumber(input, winningNumberTest) } | ||
|
||
assertThat(exception).hasMessage(ErrorMessages.BONUS_NUMBER_UNIQUE) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 코드 작성하는 것이 많이 서툰데 이런식으로 인자를 전달할 수 있군요!! 다음 과제에서 이부분을 저도 적용시켜봐야겠어요!
outputView.showYield(yield) | ||
} | ||
|
||
private fun <T> getValidatedInput( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validate 함수를 주입하는 방식이 정말 좋은 것 같아요!! 배울 점이 많은 코드인 것 같습니다!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3주차 너무 고생하셨어요... 전체적으로 변수명이 눈에 잘 들어와서 깔끔한 것 같아요
4주차도 파이팅해요!!
package lotto.util.constant | ||
|
||
object ErrorMessages { | ||
private const val ERROR_HEADER = "[ERROR]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 중복 처리 하는 거 사소하지만 깔끔해 보여서 좋은 것 같아요!!
3주차 로또
2주차 코드 리뷰
controller
내부 함수 역할 분배model
의 속성과 행동에 따라 명명하기2주차 공통 피드백
class
컨벤션 순서 지키기프로젝트 구조
기능 목록
입력
로또 발행
Randoms.pickUniqueNumbersInRange(1, 45, 6)
당첨 통계
출력
오류 처리