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

[클린코드 6기 송용주] 자동차 경주 미션 STEP 1 #222

Open
wants to merge 21 commits into
base: testrace
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"rules": {},
"env": {
"es6": true,
"node": true
"node": true,
"jest": true
},
"parserOptions": {
"ecmaVersion": "latest",
Expand Down
9 changes: 9 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"singleQuote": true,
"bracketSpacing": true,
"tabWidth": 2,
"printWidth": 120,
"semi": true,
"arrowParens": "always",
"trailingComma": "all"
}
30 changes: 30 additions & 0 deletions __tests__/domain/car.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Car } from '../../src/domain/car.js';

describe('객체 생성', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

객체 생성을 테스트하는 것은 너무 좋습니다!
다만 사용자 입장에서의 시나리오를 고민해보셨으면 좋겠습니다 :)

it('생성', () => {
const car = new Car('name');
Copy link
Contributor

Choose a reason for hiding this comment

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

테스트 코드이지만 name이라는 필드에 정말 존재할 것 같은 데이터를 넣어주는게 좋지 않을까요?!


expect(car.name).toEqual('name');
expect(car.position).toEqual(0);
Comment on lines +7 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

toEqual()말고도 괜찮은 API가 많으니 한번 확인해보세요!
https://jestjs.io/docs/expect

});
});

describe('이동', () => {
let car;

beforeEach(() => {
car = new Car('name', 0);
});

it('이동', () => {
car.move(() => true);

expect(car.position).toEqual(1);
});

it('정지', () => {
car.move(() => false);

expect(car.position).toEqual(0);
});
});
11 changes: 11 additions & 0 deletions __tests__/domain/move_condition.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { isSatisfied } from '../../src/domain/move_condition.js';

describe('전진 조건', () => {
it('4 이상일 때 전진 가능', () => {
expect(isSatisfied(4)).toBeTruthy();
});

it('3 이하일 때 전진 불가', () => {
expect(isSatisfied(3)).toBeFalsy();
});
});
18 changes: 18 additions & 0 deletions __tests__/domain/name.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Name } from '../../src/domain/name.js';

describe('객체 생성', () => {
it.each(['a', 'abcde'])('생성', (value) => {
const name = new Name(value);
expect(name.value).toEqual(value);
});

it.each(['', null, undefined])("'%s' 값으로 생성 불가", (value) => {
expect(() => new Name(value)).toThrow('빈 값으로 생성할 수 없습니다.');
});

it('길이 초과 생성 불가', () => {
expect(() => new Name('overlength')).toThrow(
'5자 이상의 이름을 생성할 수 없습니다.',
);
});
});
33 changes: 33 additions & 0 deletions __tests__/domain/position.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Position } from '../../src/domain/position.js';

describe('객체 생성', () => {
it('생성', () => {
const position = new Position();
expect(position.value).toEqual(0);
});

it('음수로 생성 불가', () => {
expect(() => new Position(-1)).toThrow('0보다 작을 수 없습니다.');
});
});

describe('위치 값 증가', () => {
it('증가', () => {
const position = new Position(10);

position.increase();

expect(position.value).toEqual(11);
});
});

it.each([
[10, 10, true],
[10, 11, false],
])('위치 값 비교', (source, target, expected) => {
const position = new Position(source);

const actual = position.isSameAs(target);

expect(actual).toEqual(expected);
});
39 changes: 39 additions & 0 deletions __tests__/domain/winners.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Car } from '../../src/domain/car.js';
import { getMaxPosition, getWinners } from '../../src/domain/winners.js';

describe('최대 위치 구하기', () => {
const k3 = new Car('k3', 7);
const k5 = new Car('k5', 5);
const k7 = new Car('k7', 3);
const cars = [k3, k5, k7];

const actual = getMaxPosition(cars);

expect(actual).toEqual(7);
});

