Conversation
tkdrb12
left a comment
tkdrb12
left a comment
There was a problem hiding this comment.
안녕하세요 가영님~
여행은 잘 다녀오셨나요?
이번 2단계 미션 코드를 보며 확실히 자바스크립트 실력이 많이 늘고있다는 게 느껴집니다!😀
짧은 기간 내에 코드를 작성하는 게 쉽지않았을텐데 작성을 잘 해주셨네요
코멘트 읽어보시고 피드백 사항 반영해주세요!
꼭 제 말이 무조건 옳은건 아니니 같이 이야기 나눠보며 수정해보면 좋을 것 같습니다~
| display: flex; | ||
| gap: 10px; |
There was a problem hiding this comment.
중앙 배치가 되어있지 않은데 align-items과 justify 속성에 대해서 알아볼까요?
There was a problem hiding this comment.
align-items: center; 를 추가하면 가로축을 기준으로 카드들이 정렬하게 되고,
justify-content: center; 를 추가하면 세로축을 기준으로 가운데 정렬하게 됩니다!
코드에 삽입하겠습니다.
src/Card.js
Outdated
| this.gameOver = false; | ||
| this.information = document.createElement("p"); | ||
| this.information.classList.add("information"); | ||
| document.body.appendChild(this.information); |
There was a problem hiding this comment.
document.body에 삽입시키는 것보다 특정 html요소(cards 등) 에 삽입하는게 구조적으로 좋답니다!
src/Card.js
Outdated
| button.addEventListener("click", () => { | ||
| if (this.gameOver) return; | ||
|
|
||
| if (randomNum === index) { | ||
| button.textContent = "당첨"; | ||
| this.endGame("(성공!) 당첨되었습니다."); | ||
| } else { | ||
| button.textContent = "꽝"; | ||
| this.chance = this.chance - 1; | ||
|
|
||
| if (this.chance === 0) { | ||
| this.endGame("(실패!) 게임이 종료되었습니다."); | ||
| } else { | ||
| this.startGame(button, cardsContainer, randomNum, index); |
There was a problem hiding this comment.
if else문이 중첩되다보면 가독성이 떨어집니다.
해당 메소드를 나누어 메소드의 기능을 분리시키는 게 좋을 것 같아요!
| restart.textContent = "재시작"; | ||
| restart.classList.add("restart"); | ||
| restart.addEventListener("click", () => { | ||
| location.reload(true); |
There was a problem hiding this comment.
버튼을 클릭했을 때 새로고침을 함으로써 재시작이 되도록 기능을 작성해주셨네요!
좋은 아이디어입니다. 하지만 제가 의도드린 사항은 start메소드를 다시 재사용해보는 것이었습니다. 시간이 된다면 다시 작성해보시는걸 추천드려요!
There was a problem hiding this comment.
start 메소드를 end에서 재활용을 해보려 했는데 재시작과 변수 변경 등등이 오류가 나서.. 좀만 시간을 두고 다시 해보겠습니다 ㅠㅠ
src/index.js
Outdated
| buttons.forEach((button, index) => { | ||
| gameManager.startGame(button, cardsContainer, randomNum, index); |
There was a problem hiding this comment.
카드 버튼에 이벤트를 주입하는 코드인데요 해당 기능은 gameManager 클래스가 소유하고 있는게 적절하다고 생각됩니다.
gameManager 클래스에서 buttons를 소유하고 해당 코드를 실행시키도록 해보는건 어떨까요?
| this.gameOver = true; | ||
| this.information.textContent = message; | ||
|
|
||
| let restart = document.createElement("button"); |
There was a problem hiding this comment.
button과 div element의 차이점은 무엇일까요?
There was a problem hiding this comment.
button은 사용자가 클릭하면 동작하는 상호작용이 가능한 엘리먼트이고
div는 구간을 나누는 엘리먼트로써 어떠한 의미를 가지진 않지만 복합적인 컨테이너로 사용됩니다.
src/index.js
Outdated
| const card1 = new Card(cardsContainer); // cardsContainer를 생성자에 전달 | ||
| const card2 = new Card(cardsContainer); // cardsContainer를 생성자에 전달 | ||
| const card3 = new Card(cardsContainer); // cardsContainer를 생성자에 전달 | ||
| const card2 = new Card(cardsContainer); | ||
| const card3 = new Card(cardsContainer); | ||
| const card4 = new Card(cardsContainer); | ||
|
|
||
| const buttons = document.querySelectorAll("button"); | ||
|
|
||
| const randomNum = Math.floor(Math.random() * 3); | ||
| const randomNum = Math.floor(Math.random() * 4); |
There was a problem hiding this comment.
카드 갯수를 상수화하고 반복문을 사용해서 코드를 개선해볼까요?
2 participants
안녕하세요.
한가지 이상의 테스트를 추가하는 것 외에는 모두 완료하였습니다. 좀 더 공부해서 테스트 추가하겠습니다.
감사합니다.