-
Notifications
You must be signed in to change notification settings - Fork 641
[자동차 경주] 오정진 미션 제출합니다. #747
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
base: main
Are you sure you want to change the base?
Changes from all commits
5354a73
e2b9947
1f9205b
6b0c356
14a88f9
fa7572e
f53a616
34c3604
d8ceb39
ab517aa
8d81cd1
ee1dd39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import Validator from '../src/utils/Validator'; | ||
|
|
||
| describe('Validator 함수 테스트', () => { | ||
| test('5글자 초과 시 예외 처리', () => { | ||
| const input = 'abcdefghi,abc'; | ||
|
|
||
| expect(() => Validator.validCarList(input)).toThrow('[ERROR]'); | ||
| }); | ||
|
|
||
| test('음의 정수 입력 시 예외 처리', () => { | ||
| const input = '-1'; | ||
|
|
||
| expect(() => Validator.rangeOverZero(input)).toThrow('[ERROR]'); | ||
| }); | ||
|
|
||
| test('정수 이외의 입력값에 대한 예외 처리', () => { | ||
| const input = 'abcd'; | ||
|
|
||
| expect(() => Validator.rangeOverZero(input)).toThrow('[ERROR]'); | ||
| }); | ||
|
|
||
| test('5글자 초과 시 false 반환', () => { | ||
| const input = 'abcdef'; | ||
|
|
||
| expect(Validator.validLength(input, { maxLength: 5 })).toBe(false); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| ## 구현할 기능 목록 | ||
|
|
||
| - [x] 경주할 자동차 이름을 입력받는다. 입력받은 자동차 이름 각각에 대해 `Car` 인스턴스를 생성한다. | ||
| - [x] 시도할 게임 횟수를 입력받는다. | ||
| - [x] 경주할 자동차에 대해 각각 0 ~ 9사이의 정수 중 무작위 값을 구한 후 4이상인 경우 이동한다. | ||
| - [x] 경주 게임 우승자는 이동 경로가 가장 긴 자동차이다. 게임 횟수가 0이 됐을 때(게임 완료) 우승한 자동차 이름을 쉼표(,)로 구분해 출력한다. | ||
| - [x] 사용자 입력을 검증한다. | ||
| - [x] 경주할 자동차 이름에 대한 검증 | ||
| - [x] 시도할 횟수에 대한 검증 | ||
| - [x] 검증 완료 후 에러 발생 시 `[ERROR]` 로 시작하는 에러 문구와 함께 예외를 throw하고 애플리케이션을 종료한다. | ||
|
|
||
| ## 테스트 코드 목록 | ||
|
|
||
| - [x] 사용자 입력 검증 기능 테스트 코드 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| import Game from './Game.js'; | ||
|
|
||
| class App { | ||
| async play() {} | ||
| async play() { | ||
| const game = new Game(); | ||
| await game.play(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. App.play()와 명확하게 구분할 수 있도록 game.play보다는 조금 더 구체적인 이름을 사용하면 어땠을까라는 생각이 듭니다 😁 |
||
| } | ||
| } | ||
|
|
||
| export default App; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { Random } from '@woowacourse/mission-utils'; | ||
|
|
||
| class Car { | ||
| #name = ''; | ||
| #path = ''; | ||
|
|
||
| constructor(name) { | ||
| this.#name = name; | ||
| } | ||
|
|
||
| move() { | ||
| const value = Random.pickNumberInRange(0, 9); | ||
|
ojj1123 marked this conversation as resolved.
|
||
| if (Number(value) < 4) { | ||
|
ojj1123 marked this conversation as resolved.
|
||
| return; | ||
| } | ||
|
|
||
| this.#path += '-'; | ||
| } | ||
|
|
||
| getName() { | ||
| return this.#name; | ||
| } | ||
|
|
||
| getPath() { | ||
| return this.#path; | ||
| } | ||
| } | ||
|
|
||
| export default Car; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { Console } from '@woowacourse/mission-utils'; | ||
|
|
||
| import Car from './Car.js'; | ||
| import Input from './Input.js'; | ||
| import Validator from './utils/Validator.js'; | ||
|
|
||
| class Game { | ||
| #cars = []; | ||
| #count = 0; | ||
|
|
||
| async play() { | ||
| await this.setCars(); | ||
| await this.setCount(); | ||
|
|
||
| Console.print('실행결과'); | ||
|
|
||
| for (let i = 0; i < this.#count; i++) { | ||
| this.#cars.forEach((car) => { | ||
| car.move(); | ||
| Console.print(`${car.getName()} : ${car.getPath()}`); | ||
| }); | ||
| Console.print(''); | ||
| } | ||
|
Comment on lines
+17
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분도 따로 함수화 했으면 좀 더 직관적으로 로직이 보일 것 같아요! |
||
|
|
||
| this.showWinner(); | ||
| } | ||
|
|
||
| async setCars() { | ||
| const input = new Input(); | ||
|
|
||
| const value = await input.readLine( | ||
| '경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 상수 파일을 따로 빼도 좋을 듯 합니다! |
||
| { | ||
| validator: Validator.validCarList, | ||
| } | ||
| ); | ||
| // TODO: 쉼표로 구분된 자동자 이름를 배열로 변환해주는 유틸함수 작성 | ||
| this.#cars = value.split(',').map((name) => new Car(name)); | ||
| } | ||
|
|
||
| async setCount() { | ||
| const input = new Input(); | ||
|
|
||
| const value = await input.readLine('시도할 횟수는 몇 회인가요?\n', { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Input을 별도로 분리하고 예외처리를 적용한 로직은 너무 깔끔하지만 하나가 아니라 확인해야하는 예외케이스가 있을 수 있으니, 아니면 "자동차 이름 Validator" 검증함수를 만들고 그 안에서, Validator.validCarList를 호출하도록 수정하면 새로운 검증로직을 추가할 때 더 용이하게 확장할 수 있을 것 같아요 어떻게 생각하시나요?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
저도 검증 로직이 더 많아지면 어떻게 처리하면 좋을까 고민하다가 validator 옵션으로 배열을 넘겨주는 걸 생각해봤어요! // use case
const value = await input.readLine(
'경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n',
{
validators: [Validator.validCarList, Validator.otherValidator, Validator.anotherValidator]
}
);
// Input.js
class Input {
async readLine(query, { validators }) {
try {
const input = await Console.readLine(query);
if(Array.isArray(validators)) {
validators.forEach(validator => validator(input));
}
return input;
} catch(error) {
throw error;
}
}
}이런식으로
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
"자동차 이름 Validator" 검증 함수를 만들자는 말이 어떤 의미인지 수도코드로 작성해주실 수 있어요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
1주차에 작성했던 코드 방식인데, "자동차 이름을 입력받을 때 수행하는 검증함수" 라는 별도의 함수를만들고, |
||
| validator: Validator.rangeOverZero, | ||
| }); | ||
|
|
||
| this.#count = Number(value); | ||
| } | ||
|
|
||
| showWinner() { | ||
|
ojj1123 marked this conversation as resolved.
|
||
| let maximumPathLength = 0; | ||
| this.#cars.forEach((car) => { | ||
| const path = car.getPath(); | ||
| maximumPathLength = Math.max(maximumPathLength, path.length); | ||
| }); | ||
|
|
||
| const winners = this.#cars | ||
| .filter((car) => { | ||
|
ojj1123 marked this conversation as resolved.
|
||
| const path = car.getPath(); | ||
| return maximumPathLength === path.length; | ||
| }) | ||
| .map((car) => car.getName()); | ||
|
|
||
| Console.print(`최종 우승자 : ${winners.join(', ')}`); | ||
| } | ||
| } | ||
|
|
||
| export default Game; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { Console } from '@woowacourse/mission-utils'; | ||
|
|
||
| class Input { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 따로 Input 부분을 빼서 간결하게 예외 처리한거 좋네요!! 저도 다음에 차용해봐야겠습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 와 입력값을 따로 검증할 생각을 못했네요 ..! 👍🏻
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용자 입력 검증은 input 과 따로 분리해 input 외부에서 검증 로직을 주입할 수 있도록 설계 했습니다 :) 아무래도 사용자 입력 검증은 복잡해질 수 있어 관심사 분리를 한거죠. react-hook-form 이라는 라이브러리에서 그 아이디어를 좀 따왔습니다 |
||
| async readLine(query, { validator }) { | ||
| try { | ||
| const input = await Console.readLineAsync(query); | ||
|
|
||
| if (typeof validator === 'function') { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 매개변수로 받은 값의 타입을 체크하는 이유가 궁금합니다!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
그리고 나중에 검증 로직이 복잡해지면 함수 뿐만 아니라 다른 값이 들어갈 수도 있어서 저렇게 해보았습니다! 만약 타입스크립트를 쓴다면 타입 좁히기(type narrow) 효과도 볼 수 있어요! 아래 코드처럼 // use case
// 여러개의 검증 함수를 배열로 주입
const complexValue = await input.readLine('복잡한 값\n', {
validator: [Validator.valid1, Validator.valid2, Validator.valid3],
});
// 함수를 통해 검증 로직 주입
const number = await input.readLine('시도할 횟수는 몇 회인가요?\n', {
validator: Validator.rangeOverZero,
});
// 객체를 통해 검증 로직 주입
const email = await input.readLine('이메일\n', {
validator: { pattern: 'email regex' },
});
// Input.ts
class Input {
async readLine(query, { validator }) {
try {
const input = await Console.readLineAsync(query);
// 타입 좁히기 효과 => 개발 생산성 올라감
if (Array.isArray(validator)) {
validator.forEach((valid) => valid(input));
} else if (typeof validator === 'function') {
validator(input);
} else {
// another validator logic
}
return input;
} catch (error) {
throw error;
}
}
}There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Input을 분리하고 매개변수로 validator를 받아오는 구조가 좋네요! 혹시 여기서 validator가 function이 아니면 그냥 input을 return하는 것 같은데 의도하신 부분일까요? |
||
| validator(input); | ||
| } | ||
|
|
||
| return input; | ||
| } catch (error) { | ||
| throw error; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저희 미션에서는 에러를 throw 하기만 하고 따로 처리를 해 주지 않고 있는데, 혹시 try-catch 구문을 사용하신 이유가 있으실까요? |
||
| } | ||
| } | ||
| } | ||
|
|
||
| export default Input; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| function rangeOverZero(input) { | ||
| if (!Number.isSafeInteger(Number(input))) { | ||
| throw new Error('[ERROR] 올바른 정수를 입력해주세요'); | ||
| } | ||
|
Comment on lines
+1
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isSafeInteger라는 함수가 있군요! JavaScript 내장 함수를 잘 사용하신 것 같습니다. 다만 rangeOverZero라는 이름은 0 이상인지만 확인해줄 것 같고 정수인지는 확인해줄 것 같지 않다는 느낌이 듭니다. 그리고 현재는 빈 입력이 들어올 때도 0이 들어왔을 때와 같이 작동하는 것 같습니다. 게임 횟수의 상한도 정하면 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 와 isSafeIntegar 이라는 게 있군요! 덕분에 하나 또 알아갑니다! ㅎㅎ |
||
| if (Number(input) < 0) { | ||
| throw new Error('[ERROR] 음이 아닌 정수를 입력해주세요'); | ||
| } | ||
|
Comment on lines
+5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q. 저는 게임 시도 횟수가 0일 경우도 게임 작동이 불가능하지 않을까 생각했었는데 어떻게 생각하시는지 궁금합니다!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이거는 요구사항을 어떻게 분석했느냐에 따라 다를거 같아요 |
||
| } | ||
|
|
||
| function validLength(input, { minLength = 1, maxLength }) { | ||
| return minLength <= input.length && input.length <= maxLength; | ||
| } | ||
|
|
||
| function validCarList(input) { | ||
| const list = input.split(','); | ||
| if (list.every((item) => validLength(item, { maxLength: 5 }))) { | ||
| return; | ||
| } | ||
| throw new Error('[ERROR] 자동차 이름을 1글자 이상 5글자 이하로 작성해주세요'); | ||
| } | ||
|
Comment on lines
+14
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이름을 받을 때 앞뒤 공백을 자르면 좋을 것 같습니다. 저같은 경우에는 입력할 때 습관적으로 쉼표 뒤에 공백을 넣는데, 그러면 이름이 5글자인 경우에도 에러가 발생합니다. 입력으로 공백만 들어온 경우나 중복된 이름이 있는 경우 등도 추가하면 좋을 것 같습니다. |
||
|
|
||
| const Validator = { | ||
| rangeOverZero, | ||
| validLength, | ||
| validCarList, | ||
| }; | ||
|
|
||
| export default Validator; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './Validator.js'; |
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.
에러 처리 뿐만 아니라 바른 입력이 들어왔을 때 pass되는 것도 테스트에 추가하면 좋을 것 같습니다.