[숫자 야구 게임] 이만재 미션 제출합니다. #2
Conversation
|
안녕하세요 만재님 ~ 리뷰를 하게 된 오혜성 입니다! 다음부터는 PR 올리실 때 우측 상단 리뷰어에 저를 넣어주실 수 있으실까용??
고생하셨습니다 👍 퇴근 이후에 살펴볼게요! |
|
|
||
| Console.print("숫자 야구 게임을 시작합니다."); |
There was a problem hiding this comment.
제출하고 리팩토링하는 과정에서 게임시작을 담당하는 함수에서 출력하는게 맞다고 생각해 수정했습니다!
There was a problem hiding this comment.
아 제가 말씀드린건 저 부분에서 출력하는게 적절하지 않은 것 같아서 리팩토링하면서 게임시작을 담당하는 함수 안에 출력문을 넣었다는 말을 드리고싶었던 것이었습니다!!
| COMPUTER.push(number); | ||
| } | ||
| } | ||
| console.log(COMPUTER); |
There was a problem hiding this comment.
기능구현을 확인하기 위해 미리 답을 보면서 확인했는데 제출할 때 지우는 걸 깜빡했습니다...
| if (inputNumber[i] === COMPUTER[i]) { | ||
| strike++; | ||
| } else { | ||
| ball++; | ||
| } |
There was a problem hiding this comment.
후위연산자의 경우 어떤 동작을 취하고 나서 값을 증가시키지만 전위연산자의 경우 동작을 취하기 전 값을 증가시키고 동작을 취한다고 이해하고 있습니다!
| if (strike === 3) { | ||
| Console.print("3스트라이크!"); | ||
| Console.print("3개의 숫자를 모두 맞히셨습니다! 게임 종료"); | ||
|
|
||
| let answer = await Console.readLineAsync( | ||
| "게임을 새로 시작하려면 1, 종료하려면 2를 입력하세요: " | ||
| ); | ||
| if (answer === "1") { | ||
| COMPUTER = generateRandomNumber(); | ||
| continue; | ||
| } else if (answer === "2") { | ||
| break; | ||
| } else { | ||
| throw new Error("[ERROR]"); | ||
| // break; | ||
| } | ||
| } else if (ball === 0 && strike === 0) { | ||
| Console.print("낫싱"); | ||
| } else { | ||
| if (ball > 0 && strike === 0) { | ||
| Console.print(`${ball}볼 `); | ||
| } else if (strike > 0 && ball === 0) { | ||
| Console.print(`${strike}스트라이크`); | ||
| } else { | ||
| Console.print(`${ball}볼 ${strike}스트라이크`); | ||
| } | ||
| } |
There was a problem hiding this comment.
반환 혹은 break, continue 등의 키워드를 적절히 사용해
파악하기 쉬운 조건문을 만드려면 어떻게 바꿀 수 있을까요?
There was a problem hiding this comment.
이부분은 한 함수가 게임을 재시작하는 부분과 사용자 입력에 대한 결과를 메시지로 출력하는 부분을 같이하고 있다고 생각하여
함수를 분리했습니다. 함수를 분리 후에도 if문과 else if문이 많다고 생각해 고치고 싶었지만 좋은 방법이 생각나지 않아 수정은 하지 못했습니다...
| inputNumber.push(parseInt(USER_INPUT / 100)); | ||
| USER_INPUT = USER_INPUT % 100; | ||
| inputNumber.push(parseInt(USER_INPUT / 10)); | ||
| USER_INPUT = USER_INPUT % 10; | ||
| inputNumber.push(parseInt(USER_INPUT)); |
There was a problem hiding this comment.
배열에 값을 삽입할 때 사용자가 입력한 순서대로 배열에 들어가기 때문에 저렇게 나눠서 할 필요가 없다는 것을 깨닫고 수정하였습니다!
|
고생하셨습니다 👍 게임 종료가 프로세스의 종료를 의미할까요? 그렇다면 프로세스의 종료는 코드상 무엇을 의미할까요? 요것도 추가적으로 고민해 보시면 좋을 거 같아요 ~ 파트너분의 PR에도 비슷한 코멘트들이 있어 서로 확인해보셔도 좋을 듯 합니다 ~ 재확인이 필요하실 때 리뷰 재요청 주세요~ |
|
궁금한 점
죄송한점
|
1. 사용자가 입력한 숫자를 체크하는 함수에 await를 붙여 사용자 입력이끝나고 체크하도록 수정 2. 게임 재시작 함수 역시 await를 붙여 게임이 종료된 다음 실행되도록 수정
|
궁금한 점
|
사용자 입력에 따라 게임 결과를 알려주는 부분에 중첩if문을 사용했는데 코드의 가독성을 떨어트리는 것 같아 Early return 패턴을 적용해 봤습니다!
|
혜성님이 다른 멤버의 코드 리뷰에 달아주신 early return 패턴에 관해 찾아보고 이전코드에서는 사용자 입력에서 게임결과 출력부분을 중첩 if문으로 구현했었는데 early return을 적용시켜 보았습니다! 확실히 코드 가독성이 높아졌다는게 눈에 보였습니다. 다만 early return패턴은 여러개의 조건을 처리해야하는 복잡한 함수의 경우 한 함수 내부에서 return문을 많이 사용하게 되면 오히려 코드 가독성을 떨어트린다는 글을 봤습니다. 그 글에서는 간결한 코드의 경우 if-else문이 코드가독성이 더 나을 수도 있다는 얘기가 나와 궁금증이 생겼습니다.
이런 경우에 개발자가 판단 하에 early return 패턴을 적용하거나 if-else문을 적절히 사용하는게 맞는지 궁금합니다. |
hyesungoh
left a comment
There was a problem hiding this comment.
컨플릭 나는 거 확인 부탁드려요 ~
작성해주신 내용들은 내일 추가적으로 확인해 볼게요! 👍 👍
|
충돌난 이유를 몰라서 해매다가 어떻게 충돌 해결은 했는데 마지막 refactor: early return이란 메시지 이름으로 커밋한 부분은 충돌을 해결한 부분이니 참고 부탁드릴게요... 죄송합니다 ㅠㅠ |
획일적으로 '그런 방법이 맞다' 같은 은 총알은 제가 개발에 대해 아는 선에서는 본 적 없어요 하나의 큰 문제(요구사항)에 다가가기 위해 만나는 많은 문제들에 그에 맞는 거 같다고 판단하는 방법들 중에서 트레이드오프를 고려해 최선의 방법으로 판단하고 작성하고 배우는 것이 개발이라고 생각하기 때문인데용 해당 문제의 경우에는 적절한 방법으로 해결하신 거 같아요. 저같아도 그렇게 했을 거 같습니다~ 👍
주체가 살짝 다른 거 같은데요 ~ 입력 받은 값에 대해 각각의 말씀하신 임시 변수 inputNumber에 형변환을 해서 넣는 행위로 이해하시면 될 거 같아요 ~
아직 코드를 본 상태는 아니라 볼 때 확인해 볼게요~
요것도 같이 확인해볼게요~
요것도 같이 확인해볼게요~ 추가적으로 하나의 함수가 많은 역할을 하면 안되는 이유, 그러면 발생하는 문제점에 대해서도 심도 깊은 고민을 해보시면 좋을 거 같아요~
요것도 같이 확인해볼게요~
죄송할 필요 없습니다 ~ 재밌으라고 개발하는건데요 |
네 ~
네 ~ 추가적으로 위 코멘트와 현재 요 말씀 같은 경우에는 리뷰어의 편의를 위해 셀프 리뷰하는 방법이 있어요 현업에서도 굉장히 많이 사용하고, 한다면 이쁨받을 수 있는 방법이니 나중에라도 해보시면 좋을거여요~
말씀하신 부분이 이해가 잘안되는데, 코드를 보고 의견을 남겨볼게요~ |
위의 코멘트와 같은 맥락일 거 같은데요. 어떤 패턴 혹은 방법이 항상 정답인 경우는 없어요 매순간 개발자의 판단으로 이루어져야하고 그것이 옳은 판단이였는지는 작성한 코드, 소프트웨어가 지속적으로 숨쉬고 성장할 때, 즉 비즈니스 요구사항에 쉽게 대응할 수 있는 지에서 알 수 있어요
그런 연유에서 말씀하신 것이 맞다고 할 수 있습니다 ~~ 이프 엘스가 더 좋다고 보인다면 그렇게 하는 것, |
hyesungoh
left a comment
There was a problem hiding this comment.
코드가 많이 좋아진 게 느껴지는 거 같아요 👍 고생하셨어요 👍
조금만 더 가다듬으시면 좋을 거 같아서 한 번 더 리퀘체 드립니다~
| @@ -0,0 +1 @@ | |||
| export const NUM_DIGITS = 3; | |||
There was a problem hiding this comment.
해당 상수의 이름이 적절할지 모르겠네요 🤔
상수의 경우 전부 대문자로 작성하시는 것 좋습니다 👍
| const number = Random.pickNumberInRange(1, 9); | ||
| if (!computerNumber.includes(number)) { | ||
| computerNumber.push(number); | ||
| const generateRandomNumber = () => { |
There was a problem hiding this comment.
generate~ 보다 get~ 이란 이름은 어떻게 생각하세요?
두 단어의 차이가 있을까요?
| const RANDOM_NUMBER = []; | ||
| while (RANDOM_NUMBER.length < NUM_DIGITS) { | ||
| const GENERATE_NUMBER = Random.pickNumberInRange(1, 9); | ||
| if (!RANDOM_NUMBER.includes(GENERATE_NUMBER)) { | ||
| RANDOM_NUMBER.push(GENERATE_NUMBER); | ||
| } |
There was a problem hiding this comment.
RANDOM_NUMBER 배열은 상수라기 보다는 변수에 가까울 거 같아요
전부 대문자로 작성하신 이유가 있으신가요??
| console.log(computerNumber); | ||
| return computerNumber; | ||
| } | ||
| return RANDOM_NUMBER; |
There was a problem hiding this comment.
반환하는 값은 배열인데, 함수의 시그니처에는 노출되고 있지 않은 거 같아요.
| let inputNumber = []; | ||
| let USER_INPUT = await Console.readLineAsync("숫자를 입력해주세요: "); | ||
|
|
||
| if (isNaN(USER_INPUT) || USER_INPUT.length != FIT_LENGTH || USER_INPUT < 0) { | ||
| throw new Error("[ERROR] 잘못 입력하셨습니다."); | ||
| } | ||
| let user_input = await Console.readLineAsync("숫자를 입력해주세요: "); |
There was a problem hiding this comment.
위 배열은 const를 사용했고, 여기는 let을 사용하신 이유가 궁금해요 ~
배열의 경우 왜 const를 써도 배열 안의 값은 변경할 수 있는지 이유에 대해 알아보셔도 좋을 거 같아요
| if (!CHECK_USER_INPUT.test(user_input)) { | ||
| throw new Error("[ERROR] 잘못된 입력입니다."); | ||
| } else { |
There was a problem hiding this comment.
throw 문을 탄다면 해당 스코프 아래의 코드는 실행되지 않아요
그러니 else 문이 필요할까요 ??
| if (!CHECK_USER_INPUT.test(user_input)) { | ||
| throw new Error("[ERROR] 잘못된 입력입니다."); | ||
| } else { | ||
| for (const INPUT_NUM of user_input) { | ||
| inputNumber.push(Number(INPUT_NUM)); | ||
| } | ||
| if (user_input.length !== NUM_DIGITS) { | ||
| throw new Error("[ERROR] 3자리 수를 입력해주세요."); | ||
| } | ||
| if (new Set(inputNumber).size !== NUM_DIGITS) { | ||
| throw new Error("[ERROR] 중복된 입력입니다."); | ||
| } |
There was a problem hiding this comment.
예외의 경우 한 번에 처리시키고, 성공하는 경우만 읽는 사람이 후루룩 읽을 수 있게 하려면 어떻게 할 수 있을까요??
| let computerNumber = generateRandomNumber(); | ||
| }; | ||
| const playGame = async () => { | ||
| Console.print("숫자 야구 게임을 시작합니다."); |
| const checkGameScore = (strike, ball) => { | ||
| if (ball === 0 && strike === 0) { | ||
| Console.print("낫싱"); | ||
| return; | ||
| } | ||
|
|
||
| if (ball > 0 && strike === 0) { | ||
| Console.print(`${ball}볼`); | ||
| return; | ||
| } | ||
|
|
||
| if (strike > 0 && ball === 0) { | ||
| Console.print(`${strike}스트라이크`); | ||
| return; | ||
| } | ||
|
|
||
| if (ball > 0 && strike > 0) { | ||
| Console.print(`${ball}볼 ${strike}스트라이크`); | ||
| return; | ||
| } | ||
| }; |
There was a problem hiding this comment.
테스트하기 쉬운 코드(어려운 개념이지만)가 되기 위해서는 여기서 로그하는 것보단 반환을 하면 어떨까요?
그렇게하면 어떤 점이 다르고, 함수의 이름은 어떻게 변경되어야하며
왜 테스트하기 쉬운 코드가 되는 것일까용??
| if (ANSWER === "1") { | ||
| await playGame(); | ||
| } else if (ANSWER === "2") { | ||
| return; | ||
| } else { | ||
| throw new Error("[ERROR]"); | ||
| } |
There was a problem hiding this comment.
| if (ANSWER === "1") { | |
| await playGame(); | |
| } else if (ANSWER === "2") { | |
| return; | |
| } else { | |
| throw new Error("[ERROR]"); | |
| } | |
| if (ANSWER === "1") await playGame(); | |
| if (ANSWER === "2") return; | |
| throw new Error("[ERROR]"); |
이렇게 작성하는 것과 어떤 차이가 있을까요?
어려웠던 점
아직 코드에서 요구하는 조건을 만족시키면서 코드를 짜는게 익숙하지 않기도 하고 자바스크립트 언어에 대해 부족함도 많이 느꼈습니다.
궁금한점
테스트 코드에서 게임 종료 후 재시작을 하는 부분에서 failed가 나와 게임 종료가 잘 처리되지 않은 것 같아요..
process.exit() 사용하지 않고 종료가 되게 하려면 어떻게 해야하는지 궁금합니다!