describe('우승자 구하기', () => {
it('한 대의 자동차가 우승', () => {
const k3 = new Car('k3', 7);
const k5 = new Car('k5', 5);
const k7 = new Car('k7', 3);
const cars = [k3, k5, k7];

const actual = getWinners(cars);

expect(actual).toEqual(['k3']);
});

it('세 대의 자동차가 우승', () => {
const k3 = new Car('k3', 8);
const k5 = new Car('k5', 7);
const k7 = new Car('k7', 6);
const k8 = new Car('k8', 8);
const k9 = new Car('k9', 3);
const cars = [k3, k5, k7, k8, k9];

const actual = getWinners(cars);

expect(actual).toEqual(['k3', 'k8']);
Copy link
Contributor

Choose a reason for hiding this comment

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

세 대의 자동차라고 정의 되어있는 것 같은데 맞을까요..?

});
});
34 changes: 34 additions & 0 deletions docs/REQUIREMENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
## 자동차 경주

### 기능 목록
- 이름
- [x] 길이는 1~5자로 제한

- 위치
- [x] 음수로 초기화 불가
- [x] 1씩 증가한다.
- [x] 동등성 비교

- 자동차
- [x] 이름
- [x] 위치
- [x] 전진할 수 있다.
- [x] 전진 조건을 만족하는 경우에 전진한다.

- 전진 조건
- [x] 4이상인 경우 전진한다.

- 경주
- [x] 5회 고정 진행
- [x] 우승자 선정
- [x] 가장 멀리간 자동차가 우승한다.
- [x] 우승자는 한 명 이상일 수 있다.

- 입력
- [x] 자동차 이름은 쉼표(,)를 기준으로 구분
- [x] 사용자가 잘못된 입력 값을 작성한 경우 프로그램을 종료한다.
- [x] Name 객체를 생성할 때 예외 처리를 하지 않아 종료된다.

- 출력
- [x] 전진하는 자동차를 출력할 때 자동차 이름을 같이 출력한다.
- [x] 우승자가 여러 명일 경우 쉼표(,)를 이용하여 구분한다.
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"version": "1.0.0",
"description": "우아한테크코스 웹 프론트엔드 레벨1 자동차 경주 미션",
"main": "src/index.js",
"type": "module",
"scripts": {
"start": "npm run build-prod && node dist/bundle.js",
"build-dev": "webpack --mode development",
Expand All @@ -28,10 +29,12 @@
"babel-jest": "^29.3.1",
"babel-loader": "^9.1.2",
"clean-webpack-plugin": "^4.0.0",
"eslint": "^8.31.0",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prettier": "^5.1.3",
"esm": "^3.2.25",
"jest": "^29.3.1",
"prettier": "^2.8.2",
"prettier": "3.0.0",
"webpack": "^5.75.0",
"webpack-cli": "^5.0.1"
},
Expand Down
30 changes: 30 additions & 0 deletions src/domain/car.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Name } from './name.js';
import { Position } from './position.js';

export class Car {
Copy link
Contributor

Choose a reason for hiding this comment

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

단일 파일에서 단 하나의 모듈만 내보내고 있으면 export default 또한 고려해볼법할 것 같은데 이유가 있으셨을까요?

#name;
#position;

constructor(name, position = 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

position = 0 기본 매개변수 처리 좋습니다 👍

this.#name = new Name(name);
this.#position = new Position(position);
}

move(movable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Car.move(콜백 함수) 이동시켜주는 함수에서 콜백으로 함수를 받는다는 것이 일반적이지는 않아서? 그럴까요
사용하는 측에서는 인지부조화가 오는 것 같은데 어떤 이유로 이런 설계를 하셨는지 궁금합니다~

if (movable()) {
this.#position.increase();
}
}

hasSamePosition(value) {
return this.#position.isSameAs(value);
}

get name() {
return this.#name.value;
}

get position() {
return this.#position.value;
}
}
3 changes: 3 additions & 0 deletions src/domain/move_condition.js
Copy link
Contributor

Choose a reason for hiding this comment

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

도메인 코드라고 하기에는 하는 역할이 너무 없어서 분리되기는 조금 애매하다는 생각은 드네요!

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const THRESHOLD = 4;

