[정찬욱] 로또 미션 Step1 #2
Conversation
src/model/Lotto.js
Outdated
| class Lotto { | ||
| static MIN_NUMBER = 1; | ||
| static MAX_NUMBER = 45; | ||
| static NUMBERS_SIZE = 6; |
There was a problem hiding this comment.
LottoNumber 라는 친구를 만들어서 1~45에 대한 검사를 위임할 수 있을 것 같아요!
| static #createLottos(count) { | ||
| return Array.from({ length: count }).map(() => LottoMachine.#createLotto()); | ||
| } |
There was a problem hiding this comment.
| static #createLottos(count) { | |
| return Array.from({ length: count }).map(() => LottoMachine.#createLotto()); | |
| } | |
| static #createLottos(count) { | |
| return Array.from({ length: count }).map(LottoMachine.#createLotto); | |
| } |
아마 이렇게 표현해도 무방해보이네요!
src/model/LottoCalculator.js
Outdated
| class LottoCalculator { | ||
| static #LOTTO_PRIZES = { | ||
| 1: 2_000_000_000, | ||
| 2: 30_000_000, | ||
| 3: 1_500_000, | ||
| 4: 50000, | ||
| 5: 5000, | ||
| }; |
There was a problem hiding this comment.
"로또 계산기" 라는건 조금 어색해보여요!
LottoResult 처럼 표현하면 어떨까 싶기도..?
src/model/LottoCalculator.js
Outdated
| this.#winningCounts = LottoCalculator.#updateWinningCounts({ | ||
| winningCounts: this.#winningCounts, | ||
| lottos, | ||
| winningLotto, | ||
| }); | ||
| this.#rateOfReturn = LottoCalculator.#calcRateOfReturn( | ||
| this.#winningCounts, | ||
| price | ||
| ); |
There was a problem hiding this comment.
이렇게 두 개의 결과를 반환하고 있는데, 다르게 생각하면 두 개의 클래스로 분리할 수도 있지 않을까요!?
src/model/LottoCalculator.js
Outdated
| lotto.numbers, | ||
| winningLotto.numbers | ||
| ), | ||
| isMatchedBonusNumber: lotto.numbers.includes(winningLotto.bonusNumber), |
There was a problem hiding this comment.
| isMatchedBonusNumber: lotto.numbers.includes(winningLotto.bonusNumber), | |
| matchedBonusNumber: lotto.numbers.includes(winningLotto.bonusNumber), |
이렇게만 표현해도 좋아보이네요!
src/model/LottoCalculator.js
Outdated
| (counts, { matchedCount, isMatchedBonusNumber }) => { | ||
| const prizeMap = { | ||
| 6: 1, | ||
| 5: isMatchedBonusNumber ? 2 : 3, | ||
| 4: 4, | ||
| 3: 5, | ||
| }; | ||
|
|
||
| if (prizeMap[matchedCount]) { | ||
| counts[prizeMap[matchedCount]] += 1; | ||
| } | ||
|
|
||
| return counts; | ||
| }, |
There was a problem hiding this comment.
reduce에 쓰이는 콜백도 함수로 분리해주면 어떨까요!?
src/model/LottoMachine.js
Outdated
| static #getLottoNumbers() { | ||
| const lottoNumbers = []; | ||
| const addLottoNumber = (number) => { | ||
| const nextLottoNumbers = lottoNumbers.concat(number); | ||
| !isDuplicated(nextLottoNumbers) && lottoNumbers.push(number); | ||
| }; | ||
|
|
||
| while (lottoNumbers.length < Lotto.NUMBERS_SIZE) { | ||
| const lottoNumber = LottoMachine.#getLottoNumber(); | ||
|
|
||
| addLottoNumber(lottoNumber); | ||
| } | ||
|
|
||
| return lottoNumbers; | ||
| } |
There was a problem hiding this comment.
2주차 회고시간에 이야기했던 shuffle 참고!
src/model/LottoMachine.js
Outdated
| static #validatePrice(price) { | ||
| validate.integer(price, "로또 구입 금액으로 정수를 입력해야 합니다."); | ||
|
|
||
| throwErrorWithCondition( | ||
| price < LottoMachine.#PRICE_PER_ONE, | ||
| `로또 구입 금액은 ${LottoMachine.#PRICE_PER_ONE}원이상이어야 합니다.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
도메인에서는 에러를 발생하고 이때 에러 코드 같은걸 얹어주고, 뷰에서 에러 코드를 기반으로 메세지를 만들어주도록 하면 어떨까요?
There was a problem hiding this comment.
도메인에서의 에러 메세지를 [ERR_001] LottoMachine 클래스의 생성자 인수는 정수여야 합니다. 이렇게 작성하고, 뷰에서 해당 에러 메세지의 에러 코드를 파싱해서 에러 코드에 해당하는 사용자가 이해하기 쉬운 메세지를 띄워주는 방식은 괜찮을까요?
There was a problem hiding this comment.
그런 방법도 좋을 것 같아요 ㅋㅋ
가령 다국어 페이지를 만든다고 생각해보면 좋긴합니다!
| class WinningLotto extends Lotto { | ||
| #bonusNumber; | ||
|
|
||
| constructor(lottoNumbers, bonusNumber) { | ||
| super(lottoNumbers); | ||
|
|
||
| if (bonusNumber === undefined) { | ||
| return (bonusNumber) => new WinningLotto(lottoNumbers, bonusNumber); | ||
| } | ||
|
|
||
| WinningLotto.#validateBonusNumber(this.numbers, bonusNumber); | ||
|
|
||
| this.#bonusNumber = bonusNumber; | ||
| } | ||
|
|
||
| get bonusNumber() { | ||
| return this.#bonusNumber; | ||
| } | ||
|
|
||
| static #validateBonusNumber(winningNumbers, bonusNumber) { | ||
| validate.integer(bonusNumber, "보너스 번호는 정수여야 합니다."); | ||
|
|
||
| throwErrorWithCondition( | ||
| Lotto.isLessThanMinLottoNumber(bonusNumber), | ||
| `보너스 번호는 ${Lotto.MIN_NUMBER}이상이어야 합니다.` | ||
| ); | ||
|
|
||
| throwErrorWithCondition( | ||
| Lotto.isGreaterThanMaxLottoNumber(bonusNumber), | ||
| `보너스 번호는 ${Lotto.MAX_NUMBER}이하여야 합니다.` | ||
| ); | ||
|
|
||
| throwErrorWithCondition( | ||
| isDuplicated(winningNumbers.concat(bonusNumber)), | ||
| "당첨 번호 중에 보너스 번호와 중복되는 번호가 있습니다." | ||
| ); | ||
| } | ||
| } |
| export const validate = { | ||
| type(value, typeValue, errorMessage) { | ||
| throwErrorWithCondition(typeof value !== typeValue, errorMessage); | ||
| }, | ||
| integer(value, errorMessage) { | ||
| throwErrorWithCondition(!Number.isInteger(value), errorMessage); | ||
| }, | ||
| array(value, errorMessage) { | ||
| throwErrorWithCondition(!Array.isArray(value), errorMessage); | ||
| }, |
There was a problem hiding this comment.
Validators 같은 이름이 더 어울릴 것 같네요!
const createWinningLotto = async () => {
const [winningNumbers, bonusNumber] = await Promise.all([
inputManager.retryScan(
"> 당첨 번호를 입력해 주세요. ",
(inputValue) => inputValue.split(",").map((value) => Number(value.trim()))
),
inputManager.retryScan(
"> 보너스 번호를 입력해 주세요. ",
(inputValue) => Number(inputValue)
)
]);
return new WinningLotto(winningNumbers, bonusNumber);
};
export default createWinningLotto;이런 느낌은 어떨까요?
일단 역할별로 잘 구분된 것 같아요!
|
jgjgill
left a comment
jgjgill
left a comment
There was a problem hiding this comment.
수고 많으셨습니다..! 🙇♂️
이름 짓기
네넵.. 참 어려운 부분인 것 같습니다. 저는 이럴 때 그냥 한글로 합니다. 🧐
개인적으로는 영어로 했을 때는 와닿지 않는 단어를 한글로 하면 가독성을 굉장히 높여준다고 생각하는데 사람마다 호불호가 갈리는 것 같더라구요.. ex) 에러가 발생할 위험이 있다? 불안하다? 낯설다?
당첨 번호와 보너스 번호 입력
넵 이번 미션의 핵심이 '규칙을 얼마나 깔끔하게 구성하는가'로 보여지더라구요.
여기서 보너스 번호가 난이도를 높여주는 역할을 한 것 같습니다 😵
mock을 어떻게 써야할지
오오 제가 생각했을 때는 이번 미션에서는 로또가 해당되지 않을까 싶네요..!
src/controller/createWinningLotto.js
Outdated
| (inputValue) => { | ||
| const bonusNumber = Number(inputValue); | ||
|
|
||
| return attachBonusNumber(bonusNumber); |
src/model/LottoCalculator.js
Outdated
| (counts, { matchedCount, isMatchedBonusNumber }) => { | ||
| const prizeMap = { | ||
| 6: 1, | ||
| 5: isMatchedBonusNumber ? 2 : 3, |
There was a problem hiding this comment.
isMatchedBonusNumber 떄문에 reduce 안에서 prizeMap을 계속 생성하는게 아쉽게 느껴지네요..! 😂
src/model/LottoCalculator.js
Outdated
| static #calcRateOfReturn(winningCounts, price) { | ||
| const sumOfPrize = Object.entries({ ...winningCounts }) | ||
| .map(([ranking, count]) => LottoCalculator.#LOTTO_PRIZES[ranking] * count) | ||
| .reduce((acc, cur) => acc + cur, 0); |
There was a problem hiding this comment.
이미 아실 것 같지만 reduce에서 초기값이 0이면 생략도 가능하더라구요..! (근데 저도 안합니다.. 😇)
.reduce((acc, cur) => acc + cur)
There was a problem hiding this comment.
아 그쵸 ㅎㅎ 저는 명시적으로 추가하는 편입니다..!
|
|
||
| static #createLotto() { | ||
| const lottoNumbers = LottoMachine.#getLottoNumbers(); | ||
| const sortedlottoNumbers = [...lottoNumbers].sort((a, b) => a - b); |
There was a problem hiding this comment.
실제 로또를 봤더니 정렬이 되어있어서 정렬을 해봤지만... 어디서 정렬을 해줄지 확신이 서지 않은 상태입니다 ㅎㅎ
There was a problem hiding this comment.
아 실제 로또는 정렬해주는군요.. 몰랐습니다 😅
그럼 하는게 맞네요 🙃
There was a problem hiding this comment.
이런 함수를 보면 괜히 망가트리고 싶더라구요.. (예외 케이스는 없나..) 🙃
|
AI가 리뷰해줬어요! 코드 분석
강점
개선점
구체적 제안
학습 포인트
요구사항 변경 시 문제될 부분
요구사항 변경 대응새로운 요구사항 예시: "로또 번호의 범위를 1 대응 방안:
이와 같이, 코드의 유연성과 확장성을 고려하여 설계하고, 변경 사항에 쉽게 대응할 수 있도록 작성하는 것이 중요합니다. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
3 participants
미션을 수행하면서 어려웠던 점
이름 짓기
함수명과 클래스명을 짓느라 많은 고민을 했습니다... 역할도 잘 나타내면서 쉬운 단어를 선택하는건 참으로 어렵네요...
당첨 번호와 보너스 번호 입력
당첨 번호와 보너스 번호를 한번에 입력하는게 아닌 두번의 입력을 통해서 값을 받다보니, 유효성 검사 재활용과 에러 처리에 대한 고민을 많이 했습니다.
리뷰 받고 싶은 부분
createWinningLotto 함수
두번의 입력을 통해 값을 받다보니
WinningLotto클래스의 인수를 두번에 걸쳐서 전달해야했습니다. 그래서WinningLotto클래스의 생성자가 이상해졌네요. 커링 비스므리한 형태가 되어버렸습니다. 어떤 방식으로 수정하면 좋을까요?MVC 패턴
MVC 패턴으로 폴더 구조를 구성해봤습니다. 제대로 분리된게 맞을까요?
궁금한 점
readlineAsync를 mock으로 대체하는게 가장 정답에 가까울까요?