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

[자동차 경주] 윤선례 미션 제출합니다 #451

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
37 changes: 36 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,36 @@
# javascript-racingcar-precourse
### 자동차 경주 게임

초간단 자동차 경주 게임을 구현한다.

### 프로그램 실행 시

- 프로그램 실행 시 ‘경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)’ 가 출력된다
- 자동차 이름을 쉼표 기준으로 구분하여 입력한다
- 각각의 자동차 이름은 5글자 이하로 입력한다
- 올바르게 입력되었을 경우 시도할 횟수를 입력할 수 있다
- 시도한 횟수만큼 전진 로직을 실행한다

### 전진 로직

- `Random.pickNumberInRange()`을 활용해 랜덤 값을 추출한다
- 추출한 랜덤 값이 4 이상일 경우 전진한다
- 전진할 경우 이전 회차에서 전진한 문자에 - 기호를 추가로 붙여준다
- 매 횟수마다 전진 결과를 콘솔로 출력한다

### 우승자 발표

- 가장 전진거리가 긴 우승자를 출력한다
- 같은 전진거리인 우승자가 여러명일 경우 쉼표로 구분하여 출력한다 (우승자는 한 명 이상일 수 있다)

### 에러 처리

- 사용자가 잘못된 값을 입력할 경우 "[ERROR]"로 시작하는 메시지와 함께 Error를 발생시킨 후 애플리케이션은 종료되어야 한다.

- 입력 값이 비어있는 경우 "[ERROR] 입력 값이 비어 있습니다"를 출력한다.
- 구분자가 입력되지 않은 경우 "[ERROR] 구분자가 입력되지 않았습니다"를 출력한다.
- 음수 값이 입력된 경우 "[ERROR] 음수는 입력하실 수 없습니다"를 출력한다.
- 시도 횟수가 입력되지 않은 경우 "[ERROR] 시도 횟수를 입력해 주세요"를 출력한다.
- 시도 횟수 입력이 숫자가 아닌 경우 "[ERROR] 시도 횟수는 숫자만 입력 가능합니다"를 출력한다.
- 자동차 이름을 쉼표로 구분하지 않은 경우 "[ERROR] 자동차 이름을 쉼표로 구분하여 입력해 주세요"를 출력한다.
- 자동차 이름이 5자를 초과한 경우 "[ERROR] 자동차 이름은 5자 이하로 입력해 주세요"를 출력한다.
- 자동차 이름에 공백이 포함된 경우 "[ERROR] 이름에 공백 문자는 사용하실 수 없습니다"를 출력한다.
20 changes: 10 additions & 10 deletions __tests__/ApplicationTest.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import App from "../src/App.js";
import { MissionUtils } from "@woowacourse/mission-utils";
import App from '../src/App.js';
import { MissionUtils } from '@woowacourse/mission-utils';

