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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 10 commits into from
Feb 18, 2021
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
33cefec
fix: 각 라운드 당 1초의 딜레이를 적용하도록 테스트 케이스 수정
yungo1846 Feb 14, 2021
0ec45c5
feat: utils.js 파일 내 sleep 함수 생성
yungo1846 Feb 14, 2021
7c6d40e
fix: startRacing 메소드를 async 메소드로 변경 및 endRacing 메소드 명을 showRacingResu…
yungo1846 Feb 14, 2021
2e5ee3f
feat: 결과 발표 2초 후 축하의 alert 메세지 띄우기 기능 추가
yungo1846 Feb 14, 2021
d85ed8d
docs: cypress integration examples 삭제
yungo1846 Feb 14, 2021
d07d60e
feat: view에 removeSpinner 메소드 추가
yungo1846 Feb 14, 2021
d40330d
fix: 우승자 축하 메세지를 alertConstants.js 객체에 포함
yungo1846 Feb 14, 2021
c719b10
feat: 우승자 축하 alert를 체크하는 테스트 케이스 추가
yungo1846 Feb 14, 2021
69cc7eb
docs: README에 데모사이트 URL 추가
yungo1846 Feb 14, 2021
70163ef
fix: getMovedCars 메소드가 프로미스를 반환하도록 수정
yungo1846 Feb 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/js/controllers/CarRacingController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import alertConstants from '../constants/alertConstants.js';
import racingConstants from '../constants/racingConstants.js';
import { sleep } from '../utils/utils.js';

export default class CarRacingController {
constructor(model, view) {
Expand Down Expand Up @@ -90,8 +91,9 @@ export default class CarRacingController {
return movedCars;
}

startRacing() {
async startRacing() {
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에 새로운 프로미스를 반환하도록 하여 시간을 지연시키도록 코드를 다시 작성하였습니다. 서버로부터 비동기 데이터를 처리하기 위한 연습일 것 같다고 예시를 들어주셔서 잘 이해할 수 있었습니다. 감사합니다 :)

this.moveCars(movedCars);
this.view.renderRacingRoundResult(movedCars);
Expand Down
7 changes: 7 additions & 0 deletions src/js/utils/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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));
이렇게도 할 수 있으니 문법적으로 혹시 모르셨다면 참고만 하시면 되겠습니다.
이 코멘트는 그냥 참고만 하시면 됩니다!

setTimeout(() => {
resolve();
}, ms);
});
}