Conversation
wzrabbit
left a comment
wzrabbit
left a comment
There was a problem hiding this comment.
@rahwan10 👋🏻
안녕하세요, 동건님! 저는 그리디의 외부 리뷰어를 맡은 김의천이라고 합니다. 만나뵙게 되어 반가워요 😊
Github가 쉽지만은 않죠. 그 답답한 검은 창을 켜고 무언가를 해야 한다는 것도 어렵고, 에러 메시지가 틈만 나면 떠서 저도 진땀 뺀 기억이 많네요
너무 걱정하실 필요는 없고, 이러한 도구들은 사용하시면서 숙달되실 거라 생각합니다.
PR을 보고 느낀 점으로는 동건님은 기능들을 쪼개는 데 집중하신 것 같아요, 숫자야구 게임은 거대한 하나의 기능이니 잘 접근해서 쉽게 풀어내신 것 같아요, 여기에 더해 왜 미션 요구사항에서 기능들을 쪼개고 하나의 함수가 하나의 일만 하라고 했는지 고민해 보시는 것도 좋을 것 같아요!
오늘은 첫 티키타카니까, 저는 여러가지 다 깊게 짚지 않고 기능 위주로 이야기해보고 싶어요. 물론 미션의 요구사항대로 클린 코드도 중요하고, MVC 패턴을 생각하는 것도 중요하지만, 결국 이 모든 것들은 잘 작동하는 프로그램을 더욱 관리하기 좋도록 하기 위한 것이라 생각하거든요.
동건님의 숫자야구 게임을 플레이해보고, 좋았던 점과 개선되면 좋을 것 같은 점들을 코멘트로 적어보았어요. 확인해 보시고 동건님도 의견 공유해주시면 좋을 것 같습니다 💪🏻
There was a problem hiding this comment.
index.html을 확인해 보았는데, app.js를 스크립트로써 실행하고 있네요? 어쩌면 이 모듈은 프로젝트의 어디에서도 쓰이지 않는 것 같아요. 맞다면 지워보시는 것은 어떠세요?
이외에도 프로젝트의 어디에도 쓰이지 않는 파일이 있는지 둘러보시겠어요?
| if (UserNum.length == 0) { | ||
| alert("숫자를 적어주세요"); | ||
| return null; | ||
| } else if (UserNum.length < 3 || UserNum.length > 3) { |
There was a problem hiding this comment.
3보다 작은 경우거나 3보다 큰 경우가 조건이라면, 결국 3이 아니라는 의미와 같네요! 그럼 이 방법은 어때요?
| } else if (UserNum.length < 3 || UserNum.length > 3) { | |
| } else if (UserNum.length !== 3) { |
보너스: !=도 아닌 !==, === 와 같이 등호를 3글자로 적는 건 무엇을 의미할까요?
| function getUserNum() { | ||
| let UserNum = Array.from(document.getElementById("user-input").value); | ||
| if (UserNum.length == 0) { | ||
| alert("숫자를 적어주세요"); | ||
| return null; | ||
| } else if (UserNum.length < 3 || UserNum.length > 3) { | ||
| alert("숫자 3개를 붙여 적어주세요!"); | ||
| return null; | ||
| } | ||
| if (isDup(UserNum)) { | ||
| alert("중복되지않는 숫자를 입력해주세요!"); | ||
| return null; | ||
| } | ||
|
|
||
| return UserNum; |
There was a problem hiding this comment.
우선 이렇게 여러 조건을 들여 꼼꼼하게 올바른지 조건을 따져 주시는 것 정말 좋다고 생각해요 👍🏻 그냥 "입력값이 잘못되었다" 라고 알려주는 것보다 "숫자 3개를 붙여 적어주세요", "중복되지 않는 숫자를 입력해주세요!" 와 같이, 무엇이 잘못되었는지 구체적으로 알려주면, 사용자가 입력값을 다시 검토하는 데 있어서 도움이 될 거에요.
여기에 더해 아마 그대로 통과할 수 있는 입력값들이 있을 것이라 생각해요. 예를 들어, 입력값이 세 글자인 경우인 건 지키는데 1~9가 아닌 다른 값들이 섞여있다든지요. 직접 숫자야구 게임을 더 많이 플레이하시면서, 다양한 입력값들을 넣어 보시면 좋을 것 같아요! 올바른 값을 올바르게 판정하는 것뿐만 아니라 잘못된 값을 잘못되었다고 판정하는 것 또한 중요한 요구사항일 테니까요
There was a problem hiding this comment.
네이밍도 좋고 사용하기도 좋아보이네요 👍🏻
- "이름이 무작위의 세 수를 뽑아라" 로 생략 없이 의미를 잘 드러내고 있어서 무엇을 할지 예측하기 쉬웠어요.
- 지금처럼 하나의 함수가 정해진 일만 수행하면, 사용처가 여러 곳인 상황에서도 범용성 높게 사용할 수 있을 거에요.
이 함수를 모델 삼아, 만약 몇몇 함수가 지나치게 너무 많은 일을 하고 있거나, 이름에 걸맞지 않는 일을 하고 있는지 점검해보셔도 좋을 것 같아요 👁️
| function play() { | ||
| const UserNum = getUserNum(); | ||
| if (!UserNum) return; | ||
| let CountBall = 0; | ||
| let CountStrike=0; | ||
| for (let i = 0; i < 3; i++) { | ||
| for (let j = 0; j < 3; j++) { | ||
| CountBall=CountBall+isBall(ComNum,UserNum,i,j); | ||
| CountStrike=CountStrike+isStrike(ComNum,UserNum,i,j); | ||
| } | ||
| } | ||
| whatIsResult(CountBall,CountStrike); | ||
| } |
There was a problem hiding this comment.
play 함수에는 아래 요구사항이 있으니, 검토해보시면 좋을 것 같아요!
- play메서드는
String으로 결과값을 return 한다.
그 외에 그냥 여담으로 이야기하고 싶은 부분인데, play 함수에서 String을 리턴해봤자 쓰이는 곳이 없는데 왜 리턴해야 하나 궁금하실 수도 있을 것 같아요.
확실히 이번 미션에서는 요구사항에만 적혀 있을 뿐 역할이 따로 달라지지는 않아요. 이 미션, 사실 테스트 코드가 있거든요. npm run test를 입력해서 테스트가 통과하는지 확인하라는 요구사항이 있었잖아요? 거기에 쓰인 코드에요. 테스트 코드란 제대로 기능을 구현했는지 검증을 해 보기 위해 짜는 코드라고 생각하셔도 좋아요.
이번 테스트 코드에서는 프로젝트의 <div id="result"></div>에 적혀있는 값을 긁어와서 검증하고 있는데, 만약 이 방식이 아니라 play 함수가 제대로 작동하는지 확인하는 방식이면 play 함수의 리턴 값이 검사 대상이 될 수 있어요. 이 경우에는 필요해져요.
하나의 시나리오 정도라고 생각하셔도 좋을 것 같아요. 내용이 어렵다면 이 부분은 넘어가셔도 상관 없어요.
| import pickRandomThreeNum from '../models/PickThreeNum.js' | ||
| export default Restart | ||
| function Restart(){ | ||
| let ComNum=pickRandomThreeNum(); |
There was a problem hiding this comment.
현재로써는 수를 맞춘 후 재시작을 하더라도 수가 초기화되지 않는 문제가 있을 거에요. pickRandomThreeNum의 문제는 아닐 거에요. pickRandomThreeNum 함수를 통해 리턴된 값이 실질적으로 Play.js의 ComNum 변수에 영향을 주지 못하고 있을 뿐이에요.
Restart 함수에서 let ComNum 으로 새로 선언하고 return까지 하고 있지만, 이 ComNum은 Restart 함수 안에서만 존재하는 지역 변수에요. 그리고 이 함수는 onclick으로 호출되기 때문에 반환값을 받아주는 곳이 없어서, 새로 뽑은 값은 그대로 사라져요. 실제로 현재 설정되어 있는 수가 저장되어 있는 Play.js의 ComNum은 별도 모듈에 있는 다른 변수라서 여전히 처음 값을 유지하고 있을 거에요.
해결하기 위한 여러 방법이 있을 텐데, 고민해보시면 좋을 것 같아요
| <form> | ||
| <input type="text" id="user-input" /> | ||
| <button id="submit">확인</button> | ||
| <button type="button"id="submit">확인</button> |
There was a problem hiding this comment.
숫자야구 게임의 입력창에서 입력을 하고 확인 버튼을 누르면 정상적으로 게임이 진행되지만, Enter 키를 누르면 그대로 브라우저가 새로고침되는 문제가 현재 있어요.
<form>과 <button>, 특히 <button>의 type에 대해 좀 더 조사해보시면 원인을 유추해보실 수 있을 것 같아요. 고민해보시고 해결이 가능하다면 해결해 보시겠어요?
| if(isAnswer){ | ||
| document.getElementById('game-restart-button').style.display='block'; | ||
| }else{ | ||
| document.getElementById('game-restart-button').style.display='none'; |
There was a problem hiding this comment.
DOM을 요구사항에 맞게 잘 조작해 주신 것 같아요! 덕분에 재시작 버튼이 답을 맞췄을 때에만 보여지네요
여기에 더해 한 가지 의견만 말씀드리고 싶은데, 게임이 끝난 이후 일부러 정답이 아닌 수를 입력했을 때 다시 재시작 버튼이 숨겨지게 되는데, 사실 이미 답을 맞춘 상황이라 다시 버튼이 가려지는 게 개인적으로 어색하다는 생각이 들었어요
답을 맞춘 상황이면 더 이상 수를 입력하지 못하도록 막는 등의 대처 방안도 괜찮아 보이는데, 어떻게 생각하시나요? 그 외에 다른 좋은 방법이 떠오르신다면 생각하신 방법을 반영해보시는 것도 좋을 것 같아요
_2026_03_17_23_50_18_454.mp4
2 participants
안녕하세요. 이번에 그리디 4기 프론트엔드로 들어오게된 김동건입니다!
기념비 미션때 깃허브를 제대로 사용을 못했어서 이번에 처음 여러가지 기능들을 이용해봤습니다.
코드 뿐만 아니라 깃허브 활용등등 여러가지 방면으로 정말 미숙했는데 이번 1주차 미션을 하고 피드백 받으며 열심히 따라가도록 하겠습니다.
기능구현을 이렇게했습니다
5가지로 나눴고 depth길이를 짧게 하고 함수를 최대 작게 한가지 일만 하도록 하라고 돼있어서 함수를 작게 쪼갰습니다
컴퓨터의 값 뽑아오기
1-1.세가지 랜덤값을 받아 저장하는 함수
1-2.저장할 값이 기존의 값과 겹치는지 확인하는 함수
유저값 받아오기
2-1. 유저값을 받아 배열로 저장하는 함수
2-2. 유저값에 겹치는 숫자가 있는지 확인하는 함수
2-3. 각 값을 비교하는 함수
컴퓨터와 유저의 값 비교하기
3-1. 유저와 컴퓨터 값을 비교하는 함수
3-2. 볼이 몇개인지 확인하는 함수
3-3. 스트라이크가 몇개인지 확인하는 함수
4.결과에 맞게 출력하기
4-1. 볼과 스트라이크 갯수에 따라 text만드는 함수
4-2. 만든 텍스트를 결과창에 출력하는 함수
이후 mvc패턴으로 정리하라하셔서 제가 이해한대로 정리를 해봤습니다!
controllers-> 컴퓨터와 유저값 비교함수, 재시작함수
models-> 컴퓨터값 뽑는 함수,유저값 받는 함수
views->결과를 출력하는 함수
코드부터 설명, 제출방식 등등 다 부족하지만 모르는것들은 또 묻고 피드백 받으며 열심히 배우겠습니다.
미션숙지안된게 있다면 정말 죄송합니다.
(커밋에 테스트라 적혀있는것들은 제가 커밋이 뭔지 잘몰라서 잘 작동하는지 계속 확인해봤던 흔적입니다..)