export const isSatisfied = (number) => number >= THRESHOLD;
27 changes: 27 additions & 0 deletions src/domain/name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
export class Name {
#MIN_LENGTH = 1;
#MAX_LENGTH = 5;
Comment on lines +2 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

변하지 않는 상수인데 클래스 내부의 필드로 존재할 필요가 있을까요?


#value;

constructor(value) {
this.#validate(value);
this.#value = value;
}

#validate(value) {
if (!value || value.length < this.#MIN_LENGTH) {
throw new Error('빈 값으로 생성할 수 없습니다.');
}

if (value.length > this.#MAX_LENGTH) {
throw new Error(
`${this.#MAX_LENGTH}자 이상의 이름을 생성할 수 없습니다.`,
);
}
}

get value() {
return this.#value;
}
}
25 changes: 25 additions & 0 deletions src/domain/position.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export class Position {
#MIN_VALUE = 0;
#INCREASE_VALUE = 1;

#value;

constructor(value = 0) {
if (value < this.#MIN_VALUE) {
throw new Error(`${this.#MIN_VALUE}보다 작을 수 없습니다.`);
}
this.#value = value;
}

increase() {
this.#value += this.#INCREASE_VALUE;
}

isSameAs(value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isSameAs + 뒤에 붙는 접미사를 통해서 무엇을 해주는건지 알 수 있을까요?
사용하는 측에서는 isSameAs(someValue)가 있더라도 비교 대상 연산을 모르기에 헷갈릴 수 있을 것 같아요!

return this.#value === value;
}

get value() {
return this.#value;
}
}
10 changes: 10 additions & 0 deletions src/domain/random_move_strategy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { isSatisfied } from './move_condition.js';

const RANDOM_BOUND = 10;

const randomNumber = () => Math.floor(Math.random() * RANDOM_BOUND);
Comment on lines +3 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

랜덤의 바운더리가 최대 10까지라면 과연 이 함수는 randomNumber()로 정의하는게 맞을까요?


export const movable = () => {
const number = randomNumber();
Copy link
Contributor

Choose a reason for hiding this comment

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

movable() 에서 randomNumber()의 의존성을 가지고 있는게 좋을까요?

return isSatisfied(number);
};
10 changes: 10 additions & 0 deletions src/domain/winners.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export function getMaxPosition(cars) {
return Math.max(...cars.map((car) => car.position));
}

export const getWinners = (cars) => {
const maxPosition = getMaxPosition(cars);
return cars
.filter((car) => car.hasSamePosition(maxPosition))
.map((car) => car.name);
};
26 changes: 26 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Car } from './domain/car.js';
import { movable } from './domain/random_move_strategy.js';
import { getWinners } from './domain/winners.js';
import { inputNames } from './view/input_view.js';
import { printCar, printWinners } from './view/output_view.js';

const RACE_ROUND = 5;

const main = async () => {
const names = await inputNames();
const cars = names.map((name) => new Car(name));

console.log('실행 결과');
for (let i = 0; i < RACE_ROUND; i++) {
cars.forEach((car) => {
car.move(movable);
printCar(car);
});
console.log();
}

const winners = getWinners(cars);
printWinners(winners);
};

main();
8 changes: 8 additions & 0 deletions src/view/input_view.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { readLineAsync } from './read_line_async.js';

export const inputNames = async () => {
const names = await readLineAsync(
'경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).',
);
return names.split(',').map((name) => name.trim());
};
7 changes: 7 additions & 0 deletions src/view/output_view.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const printCar = (car) => {
console.log(`${car.name}: ${'-'.repeat(car.position)}`);
};

export const printWinners = (winners) => {
console.log(`${winners.join(', ')}가 최종 우승했습니다.`);
};
24 changes: 24 additions & 0 deletions src/view/read_line_async.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import readline from 'readline';

export function readLineAsync(query) {
return new Promise((resolve, reject) => {
if (arguments.length !== 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

더 넓은 에러 범위를 생각해보시는게 어떨까요?
숫자를 검사할때는 타입과 범위를 고민해보셔야합니다 1 이상 이하 모두 생각해볼 필요가 있는 것이죠

reject(new Error('arguments must be 1'));
}

if (typeof query !== 'string') {
reject(new Error('query must be string'));
}

const rl = readline.createInterface({
input: process.stdin,
output: process.stdout,
});

const question = `${query}\n`;
rl.question(question, (input) => {
rl.close();
resolve(input);
});
});
}