-
Notifications
You must be signed in to change notification settings - Fork 10
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
[이재빈] up and down 구현 #10
base: main
Are you sure you want to change the base?
Conversation
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.
👍👍👍 고생하셨습니다!
src/index.js
Outdated
const gameState = { | ||
randomNumber: Math.floor(Math.random() * 50) + 1, | ||
attempts: 0, | ||
userInput: [], | ||
}; | ||
|
||
const resetGameState = () => { | ||
gameState.randomNumber = Math.floor(Math.random() * 50) + 1; | ||
gameState.attempts = 0; | ||
gameState.userInput = []; | ||
}; |
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.
게임에 대한 상태를 만들고 상태로써 관리하는 부분 좋네요 👍👍
다만 randomNumber
를 생성하는 부분을 별도의 함수로 분리해두고 사용한다면 보다 가독성이 좋을것 같아요
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.
다만
randomNumber
를 생성하는 부분을 별도의 함수로 분리해두고 사용한다면 보다 가독성이 좋을것 같아요
randomNumber 생성을 별도의 함수로 분리하는 게 맞는 거 같습니다.
확장성을 생각했을 때 숫자의 범위를 조절하고 싶다면 직접 코드를 고치는 것보다는 randomNumber를 호출하는 부분에서 제어하면 좋을 거 같네요 감사합니다!
const inputNumber = Number(inputValue); | ||
const isNotNumber = Number.isNaN(inputNumber); |
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.
문자열이 들어오더라도 잘 방어될 수 있도록 꼼꼼하게 작성되었네요 👍
src/index.js
Outdated
if (isNotNumber || isNotInteger || isOutOfRange) { | ||
console.log("1에서 50사이의 숫자 값만 입력해주세요"); | ||
} | ||
|
||
return inputNumber; |
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.
근데 validation이 통과하지 않을 때 inputNumber
를 그대로 반환하기보다는 undefined나 null을 반환하고, 그 상태를 상위 로직에서 비교하는 식으로 처리하는게 좀 더 좋을것 같긴 하네요
아니면 에러를 throw하고 메인 로직을 그에 맞게 수정하는것도 괜찮아 보이긴 하구요
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.
아니면 에러를 throw하고 메인 로직을 그에 맞게 수정하는것도 괜찮아 보이긴 하구요
에러를 throw하는 게 가독성 면에서나 코드 안정성 면에서 좋을 거 같네요! 고쳐보겠습니다
src/index.js
Outdated
const handleGameResult = (isCorrect, randomNumber, attempts) => { | ||
if (isCorrect) { | ||
console.log(`정답! ${attempts}번 만에 숫자를 맞추셨습니다.`); | ||
return true; | ||
} | ||
|
||
if (attempts === 5) { | ||
console.log(`5회 초과! 숫자를 맞추지 못했습니다. (정답: ${randomNumber})`); | ||
return true; | ||
} | ||
|
||
return false; | ||
}; |
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.
이렇게 인자를 2개이상 받는 함수들에 대해서는 인자를 객체 하나로 묶어서 관리하면 좀 더 좋을 것 같아요
그렇지 않으면 매번 handleGameResult
함수를 사용할 때 마다 isCorrect, randomNumber, attempts 인자들의 순서를 기억하고 있어야 하는 불편함이 있기 때문이에요
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.
이렇게 인자를 2개이상 받는 함수들에 대해서는 인자를 객체 하나로 묶어서 관리하면 좀 더 좋을 것 같아요
이 부분은 생각해보지 못했었네요. 항상 함수를 사용할 때 매개변수를 줄줄이 나열하곤 했었는데 이제는 객체로 묶어 보내는 걸 고려해보겠습니다 감사합니다!
src/index.js
Outdated
while (true) { | ||
const inputValue = await readLineAsync("숫자 입력: "); | ||
const validNumber = validateInputValue(inputValue); | ||
const isCorrect = validNumber === gameState.randomNumber; | ||
|
||
if (!validNumber) continue; | ||
|
||
gameState.attempts++; | ||
gameState.userInput.push(validNumber); | ||
|
||
const gameFinished = handleGameResult(isCorrect, gameState.randomNumber, gameState.attempts); | ||
|
||
if (gameFinished) break; | ||
|
||
displayHint(validNumber, gameState.randomNumber, gameState.userInput); | ||
} |
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 구문을 활용해서 attempts 상태를 계속해서 업데이트 하고 handleGameResult
에서 최종 결과를 처리하는 논리의 흐름이 보기 좋네요 👍👍👍
src/index.js
Outdated
if (attempts === 5) { | ||
console.log(`5회 초과! 숫자를 맞추지 못했습니다. (정답: ${randomNumber})`); | ||
return true; | ||
} |
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.
5회 초과에 대한 로직에 대해서는 attempts === 5
의 비교보다는 attempts >= 5
의 비교가 좀 더 명확한 것 같습니다
요구 사항
멘토님께