Skip to content
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

[2단계 - 자동차 경주 구현] 곤이(김기융) 미션 제출합니다. #40

Merged
merged 10 commits into from Feb 18, 2021

Conversation

yungo1846
Copy link

안녕하세요 :)
항상 바쁘신 와중에도 신경써서 리뷰를 해주시는 리뷰어분들께 감사드립니다.

🎮URL

🎯🎯 step2

  • 자동차 경주 게임의 턴이 진행 될 때마다 1초의 텀(progressive 재생)을 두고 진행한다.
    • 애니메이션 구현을 위해 setInterval, setTimeout, requestAnimationFrame 을 활용한다.
  • 정상적으로 게임의 턴이 다 동작된 후에는 결과를 보여주고, 2초 후에 축하의 alert 메세지를 띄운다.
  • 위 기능들이 정상적으로 동작하는지 Cypress를 이용해 테스트한다.

step2부터는 위와 같은 요구사항이 추가되었습니다.
이번 미션을 통해서 이전까지 어렴풋이만 알고 있던 자바스크립트의 동기화에 대해 조금 더 이해할 수 있었습니다.
그 덕에 promise, async, await 사용에 조금 더 익숙해진 것 같습니다. 그리고 동기화를 할 때, 콜백지옥을 조심하라는 얘기를 들었습니다. 만약 제가 작성한 코드 스타일대로 더 큰 규모의 동기화 작업을 진행한다고 한다면 어떨지 궁금합니다.
또한 실무에서 자주 쓰이는 동기화 방식(?)이 있는지 알고 싶습니다!

감사합니다 😊

Copy link

@wow9144 wow9144 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sleep 을 따로 만들어서 다른 코드와 섞지 않고 util 로 잘 빼신것 같습니다.
다만 문제의도가 sleep을 의도한건 아닐 것 같아서 관련 부분에 코멘트 드렸으니 한 번 고민 해보시면 좋을 것 같습니다!

@@ -0,0 +1,7 @@
export function sleep(ms) {
return new Promise((resolve, _) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

식별자명을 _ (언더바) 로 하는게 private이라는 의미로 쓸 수도 있고 사용 하신것 처럼 파라미터의 경우는 사용되지 않는 변수라는 의미로 쓸 수도 있고 언어차원에서 문법적으로 지원하는 언어도 있고 그저 관례적으로 쓰는 경우도 있고 다양합니다.
지금과 같은 경우는 그냥 사용되지 않으니 아예 없애는게 더 좋을 것 같습니다.
코드를 굳이 극단적으로 줄이면
const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));
이렇게도 할 수 있으니 문법적으로 혹시 모르셨다면 참고만 하시면 되겠습니다.
이 코멘트는 그냥 참고만 하시면 됩니다!

for (let i = 0; i < this.model.racingCount; i++) {
await sleep(1000);
const movedCars = this.getMovedCars();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아마 문제의 의도는 그저 코드 실행을 지연 시키라는게 아니라 결과를 비동기적으로 얻어서 사용하는 경험을 의도하지 않았을까 싶습니다.
이를테면 자동차가 앞으로 나가는데, 앞으로 나간 그 결과가 1초뒤에 온다는걸 가정하지 않았을까 싶습니다.
한 턴의 요청과 결과 사이에 로딩바가 도는걸테고요.
따라서 sleep 는 제거하고 this.getMoveCars() 의 반환이 프로미스여야 할 것 같습니다.

서버로부터 비동기적으로 데이터를 받아 화면에 그려내는 작업이 프론트 개발의 주를 이루기에 제가 만약 이런 문제를 냈다면 그런 의도였을것 같습니다.

문제 의도는 제 추측일뿐이니 혹시 확인 가능하시면 확인 해보고 하셔도 좋고 확인 필요없이 수정을 시도 해보셔도 좋을 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문제의 의도를 곰곰이 생각해보니 말씀하신대로 getMovedCars가 프로미스를 반환하는 것이 맞다고 생각합니다!
따라서 startRacing에서 sleep을 사용하는 대신 getMovedCars에 새로운 프로미스를 반환하도록 하여 시간을 지연시키도록 코드를 다시 작성하였습니다. 서버로부터 비동기 데이터를 처리하기 위한 연습일 것 같다고 예시를 들어주셔서 잘 이해할 수 있었습니다. 감사합니다 :)

