-
Notifications
You must be signed in to change notification settings - Fork 112
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
[로또] 이지은 미션 제출합니다. #82
base: main
Are you sure you want to change the base?
Conversation
로또 번호 6개 입력, 1부터 45 사이의 숫자인지, 중복된 값이 없는지 검사
빈값이 들어왔는지, 모두 숫자인지, 0보다 큰 값인지, 1000원 단위인지 검사
빈값을 입력했는지, 6개 값을 모두 입력했는지, 1~45 범위 내에 있는지, 중복되는 값이 있는지 검사
빈값을 입력했는지, 1~45 범위 내에 있는지, 로또 번호와 중복되는지 유효성 검사
private val validator = Validator() | ||
private val parser = Parser() | ||
private val inputView = InputView(validator, parser) | ||
private val resultView = ResultView() | ||
private val lottoGame = LottoGame() |
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.
MVC 모르는 문외한이라 Controller가 내부 요소들을 직접 인스턴스화하는 역할이 있는걸까요 ?
없다면 생성자에서 주입하지 않고 직접 인스턴스를 생성하신 이유가 있으실까요 ?
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.
작은 프로그램에선 Controller가 내부 요소들을 직접 인스턴스화 하기도 하고, 사실 저는 main에서 어떠한 설정 없이 바로 Controller를 통해 프로그램을 시작해주고 싶어서 내부에서 인스턴스를 생성하도록 했습니다!
그런데 유지보수성이나 앞으로 더 큰 프로그램을 작성할 땐 생성자에서 주입하는 방향이 더 유용할 것 같네요 :)
const val ERROR_INVALID_WINNING_NUMBERS_COUNT = "[ERROR] 로또 번호는 6개여야 합니다." | ||
const val ERROR_INVALID_WINNING_NUMBERS_RANGE = "[ERROR] 로또 번호는 1부터 45 사이여야 합니다." | ||
const val ERROR_DUPLICATE_WINNING_NUMBERS = "[ERROR] 로또 번호는 중복될 수 없습니다." | ||
const val ERROR_EMPTY_WINNING_NUMBERS = "[ERROR] 당첨 번호가 비어 있습니다." | ||
const val ERROR_INVALID_DELIMITER = "[ERROR] 당첨 번호는 쉼표(,)로 구분해야 합니다." | ||
|
||
const val ERROR_INVALID_PURCHASE_AMOUNT = "[ERROR] 로또는 1,000원 단위로 입력해야 합니다." | ||
const val ERROR_EMPTY_PURCHASE_AMOUNT = "[ERROR] 구매 금액이 비어 있습니다." | ||
const val ERROR_NOT_A_NUMBER = "[ERROR] 구매 금액은 숫자여야 합니다." | ||
const val ERROR_OUT_OF_RANGE = "[ERROR] 구매 금액이 정수의 범위를 벗어납니다." | ||
|
||
const val ERROR_INVALID_BONUS_NUMBER = "[ERROR] 보너스 번호는 1부터 45 사이여야 합니다." | ||
const val ERROR_DUPLICATE_BONUS_NUMBER = "[ERROR] 보너스 번호가 당첨 번호와 중복됩니다." | ||
const val ERROR_EMPTY_BONUS_NUMBERS = "[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.
[ERROR] 요 부분은 공통되는 부분이라 따로 분리해도 될 것 같네요!
require(numbers.size == 6) { ExceptionConstants.ERROR_INVALID_WINNING_NUMBERS_COUNT } | ||
require(numbers.all { it in 1..45 }) { ExceptionConstants.ERROR_INVALID_WINNING_NUMBERS_RANGE } | ||
require(numbers.distinct().size == 6) { ExceptionConstants.ERROR_DUPLICATE_WINNING_NUMBERS } |
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.
이제 보니 Constant 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.
선언해두고 다른 클래스들에선 사용했는데 여기 부분을 놓쳤네요.. ㅎㅎ
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. 로또 번호 생성 | ||
- [x] 로또 번호를 생성 | ||
- [x] `Randoms.pickUniqueNumbersInRange(1, 45, 6)` 이용 | ||
- 제공된 `Lotto` 클래스를 사용하여 구현 |
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 class
에서는 난수생성을 하고 있지 않네요.
LottoGame class
에서 해당 작업을 진행중입니다!
2주차 공통 피드백에서 기능 구현 중 바뀐 목록도 지속적으로 업데이트 해달라는 피드백이 있어서 알려드립니다!
val lottoResult = lottoGame.calculateResult(lottos, winningNumbers, bonusNumber) | ||
|
||
resultView.displayResult(lottoResult) | ||
} |
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.
저도 이전에 받았던 리뷰인데, start
함수에서도 메서드를 나눠 함수가 하나의 역할을 수행하도록 하는게 좋다네요.
예를 들어 input
을 받는 setUp()
, 결과를 주는 showResult()
와 같이 분리하시더라고요!
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.
start
함수에 대해서 고민했던 부분인데 피드백 감사합니다 !
} | ||
} | ||
|
||
val purchaseAmount = lottos.size * Constants.LOTTO_UNIT_PRICE |
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 fun Double.roundToTwoDecimalPlaces(): Double { | ||
return String.format("%.2f", this).toDouble() |
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.
"%.2f"
는 소수점 둘째자리까지 나타내는 것이므로 소수점 셋째자리에서 반올림 하는 포멧입니다!
README.md에서는 소수점 둘째자리에서 반올림한다고 되어있으니 "%.1f"
가 올바른 포멧같습니다.
그리고 이런 포멧도 상수화해도 될 것 같아요!
|
||
return 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.
while
, try-catch
를 통해 입력, 유효성 검사, 반환 하는 로직으로 중복되는 감이 있어서 하나의 함수로 통일 해보는건 어떨까요?
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]
로 시작하는 에러 문구 출력)Randoms.pickUniqueNumbersInRange(1, 45, 6)
이용Lotto
클래스를 사용하여 구현[ERROR]
로 시작하는 에러 문구 출력)[ERROR]
로 시작하는 에러 문구 출력)당첨금액 / 구입금액 * 100
으로 수익률 계산