[1단계 - 자동차 경주 구현] 가브리엘(윤주현) 미션 제출합니다. #153
Conversation
- @woowacourse/mission-utils 설치
Co-authored-by: Gabriel Ju Hyun, Yoon <gabrielyoon7@gmail.com>
Co-authored-by: Gabriel Ju Hyun, Yoon <gabrielyoon7@gmail.com>
Co-authored-by: Gabriel Ju Hyun, Yoon <gabrielyoon7@gmail.com>
- 원래 콜백패턴에서 사용해 의존성이 입력 로직에 얽혀있었는데, play 메서드로 이동했다.
There was a problem hiding this comment.
안녕하세요 가브리엘!
이틀만에 미션 구현하시느라 고생 많으셨습니다 ㅎㅎ 전체적으로 함수 분리도 잘 되어있고 도메인 분리 등 1단계 요구사항에 대해 잘 구현하신 것 같네용.👍👍 수고하셨습니다! 리뷰 확인해보시고 궁금한 점이 있다면 편하게 코멘트 남겨주세요~!
+) package-lock.json 과 yarn.lock 이 함께 있는 걸로 보아 yarn, npm 둘 다 사용하신 것으로 보이는데 패키지 매니저 간 관리 방식에 차이가 있어서 혼용은 피해주시는 것이 좋습니다.
__tests__/GameManager.test.js
Outdated
| [4, true], | ||
| [3, false], | ||
| [0, false], | ||
| ])('isFoward Test Random value : %i', (number, expected) => { |
There was a problem hiding this comment.
다른 테스트와 다르게 이 파일 테스트 description은 영어로 작성되어있네용. 별개로 테스트에 대한 항목은 테스트가 무엇을 테스트 하는지를 정확히 적어주시는게 좋습니다~ 예를 들면 "isForward returns true if random valuse is over 4" 또는 "랜덤 값이 4 이상이면 isForward는 true를 반환한다." 와 같이 표현해 볼 수 있을 것 같습니다.
이 코드를 읽는 사람이 동일한 요구사항, 동일한 도메인에 대해서 이해를 하고 있을 수도 있지만, isForward가 무슨 조건에 의해서 동작하는지 잘 모르는 사람이 테스트 코드를 확인하게 된다면, 현재 설명문으로는 isForward 가 어떤 동작을 정상적으로 수행하는지는 알기 어려울 것 같아요.
There was a problem hiding this comment.
다음부터는 테스트 명 작성에 주의하도록 하겠습니다!
- 자동차가 아닌, 게임 매니저가 판단하도록 개선
- tryMoveCars => raceCars
- description을 이해할 수 있는 표현으로 수정 - 로직 수정으로 인한 테스트케이스 fail을 해결
|
늦어서 죄송합니다. |
| @@ -45,11 +45,12 @@ describe('GameManager Test', () => { | |||
| [0, 0, 5, 5, 0, 0, 9, 9], | |||
| ['aa', 'bb'], | |||
| ], | |||
| ])('judgeWinners Test (%#)', (carNames, moves, winners) => { | |||
| ])('승자를 판별하는 테스트 (%#)', (carNames, moves, winners) => { | |||
There was a problem hiding this comment.
잘 통일해주셨는데요~ 요부분도 보다 "어떻게" 판단하는지 에 대한 정보가 추가된다던가 하는 등 더 구체적이면 좋을 것 같아요! 미션에서는 README.md를 통해 기능요구사항을 정리해보는 연습과 함께 하고 있어서 와닿지 않을지도 모르겠지만, 테스트코드는 하나의 README.md 의 역할을 하기도 합니다. 테스트 설명에 대해서 step2 구현해보시면서 같이 고민해보아요~!
No description provided.