[자동차 경주] 정석찬 미션 제출합니다.#433
[자동차 경주] 정석찬 미션 제출합니다.#433standard-Chan wants to merge 14 commits intowoowacourse-precourse:mainfrom
Conversation
2wndrhs
left a comment
2wndrhs
left a comment
There was a problem hiding this comment.
각 클래스로 적절하게 역할을 분리하신 것 같습니다👍
2주차 공통 피드백에 나와있는 것처럼 각 클래스의 기능에 대해 단위 테스트를 작성하여 기능을 점검하면 더 좋을 것 같네요🤗
src/View.js
Outdated
| import { Console } from "@woowacourse/mission-utils"; | ||
|
|
||
| class View { | ||
| #inputCarNameRegx = /^([a-zA-z]{1,5})(,[a-zA-Z]{1,5})*$/ |
There was a problem hiding this comment.
정규식으로 유효성 검증하는게 좋네요👍
Regx 이름은 의도하신게 아니라면 Regex가 보다 일반적인 정규식 이름인 것 같습니다!
Code Spell Checker 익스텐션 설치해서 사용하시면 좋을 것 같네요
src/View.js
Outdated
| async readInputCar() { | ||
| const carNamesStr = await Console.readLineAsync(this.#carInputMessage); | ||
|
|
||
| if (!this.#inputCarNameRegx.test(carNamesStr)){ |
There was a problem hiding this comment.
유효성 검사는 비즈니스 로직에 해당한다고 생각하는데 View에서 유효성 검사를 수행하면 UI와 비즈니스 로직의 결합이 강해져 추후 유효성 검사 로직이 변경되면 View까지 수정해야 되는 상황이 발생할 수 있을 것 같습니다!
View 클래스는 사용자와의 상호작용에 집중하고 유효성 검사는 별도의 클래스나 모듈로 분리하는 것이 변경에 유연하게 대응할 수 있을 것 같네요
src/View.js
Outdated
| if (raceCount==NaN || raceCount <= 0){ | ||
| throw new Error("[ERROR] : 잘못된 숫자 입력\n"); | ||
| } |
There was a problem hiding this comment.
JS에서 NaN은 다른 어떤 값과도 같지 않아 raceCount == NaN은 항상 false를 반환합니다!(심지어 NaN == NaN or NaN === NaN도 false를 반환합니다)
raceCount가 NaN인지 검사하려면 Number.isNaN() 함수를 사용하시면 될 것 같습니다
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NaN
| @@ -0,0 +1,31 @@ | |||
| import { Console } from "@woowacourse/mission-utils"; | |||
|
|
|||
| class View { | |||
src/Car.js
Outdated
| @@ -0,0 +1,24 @@ | |||
| class Car { | |||
| #forwadCount=0; | |||
There was a problem hiding this comment.
forwadCount -> forwardCount로 수정되어야 할 것 같습니다!
| return this.#raceCount; | ||
| } | ||
|
|
||
| racing() { |
There was a problem hiding this comment.
메서드나 함수의 이름은 동사로 시작하도록 짓는 것이 가독성이 좋을 것 같습니다!
https://velog.io/@pjc0247/%EA%B0%9C%EB%B0%9C%EC%9E%90-%EC%98%81%EC%96%B4
src/Race.js
Outdated
| }) | ||
|
|
||
| this.#cars.forEach(car => { | ||
| if (car.getForwardCount() == max){ |
There was a problem hiding this comment.
JS에서 == (동등 연산자)는 비교하는 두 값의 타입이 다르면, 타입을 강제로 변환하여 비교하기 때문에 타입 변환을 수행하지 않고 비교하는 === (일치 연산자)를 사용하는 것이 코드의 예측 가능성이 좋습니다!
https://github.com/airbnb/javascript?tab=readme-ov-file#comparison--eqeqeq
src/Race.js
Outdated
| Winner() { | ||
| let max = 0; | ||
| this.#cars.forEach(car => { | ||
| let forwardCount = car.getForwardCount(); |
There was a problem hiding this comment.
forwardCount 변수는 추후에 재할당 되지 않으니 const로 선언해도 될 것 같습니다!
src/Race.js
Outdated
| let max = 0; | ||
| this.#cars.forEach(car => { | ||
| let forwardCount = car.getForwardCount(); | ||
| if (max < forwardCount) { | ||
| max = forwardCount; | ||
| } | ||
| }) |
There was a problem hiding this comment.
| let max = 0; | |
| this.#cars.forEach(car => { | |
| let forwardCount = car.getForwardCount(); | |
| if (max < forwardCount) { | |
| max = forwardCount; | |
| } | |
| }) | |
| const max = Math.max(...this.#cars.map(car => car.getForwardCount())); |
로 짧게 줄일 수 있을 것 같습니다!
src/Race.js
Outdated
| Console.print(''); | ||
| } | ||
|
|
||
| Winner() { |
There was a problem hiding this comment.
해당 함수는 우승자를 설정하는 역할을 하는 것 같은데 이름에서 그 역할을 잘 나타내지 못하는 것 같습니다!
특히 JS에서 변수나 함수, 메서드의 이름은 카멜 케이스로 짓는 것이 일반적이니 다른 이름을 고민해보시면 좋을 것 같네요!
2 participants
[자동차 경주] 정석찬 미션 제출합니다.