-
Notifications
You must be signed in to change notification settings - Fork 476
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
[자동차 경주] 윤선례 미션 제출합니다 #451
base: main
Are you sure you want to change the base?
[자동차 경주] 윤선례 미션 제출합니다 #451
Conversation
* 기능 목록 작성
* Random.pickNumberInRange()을 활용해 랜덤 값을 추출 * 추출한 랜덤 값이 4 이상일 경우 전진한다 * 전진할 경우 이전 회차에서 전진한 문자에 - 기호를 추가로 붙여준다 * 시도한 횟수만큼 전진 로직을 실행한다 * 전진 결과를 콘솔로 출력한다
* 가장 전진거리가 긴 우승자를 출력 * 같은 전진거리인 우승자가 여러명일 경우 쉼표로 구분하여 출력
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요! 먼 길 달려와 주셔서 너무 감사합니다.
제공해주신 리뷰는 물론 제 성장에 도움이 많이 되고 있습니다.👍
너무 감사한 마음에 저도 코드를 샅샅이 보게 된 것 같습니다.
벌써 프리코스의 절반이 지나갔네요. 나머지 절반도 화이팅입니다!😊
}; | ||
|
||
export const displayRaceProgress = (cars) => { | ||
cars.forEach((car) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Destructuring 문법을 사용하면 좋을 것 같습니다🙂
cars.forEach(({name, distance}) => {
Console.print(`${name} : ${distance}`);
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 다시 보니 그렇네요! 놓쳤던 부분인 것 같습니다 ;ㅅ;
알려주셔서 감사해요!
const attempts = await getAttempts(); | ||
validateAttempts(attempts); | ||
|
||
for (let attemptIndex = 0; attemptIndex < attempts; attemptIndex++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이게 말씀해주신 반복 부분이군요! 제 개인적인 생각으론 그대로 i
가 더 좋아보여요!
추가로 이 부분을 레이싱을 진행하는 기능 하나로 봐서 하나의 함수로 분리하는 것도 좋아보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다시 보니 그렇네요! 반복 부분을 함수로 감싸야 문맥에 더 자연스러울 것 같아요
좋은 의견 감사해요!
} | ||
|
||
createCars(splittedNames) { | ||
return splittedNames.map((name) => new Car(name)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 생각하기에 클래스 문법의 이점은 클래스 내 다른 함수에서 조작한 this.cars
를 클래스 내 어디서든 그대로 사용할 수 있다는 점인 것 같습니다.
마침 this.car
가 선언되어 있는 참이니까 createCars 함수 내부에서 this.cars
안에 값을 직접 넣는 방식은 어떨까요? 그러면 execute 내에서 this.createCars(splittedNames);
만으로 쉽게 this.cars
를 조작할 수 있어보여요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 말씀해주신 것처럼 createCars 함수를 수정하면 더 좋을 것 같아요!
꼼꼼한 피드백 감사합니다 👍👍
validateAttempts(attempts); | ||
|
||
for (let attemptIndex = 0; attemptIndex < attempts; attemptIndex++) { | ||
this.startCarRace(this.cars); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constructor에 this.cars
가 선언되어 있으므로 this.car
를 파라미터로 굳이 넣지 않아도 괜찮을 것 같습니다.
class 문법의 장점이죠😊 밑의 함수도 마찬가지입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 그렇네요! 클래스 문법이 아직 익숙하지 않아서 자꾸 놓치는 것 같아요
다음 번엔 클래스 문법의 장점을 좀 더 살려서 작성해볼게요!
피드백 감사합니다! :)
|
||
export const utils = { | ||
hasValidCharacter(name) { | ||
const regex = /^[\p{L},\s]*$/u; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정규표현식을 이용해 세부적인 오류를 검출하셨네요!👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 선례님! 2주차 과제 너무 고생 많으셨습니다 :>
구현 목표에 맞게 심플하면서도 깔끔하게 로직이 잘 작성되어 있다고 느꼈습니다! 커밋 메세지에서 어떤 부분을 변경했는지 구체적으로 적어주셔서 이해하는데 많이 도움이 되었습니다 :>
코드도 관심사에 따라서 분리하려고 노력한 모습이 보여서 인상적이었습니다. 몇몇 로직의 이름이나 메서드 이름이 조금 더 직관적이면 더 읽기 편한 코드가 될 것 같습니다 😄
추가적으로 지금보다 조금 더 세밀하게 코드가 나눠질 수 있는 부분이 있다고 생각이 듭니다! 가령 예를들어서 지금은 Model에 Car
하나밖에 없지만 경주 게임 자체도 하나의 클래스로 만들 수 있지 않을까요? 이런 부분이 조금 더 보완되면 훨씬 좋은 코드가 될 것 같습니다
2주차 고생 많으셨고 3주차도 화이팅입니다!
export const MOVE_REQUIREMENT = 3; | ||
export const CAR_NAME_LENGTH_LIMIT = 5; | ||
export const DELIMITER = ','; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
자주 쓰이는 숫자나 변수들을 상수로 관리하는 건 좋은 방법이라고 생각해요! 지금 작성해주신 상수에서 조금 더 나아가서 필요한 게 어떤게 있을까 고민해보면 더 좋을 것 같아요 :>
예를들어 지금은 NAME_LENGTH_LIMIT
으로 5라는 상한선만 가지고 있는데 하한선도 포함해 보는건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 의견 감사합니다! 하한선을 두는 것도 좋은 것 같아요! :)
상수명 부분에서도 확장성을 좀 더 고려해볼게요!
INPUT_ADDITION_CAR: | ||
'경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n', | ||
INPUT_ATTEMPTS: '시도할 횟수는 몇 회인가요?\n', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 이름이 조금 더 명확하게 변경되면 좋을 것 같아요 INPUT_ADDITION_CAR
라는 이름보다 INPUT_CAR_NAME
이라는 이름이 더 직관적이고 이해하기 쉽다고 생각하는데 어떻게 생각하시나용?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아! 추천해주신 이름이 더 직관적인 것 같아요!
변수명이나 함수명 지을 때 많이 고민했는데 더 간단한 이름으로 지을 수 있었군요😅
async execute() { | ||
const enteredCarNames = await getCarNames(); | ||
const splittedNames = enteredCarNames.split(DELIMITER); | ||
validCarName(splittedNames); | ||
this.cars = this.createCars(splittedNames); | ||
const attempts = await getAttempts(); | ||
validateAttempts(attempts); | ||
|
||
for (let attemptIndex = 0; attemptIndex < attempts; attemptIndex++) { | ||
this.startCarRace(this.cars); | ||
displayRaceProgress(this.cars); | ||
} | ||
|
||
const result = this.getWinners(this.cars); | ||
announceWinners(result.join(',')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금도 책임 소재에 맞게 메서드를 잘 분리해 주셨다고 생각해요! 여기서 조금더 수정해본다면 execute를 약간 더 다듬어 볼 수 있지 않을까요? 이렇게 다듬어보는건 어떨까 제안드려봅니다
async execute() { | |
const enteredCarNames = await getCarNames(); | |
const splittedNames = enteredCarNames.split(DELIMITER); | |
validCarName(splittedNames); | |
this.cars = this.createCars(splittedNames); | |
const attempts = await getAttempts(); | |
validateAttempts(attempts); | |
for (let attemptIndex = 0; attemptIndex < attempts; attemptIndex++) { | |
this.startCarRace(this.cars); | |
displayRaceProgress(this.cars); | |
} | |
const result = this.getWinners(this.cars); | |
announceWinners(result.join(',')); | |
} | |
async execute() { | |
const splittedNames = await this.getValidatedCarNames(); | |
this.cars = this.createCars(splittedNames); | |
const attempts = await this.getValidatedAttempts(); | |
this.executeRaceRounds(attempts); | |
const winners = this.getWinners(this.cars); | |
announceWinners(winners.join(',')); | |
} | |
async getValidatedCarNames() { | |
const enteredCarNames = await getCarNames(); | |
const splittedNames = enteredCarNames.split(DELIMITER); | |
validCarName(splittedNames); | |
return splittedNames; | |
} | |
async getValidatedAttempts() { | |
const attempts = await getAttempts(); | |
validateAttempts(attempts); | |
return attempts; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제안해주신 코드가 더 깔끔하고 보기 좋은 것 같아요!
execute 함수 내부에서의 엔터 간격도 가독성에 큰 영향을 주는 것 같아요
다음 번 과제때 참고해서 구현해볼게요! 감사합니다 👍👍
자동차 경주 게임
초간단 자동차 경주 게임을 구현한다.
프로그램 실행 시
전진 로직
Random.pickNumberInRange()
을 활용해 랜덤 값을 추출한다우승자 발표
에러 처리
사용자가 잘못된 값을 입력할 경우 "[ERROR]"로 시작하는 메시지와 함께 Error를 발생시킨 후 애플리케이션은 종료되어야 한다.