const mockQuestions = (inputs) => {
MissionUtils.Console.readLineAsync = jest.fn();
Expand All @@ -19,18 +19,18 @@ const mockRandoms = (numbers) => {
};

const getLogSpy = () => {
const logSpy = jest.spyOn(MissionUtils.Console, "print");
const logSpy = jest.spyOn(MissionUtils.Console, 'print');
logSpy.mockClear();
return logSpy;
};

describe("자동차 경주", () => {
test("기능 테스트", async () => {
describe('자동차 경주', () => {
test('기능 테스트', async () => {
// given
const MOVING_FORWARD = 4;
const STOP = 3;
const inputs = ["pobi,woni", "1"];
const logs = ["pobi : -", "woni : ", "최종 우승자 : pobi"];
const inputs = ['pobi,woni', '1'];
const logs = ['pobi : -', 'woni : ', '최종 우승자 : pobi'];
const logSpy = getLogSpy();

mockQuestions(inputs);
Expand All @@ -46,15 +46,15 @@ describe("자동차 경주", () => {
});
});

test("예외 테스트", async () => {
test('예외 테스트', async () => {
// given
const inputs = ["pobi,javaji"];
const inputs = ['pobi,javaji'];
mockQuestions(inputs);

// when
const app = new App();

// then
await expect(app.run()).rejects.toThrow("[ERROR]");
await expect(app.run()).rejects.toThrow('[ERROR]');
});
});
10 changes: 9 additions & 1 deletion src/App.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import Controller from './controller/controller.js';

class App {
async run() {}
constructor() {
this.controller = new Controller();
}

async run() {
await this.controller.execute();
}
}

export default App;
3 changes: 3 additions & 0 deletions src/constants/constant.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const MOVE_REQUIREMENT = 3;
export const CAR_NAME_LENGTH_LIMIT = 5;
export const DELIMITER = ',';
Comment on lines +1 to +3

Choose a reason for hiding this comment

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

자주 쓰이는 숫자나 변수들을 상수로 관리하는 건 좋은 방법이라고 생각해요! 지금 작성해주신 상수에서 조금 더 나아가서 필요한 게 어떤게 있을까 고민해보면 더 좋을 것 같아요 :>

예를들어 지금은 NAME_LENGTH_LIMIT 으로 5라는 상한선만 가지고 있는데 하한선도 포함해 보는건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

좋은 의견 감사합니다! 하한선을 두는 것도 좋은 것 같아요! :)
상수명 부분에서도 확장성을 좀 더 고려해볼게요!

18 changes: 18 additions & 0 deletions src/constants/messages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export const PROMPT_MESSAGES = Object.freeze({
INPUT_ADDITION_CAR:
'경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n',
INPUT_ATTEMPTS: '시도할 횟수는 몇 회인가요?\n',
Comment on lines +2 to +4

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 이라는 이름이 더 직관적이고 이해하기 쉽다고 생각하는데 어떻게 생각하시나용?

Copy link
Author

Choose a reason for hiding this comment

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

아! 추천해주신 이름이 더 직관적인 것 같아요!
변수명이나 함수명 지을 때 많이 고민했는데 더 간단한 이름으로 지을 수 있었군요😅

});

export const ERROR_MESSAGES = Object.freeze({
EMPTY_INPUT: '[ERROR] 입력 값이 비어 있습니다.',
MISSING_DELIMITER: '[ERROR] 구분자가 입력되지 않았습니다.',
NEGATIVE_NUMBER: '[ERROR] 음수는 입력하실 수 없습니다.',
EMPTY_ATTEMPTS: '[ERROR] 시도 횟수를 입력해 주세요',
INVALID_ATTEMPTS: '[ERROR] 시도 횟수는 숫자만 입력 가능합니다',
CAR_NAME_DELIMITER_REQUIRED:
'[ERROR] 자동차 이름을 쉼표로 구분하여 입력해 주세요.',
CAR_NAME_LENGTH_EXCEEDED: '[ERROR] 자동차 이름은 5자 이하로 입력해 주세요',
CAR_NAME_CONTAINS_WHITESPACE:
'[ERROR] 이름에 공백 문자는 사용하실 수 없습니다',
});
55 changes: 55 additions & 0 deletions src/controller/Controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import Car from '../model/model.js';
import { DELIMITER } from '../constants/constant.js';
import { Random } from '@woowacourse/mission-utils';
import {
getAttempts,
getCarNames,
announceWinners,
displayRaceProgress,
} from '../view/view.js';
import { validateAttempts, validCarName } from '../utils/validator.js';

class Controller {
constructor() {
this.cars = [];
}

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++) {

Choose a reason for hiding this comment

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

이게 말씀해주신 반복 부분이군요! 제 개인적인 생각으론 그대로 i가 더 좋아보여요!

추가로 이 부분을 레이싱을 진행하는 기능 하나로 봐서 하나의 함수로 분리하는 것도 좋아보입니다.

Copy link
Author

Choose a reason for hiding this comment

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

다시 보니 그렇네요! 반복 부분을 함수로 감싸야 문맥에 더 자연스러울 것 같아요
좋은 의견 감사해요!

this.startCarRace(this.cars);

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 문법의 장점이죠😊 밑의 함수도 마찬가지입니다.

Copy link
Author

Choose a reason for hiding this comment

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

아 그렇네요! 클래스 문법이 아직 익숙하지 않아서 자꾸 놓치는 것 같아요
다음 번엔 클래스 문법의 장점을 좀 더 살려서 작성해볼게요!
피드백 감사합니다! :)

displayRaceProgress(this.cars);
}

const result = this.getWinners(this.cars);
announceWinners(result.join(','));
}
Comment on lines +17 to +32

Choose a reason for hiding this comment

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

지금도 책임 소재에 맞게 메서드를 잘 분리해 주셨다고 생각해요! 여기서 조금더 수정해본다면 execute를 약간 더 다듬어 볼 수 있지 않을까요? 이렇게 다듬어보는건 어떨까 제안드려봅니다

Suggested change
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;
}

Copy link
Author

Choose a reason for hiding this comment

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

제안해주신 코드가 더 깔끔하고 보기 좋은 것 같아요!
execute 함수 내부에서의 엔터 간격도 가독성에 큰 영향을 주는 것 같아요
다음 번 과제때 참고해서 구현해볼게요! 감사합니다 👍👍


createCars(splittedNames) {
return splittedNames.map((name) => new Car(name));

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를 조작할 수 있어보여요!

Copy link
Author

Choose a reason for hiding this comment

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

아하 말씀해주신 것처럼 createCars 함수를 수정하면 더 좋을 것 같아요!
꼼꼼한 피드백 감사합니다 👍👍

}

startCarRace(cars) {
cars.forEach((car) => {
const number = Random.pickNumberInRange(0, 9);
car.race(number);
});
return cars;
}

getWinners(cars) {
const maxScore = Math.max(...cars.map((car) => car.distance.length));

return cars
.filter((car) => car.distance.length === maxScore)
.map((car) => car.name);
}
}

export default Controller;
16 changes: 16 additions & 0 deletions src/model/Model.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { MOVE_REQUIREMENT } from '../constants/constant.js';

class Car {
constructor(name) {
this.name = name;
this.distance = '';
}

race(pickNumber) {
if (pickNumber > MOVE_REQUIREMENT) {
this.distance = this.distance + '-';
}
}
}

export default Car;
21 changes: 21 additions & 0 deletions src/utils/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { CAR_NAME_LENGTH_LIMIT } from '../constants/constant.js';

export const utils = {
hasValidCharacter(name) {
const regex = /^[\p{L},\s]*$/u;

Choose a reason for hiding this comment

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

정규표현식을 이용해 세부적인 오류를 검출하셨네요!👍

return regex.test(name);
},
isNameLengthLimit(name) {
return name.length <= CAR_NAME_LENGTH_LIMIT;
},
hasWhitespace(name) {
const regex = /\s/;
return regex.test(name);
},
isNumberType(number) {
return !isNaN(number);
},
isNegativeNumber(number) {
return number < 0;
},
};
28 changes: 28 additions & 0 deletions src/utils/validator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { utils } from './utils.js';
import { ERROR_MESSAGES } from '../constants/messages.js';

export const validCarName = (splittedNames) => {
splittedNames.forEach((car) => {
if (!utils.hasValidCharacter(car)) {
throw new Error(ERROR_MESSAGES.CAR_NAME_DELIMITER_REQUIRED);
}
if (!utils.isNameLengthLimit(car)) {
throw new Error(ERROR_MESSAGES.CAR_NAME_LENGTH_EXCEEDED);
}
if (utils.hasWhitespace(car)) {
throw new Error(ERROR_MESSAGES.CAR_NAME_CONTAINS_WHITESPACE);
}
});
};

export const validateAttempts = (attempts) => {
if (!attempts) {
throw new Error(ERROR_MESSAGES.EMPTY_ATTEMPTS);
}
if (!utils.isNumberType(attempts)) {
throw new Error(ERROR_MESSAGES.INVALID_ATTEMPTS);
}
if (utils.isNegativeNumber(attempts)) {
throw new Error(ERROR_MESSAGES.NEGATIVE_NUMBER);
}
};
25 changes: 25 additions & 0 deletions src/view/View.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Console } from '@woowacourse/mission-utils';
import { PROMPT_MESSAGES } from '../constants/messages.js';

export const getCarNames = async () => {
const inputs = await Console.readLineAsync(
PROMPT_MESSAGES.INPUT_ADDITION_CAR
);
return inputs;
};

export const getAttempts = async () => {
const attempts = await Console.readLineAsync(PROMPT_MESSAGES.INPUT_ATTEMPTS);
return attempts;
};

export const announceWinners = (winner) => {
Console.print(`최종 우승자 : ${winner}`);
};

export const displayRaceProgress = (cars) => {
cars.forEach((car) => {

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}`);
});

Copy link
Author

Choose a reason for hiding this comment

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

앗 다시 보니 그렇네요! 놓쳤던 부분인 것 같습니다 ;ㅅ;
알려주셔서 감사해요!

Console.print(`${car.name} : ${car.distance}`);
});
Console.print(`\n`);
};