Copy link

@wow9144 wow9144 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고 많으셨습니다.
Promise를 이용해서 비동기적 데이터 사용을 잘 학습 하셨으리라 생각합니다.

@wow9144 wow9144 merged commit 0717c86 into woowacourse:yungo1846 Feb 18, 2021
@yungo1846
Copy link
Author

yungo1846 commented Mar 9, 2021

✅ 3주 만에 하는 셀프 리뷰

잘한 점

  1. 상수 분리
  2. MVC 패턴 적용을 위한 고민
  3. DOM 노드를 필요할 때마다 호출 하는 것이 아닌 변수에 저장해놓은 것
  4. 컨트롤러에서 모델과 뷰를 생성하는 것이 아닌 외부에서 주입 받아 의존성을 낮춘 것

아쉬운 점

  1. CarRacingView에서 hide, show, disableElement, enableElement 처럼 자주 쓰일만한 요소는 util에 따로 분리해도 좋을 듯
  2. CarRacingModel에서 cars 배열 정보를 가져올 메소드를 추가하면 좋을 듯. 외부에서 배열을 변경할 수 없도록 private 프로퍼티를 사용하거나 getCars를 만들어 사본배열을 반환하면 어떨까?
  3. constants를 객체로 반환할거면 Object.freeze를 하여 외부에서 객체 내부를 변경할 수 없도록 해야한다.
  4. re export를 사용하여 파일을 좀 더 쉽게 import 할 수 있게 하면 좋을 듯.
  5. CarRacingController에서 다른 model이나 view가 추가될 수 있으니 이름을 단순히 model과 view가 아니라 CarModel, CarView로 하는 것이 좋을 듯.
  6. 이벤트 핸들링 함수 네이밍에 굳이 특정 이벤트명을 집어넣을 필요는 없다. onClick... (x)
  7. 숫자만큼의 반복문을 돌아야할 때는, 숫자만큼의 배열을 만들어 돌리는 편이 좋다. 실수를 방지하기 위함.
  8. document.querySelector처럼 길게 DOM api를 사용하는 것보다 jQuery 스타일을 사용하는 것이 코드의 가시성이 올라간다

배운 점

당시에는 굉장히 고민하고 노력한 코드지만 3주 뒤에 보니 부족한 점이 많았다. 그만큼 그 때 보다 많은 성장을 했다고 생각한다. 앞으로 남은 기간동안 얼마나 더 성장할지 기대가 된다. 그리고 가끔 자존감이 떨어질 때, 이전에 내가 짠 코드를 보면서 셀프 자신감을 충전하는 것도 괜찮은 방법인 것 같다.

아직도 안 풀린 궁금증은 무엇인가?

MVC 패턴을 두번정도 사용하면서 모델, 뷰 컨트롤러의 역할은 어느정도 숙지했다. 그러나 프로젝트의 규모가 커지면 필연적으로 더많은 모델과 컨트롤러, 뷰가 필요해진다. 아직까지는 컨트롤러를 하나만 생성봤기 때문에 여러개의 컨트롤러를 만들고 그 컨트롤러들을 관리하는 로직을 어떻게 짤 것인지에 대한 고민이 필요하다.

앞으로 어떻게 다르게 할 것인가?

앞의 미션들은 상대적으로 난이도가 낮아 다른 크루들의 코드와 리뷰를 참고할 시간이 있었다. 하지만 미션 난이도가 점점 올라가면서 미션 외의 시간을 내는게 조금씩 부담스러워지고 있다. 그럼에도 다른 크루의 코드와 리뷰를 보는 것이 매우 큰 도움이 되기에 일상에서 생기는 잉여시간을 줄여서 성장하는 방향에 시간을 투자해야겠다.

@yungo1846
Copy link
Author

레벨 1-1. 자동차 게임 학습로그

📌 학습 로그

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants