-
Notifications
You must be signed in to change notification settings - Fork 209
[자동차 경주] 김지훈 미션 제출합니다. #21
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?
[자동차 경주] 김지훈 미션 제출합니다. #21
Conversation
- 자동차 이름 및 시도 횟수 관련 검증 에러 메시지를 상수로 선언 - 공통 접두사 [ERROR]를 ERROR_PREFIX로 분리해 일관성 유지
- 자동차 이름 및 시도 횟수 입력을 위한 안내 메시지 추가 - 실행 결과 및 우승자 표시용 출력 문자열 정의
- 자동차의 이름과 이동 거리를 관리하는 Car 클래스 정의 - move 메서드를 사용하여 이동 거리 증가 - 이름과 이동 거리 접근을 위한 getter 메서드 정의
- 입력받은 자동차 이름 문자열을 쉼표 기준으로 분리 - 각 이름을 trim 처리 후 Car 인스턴스로 변환 - 생성된 Car 인스턴스 배열을 반환
- 0부터 9 사이의 난수를 생성하여 4 이상일 경우 자동차를 전진 - 각 자동차 인스턴스의 move 메서드를 호출하여 거리 증가 처리
- RaceCarFactory를 통해 초기 자동차 객체 생성 - RaceStepProcessor를 호출해 매 시도마다 전진 여부 결정 - 현재 위치 조회, 남은 시도 여부 판단, 최종 우승자 계산 기능 포함 - 한 번의 실행 단계 결과를 반환하는 executeStep 메서드 구현
- Console.readLineAsync를 사용해 프롬프트 메시지 출력 및 입력 대기
- 실행 결과 헤더, 각 단계별 경주 결과, 최종 우승자 출력 기능 구현 - Console.print를 이용해 단계별 결과와 구분 공백 라인 처리
- 입력, 출력, 시뮬레이션 로직을 통합 관리하는 Controller 구현 - 사용자의 자동차 이름과 시도 횟수를 입력받아 경주 실행 - 각 단계별 결과와 최종 우승자를 출력하는 전체 실행 흐름 구성
RaceCarFactory와 RaceStepProcessor는 인스턴스 상태를 가지지 않고, 단순히 입력값을 처리하는 정적 메서드만 포함하고 있었습니다. 이에 따라 클래스로 유지할 필요가 없어, 각각의 로직을 순수 함수(createCars, runOneStep) 형태로 분리하여 utils로 이동합니다. - createCars, runOneStep을 유틸 함수 형태로 변경 - RaceSimulator에서 해당 함수 직접 호출하여 구조 단순화
- NAME_CONTAINS_COMMA를 NAME_LIST_FORMAT_INVALID로 상수명 변경 - 오류 메시지를 쉼표로 구분된 올바른 형식을 입력하도록 구체화
- 자동차 이름이 비어있거나 5자를 초과하거나 중복된 경우 예외 발생 - 시도 횟수가 숫자가 아니거나 0 이하인 경우 예외 발생
- Car 인스턴스 생성 시 validateNameLength 호출
- 자동차 이름 입력 시 validateNameListFormat 호출 - 시도 횟수 입력 시 validateNumericValue 호출 - 컨트롤러 수준에서 입력 검증 책임 수행
- 생성자에서 validatePositiveNumber 호출
- createCars 내부에서 validateNameDuplication 호출
- 빈 문자열 입력 시 예외 추가 - validatePositiveNumber를 validatePositiveInteger로 변경 - 정수 여부 검사 로직 및 관련 에러 메시지 추가
- carNames.split(,) 후 각 이름의 앞뒤 공백 제거 - validateNameDuplication 적용 전 공백 제거된 리스트 사용
- Car 클래스 내부 필드 #distance를 #position으로 수정 - 관련 메서드 getDistance를 getPosition으로 수정 - RaceSimulator에서 getDistance에서 getPosition으로 업데이트
- readString 호출 시 return await 대신 바로 Promise를 반환하도록 수정
- 자동차 이동 조건을 테스트하기 위한 상수 추가
- App 실행 흐름을 사용자 입력 및 랜덤값을 모킹하여 검증 - 단독/공동 우승 시나리오, 이름 공백 제거, 유효성 검사 케이스 포함
- App 실행 흐름의 중복을 runApp으로 통합 - 로그 검증 로직을 expectLogsContain으로 분리 - 정상 동작과 예외 케이스 구성을 분리
- 경주 단계 출력 로직을 #printRaceProgress로 분리 - 최종 결과 출력을 #printFinalResults로 분리 - #runRace는 실행 흐름을 관리하는 역할만 수행하도록 단순화
- 테스트 메서드명을 getDistance에서 getPosition으로 수정
- RACE_CONFIG 상수 파일을 추가하여 경주 규칙 관련 상수를 중앙 관리 - raceUtils, validators에서 상수 정의 제거 및 RACE_CONFIG를 참조하도록 수정
iftype
left a comment
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.
커밋 메세지를 읽다보니 e77c073 에서 CarFactory를 제거한게 인상 깊었습니다.
남들은 코드를 한 줄이라도 더 쓰려고 할 때, 오히려 필요없는 부분을 줄여나간 것이 가독성 높은 코드로 이어진 것 같습니다
2주차 과제의 정답지를 본 기분입니다 읽기 쉬우니 많은 사람들이 봤으면 좋겠네요
| #position; | ||
|
|
||
| constructor(name) { | ||
| validateNameLength(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.
validator를 생성자 안에 넣어 안전한 상태를 유지할 수도 있다는 걸 배워갑니다..
| #runRace(carNames, attemptCount) { | ||
| const simulator = new RaceSimulator(carNames, attemptCount); | ||
| this.#printRaceProgress(simulator); | ||
| this.#printFinalResults(simulator); | ||
| } |
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.
start 메서드에서 게임 진행과 출력 로직을 분리하여 runRace 메서드로 나눈게 읽기 편했습니다
| } | ||
|
|
||
| executeStep() { | ||
| this.#remainingAttempts -= 1; |
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.
시도할 횟수를 프로퍼티로 가지고있는 모습이 인상깊었습니다
| if (attemptInput === '') { | ||
| throw new Error(ERROR_MESSAGES.ATTEMPT_COUNT_EMPTY); | ||
| } | ||
| if (isNaN(attemptInput)) { |
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.
전역 isNaN보다 Number.isNaN 을 사용하는게 좋아보입니다
29.1 airbnb
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.
아 변환값을 같이 고려하신거였군요!
제 견문이 짧았네요
| export const runOneStep = (cars) => { | ||
| cars.forEach((car) => { | ||
| if ( | ||
| Random.pickNumberInRange(RACE_CONFIG.MIN_RANDOM_VALUE, RACE_CONFIG.MAX_RANDOM_VALUE) >= | ||
| RACE_CONFIG.MOVE_THRESHOLD | ||
| ) { | ||
| car.move(); | ||
| } | ||
| }); | ||
| }; |
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.
Random.pickNumberInRange(RACE_CONFIG.MIN_RANDOM_VALUE, RACE_CONFIG.MAX_RANDOM_VALUE) >= RACE_CONFIG.MOVE_THRESHOLD차 리스트의 이동여부를 결정하는 전략과 이동시키는 책임이 같이 있어 보입니다
해당 전략을 분리하는 건 어떻게 생각하시나요?
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.
에러 메세지가 굉장히 이해하기 쉽게 선언 되어 있는게 인상 깊습니다. 따로 참고하고 계신 자료가 있으실까요? 아니면 고민해서 작성하시는 편이실까요? 저도 이렇게 네이밍 하고 싶어 여쭤봅니다...
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.#inputView = new InputView(); | ||
| this.#outputView = new OutputView(); | ||
| } |
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.
생성자를 통해 바로 값을 입력 받아서 넣는게 처리가 굉장히 깔끔한 것 같습니다. 저는 별도로 입력을 받은 후에 클래스를 선언하여 사용했는데 너무 좋은 방법 배워갑니다!
|
에러메세지 뿐만 아니라 대부분의 상수/변수명이 이름만으로도 이해하기 쉽게 선언 되어 있는 것 같습니다. 커밋 메세지도 굉장히 깔끔해서 군더더기 없었던 것 같습니다! 배울 점 가득 얻고 갑니다. 코드 정말 잘 봤습니다! 3주차 미션도 힘내세요! |
holdn2
left a comment
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주차도 고생 많으셨습니다! 저도 클래스로 객체를 만들어서 과제를 해보려고 공부 중인데 부족한 부분이 많은 것 같습니다. 지훈님 코드 보면서 많이 배워갑니다~
| @@ -0,0 +1,14 @@ | |||
| const ERROR_PREFIX = '[ERROR]'; | |||
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.
저도 다음부터는 prefix로 만들어서 사용해봐야겠어요!
| @@ -0,0 +1,8 @@ | |||
| const RACE_CONFIG = Object.freeze({ | |||
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.
하드 코딩 대신 상수로 잘 정의하시는 것 같아요!
| import { validateNameDuplication } from '../../utils/validators.js'; | ||
| import RACE_CONFIG from '../../constants/raceConfig.js'; | ||
|
|
||
| export const createCars = (carNames) => { |
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.
validateNameDuplication을 함수 내부에서 호출한 이유가 있으실까요?
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.
raceUtils도 모델의 일부로 보고, 배열 내 이름 중복 검증은 모델의 책임에 포함된다고 생각했습니다!
| export const runOneStep = (cars) => { | ||
| cars.forEach((car) => { | ||
| if ( | ||
| Random.pickNumberInRange(RACE_CONFIG.MIN_RANDOM_VALUE, RACE_CONFIG.MAX_RANDOM_VALUE) >= |
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.
Random.pickNumberInRange(0, 9) >= 4이 조건의 매직 넘버를 상수로 치환하면서 가독성이 더 떨어져 보이는 것 같아요 😅
논리적 복잡도는 동일하지만 한눈에 읽기 좋게 변수로 분리하는 방향도 고려해 보겠습니다!
| } | ||
|
|
||
| export function validateNameListFormat(rawNames) { | ||
| const VALID_NAME_LIST_PATTERN = /^[^,]+(,[^,]+)*$/; |
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.
정규식도 constant에 정의해두면 좋을 것 같아요!
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.
이번 과제에서는 정규식을 사용하는 부분이 해당 검증식밖에 없어서, constants로 분리하면 응집도가 떨어질 것 같다고 판단했습니다!
모든 상수를 constants로 분리해 관리하시는 편인가요? 항상 상수를 분리할 기준을 어디까지 둘지 고민되네요 🤔
| } | ||
| } | ||
|
|
||
| export function validatePositiveInteger(number) { |
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.
validateNumericValue에서 한번에 양의 정수처리까지 하는 것도 좋아보이는데 어떻게 생각하시나요?
에러 메시지를 둘로 나누지 않아도 될 것 같아요!
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.
형식 검증 단계에서는 입력이 숫자 형태로 유효한지만 확인하고, 양수가 아닌 값에 대한 예외는 비즈니스 규칙으로 보고 validatePositiveInteger에서 처리하도록 분리했습니다. 그래서 형식 검증에서는 음수나 0을 허용하는 것이 자연스럽다고 판단했습니다! 저도 구현하면서 많이 고민했던 부분이라 다양한 의견을 들어보고 싶네요~
| @@ -0,0 +1,10 @@ | |||
| import { Console } from '@woowacourse/mission-utils'; | |||
|
|
|||
| class InputView { | |||
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.
InputView와 OutputView에는 내부에서 관리할 상태가 없는데 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.
InputView와 OutputView는 내부 상태를 따로 관리하진 않지만, MVC 구조를 명확하게 드러내고자 클래스 형태로 유지했습니다!
특히 OutputView의 경우 출력 관련 메서드들이 하나의 책임 아래 묶여 있어서, Controller에서 this.#outputView.printWinners()처럼 호출하는 흐름이 구조적으로 자연스럽다고 생각했습니다.
저도 불필요한 구조를 최소화하려고 하는 편인데, View 계층을 명확히 표현하기 위한 선택으로 봐주시면 좋을 것 같습니다. 물론 상태가 전혀 없기 때문에 단순 함수로 구현해도 타당한 접근이라고 생각합니다 🙂
| Console.print(PROMPT_MESSAGES.RESULT_HEADER); | ||
| } | ||
|
|
||
| printStepResult(result) { |
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와 distance를 나눠서 파라미터로 넣어줬는데 안쪽에서 처리하셨네요!
gustn99
left a comment
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주차 코드도 역시 깔끔하네요! 많이 배우고 갑니다 고생 많으셨어요~
|
|
||
| #printRaceProgress(simulator) { | ||
| this.#outputView.printResultHeader(); | ||
| while (simulator.hasRemainingAttempts()) { | ||
| const positions = simulator.executeStep(); | ||
| this.#outputView.printStepResult(positions); | ||
| } | ||
| } | ||
|
|
||
| #printFinalResults(simulator) { | ||
| this.#outputView.printWinners(simulator.getRaceWinners()); | ||
| } | ||
|
|
||
| #runRace(carNames, attemptCount) { | ||
| const simulator = new RaceSimulator(carNames, attemptCount); | ||
| this.#printRaceProgress(simulator); | ||
| this.#printFinalResults(simulator); | ||
| } |
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.
개인적으로는 runRace가 다른 두 메서드보다 먼저 선언되는 게 흐름 상 읽기 좋을 것 같습니당
| export const createCars = (carNames) => { | ||
| const carNameList = carNames.split(',').map((name) => name.trim()); | ||
| validateNameDuplication(carNameList); | ||
| return carNameList.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.
이름 자체에는 문제가 없는 것이기 때문에 length와 duplication 검증 로직을 분리하신 걸까요?
다른 코드를 리뷰하면서도 동일하게 중복 검사를 createCars 내부에 작성한 걸 봤었는데, 흥미로운 구분인 것 같습니다.
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.
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.
앗, 조금 더 명확하게 설명하면, Car에서 수행하는 이름 검증(length 조건 검사)과 Cars에서 수행하는 이름 검증(중복 검사)를 구분하신 이유에 대한 질문이었습니다!
저는 단순히 중복 검사도 이름 입력에 대한 검증으로 판단 -> 하나의 name validator에서 중복 검사와 length 조건 검사를 같이 진행했었거든요. 하지만 그렇게 작성하면서도 둘 사이에 미묘한 구분이 있다는 생각이 들었었습니다. 중복 검사는 전체 배열이 필요하고, length 조건 검사는 개별 요소들만 필요해서, 하나는 배열 자체에 대한 검증을 수행하고, 나머지는 배열을 순회하면서 검증을 수행해야 했으니까요.
지금 생각하기로는 Car와 Cars를 구분하는 순간 구현하신 방향이 자연스러운 흐름인 것 같네요! 중복 이름이라는 것 자체가 개별 Car 객체로 존재할 때에는 애초에 발생하지 않았다가, Cars로 모이면서 발생하는 상황이니까요. 또 하나 배워갑니다 👍
| test('각 자동차의 전진 여부는 독립적으로 결정된다', () => { | ||
| // Arrange | ||
| Random.pickNumberInRange | ||
| .mockReturnValueOnce(TEST_CONFIG.MOVING_FORWARD) | ||
| .mockReturnValueOnce(TEST_CONFIG.STOP); | ||
| const cars = createCars('pobi,woni'); | ||
|
|
||
| // Act | ||
| runOneStep(cars); | ||
|
|
||
| // Assert | ||
| expect(cars[0].getPosition()).toBe(1); | ||
| expect(cars[1].getPosition()).toBe(0); | ||
| }); |
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.
동일한 조건에서 모든 자동차가 동시에 움직이거나 멈추는 것이 아니라, 각 자동차의 전진 여부가 랜덤 값에 따라 개별적으로 결정되는 로직을 검증하는 것이 목적이었습니다! 지금 보니 테스트 설명을 이해하기 어렵게 작성했나 싶기도 하네요 😅
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.
이해했습니다! runOneStep 함수의 핵심 책임이 여러 자동차를 각각 독립적으로 처리하는 것이라고 생각했습니다. forEach 내부에서 각 차마다 랜덤 값을 새로 받아야 하는데, 이 부분이 제대로 동작하는지 검증하지 않으면 함수의 핵심 동작을 보장할 수 없다고 봤습니다.
| test('스텝 실행 시 남은 시도 횟수가 감소한다', () => { | ||
| // Arrange | ||
| Random.pickNumberInRange.mockReturnValue(TEST_CONFIG.MOVING_FORWARD); | ||
| const simulator = new RaceSimulator('pobi,woni', 2); | ||
|
|
||
| // Assert: 초기 상태 | ||
| expect(simulator.hasRemainingAttempts()).toBe(true); | ||
|
|
||
| // Act & Assert: 1회 실행 후 | ||
| simulator.executeStep(); | ||
| expect(simulator.hasRemainingAttempts()).toBe(true); | ||
|
|
||
| // Act & Assert: 2회 실행 후 | ||
| simulator.executeStep(); | ||
| expect(simulator.hasRemainingAttempts()).toBe(false); | ||
| }); |
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.#remainingAttempts -= 1이 실행되는가를 테스트하는 코드이지만, hasRemaningAttempts가 실패하면 이 테스트도 실패하기 때문에 그 경계가 애매하다는 생각이 듭니다! 어떻게 생각하시나요?
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.#remainingAttempts -= 1)을 확인하는 거였는데,
실제로는 hasRemainingAttempts()의 결과로 간접적으로 검증하고 있어서 경계가 조금 애매한 것 같네요.
단위 테스트에서는 public 메서드 기준으로 확인하는 게 자연스럽다고 생각해서 지금처럼 외부 동작을 기준으로 테스트하는 것도 괜찮다고 생각했는데, 만약 내부 동작 자체를 명확히 확인해야 한다면, private 필드를 직접 노출하지 않는 선에서 getter 메서드를 두는 방법도 있을 것 같습니다. 좋은 질문 감사합니다 👍
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.
woowacourse-precourse/javascript-lotto-8#167 (comment)
3주차 미션 코드 리뷰 진행하면서 이 리뷰가 생각이 났습니다 😅 제가 위에서 생각했던 방법(테스트를 위해 내부 상태를 노출)은 안티 패턴인 경우가 많다고 하니 주의해야 할 것 같아요
|
너무 깔끔하네요.. 저도 3주차 때 MVC 패턴으로 구현한다면, 이렇게만 하고 싶습니다... 리뷰가 아닌 저는 보고 배우기만 했네요 ㅋㅋㅋ 감사합니다!! |
JetProc
left a comment
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 readString(promptMessage) { | ||
| const input = await Console.readLineAsync(promptMessage); | ||
| return input; | ||
| } |
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 run() { | ||
| const controller = new RaceController(); | ||
| await controller.start(); | ||
| } | ||
| } |
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.
App에서 의도적으로 try...catch를 빼신 건지 궁금합니다!
물론 굳이 쓰지 않아도 throw error에서 에러를 던지며 프로그램이 종료되긴 하지만
그 종료된 이후의 흐름을 제어하기 위해선 error throw시 상위 코드에서 catch 문이 필수라고 생각해서요!
(과제에선 에러 메시지를 던지라고만 하기 때문에 흐름 제어를 할 필요 없긴 합니다..!)
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.
1주차 미션에서는 테스트를 통과하기 위해 컨트롤러에서 try-catch로 예외를 잡은 뒤, Console.print로 에러 메시지를 출력하고 다시 예외를 그대로 throw하는 구조로 작성했었습니다!
다만 이 구조가 조금 어색하다고 느껴져서, 2주차 미션에서는 메시지 출력 후 예외를 다시 던지는 부분은 제거했습니다. 말씀해 주신 것처럼 별도의 에러 상황에 대한 예외 처리가 필요하지 않아서 상위 레벨에서 try-catch를 사용하지 않았습니다.
| class App { | ||
| async run() {} | ||
| async run() { | ||
| const controller = new RaceController(); |
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에서 하면 어떨까 생각했는데
어떻게 생각하시는지 궁금합니다!
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.
저는 run()이 한 번 실행되고 종료되는 단일 실행 구조라 RaceController를 재사용하거나 필드로 보관할 필요는 없다고 생각했습니다.
constructor에서 생성하는 방식은 App 인스턴스가 여러 번 재사용되거나, 컨트롤러를 여러 메서드에서 공유해야 하거나, 의존성을 명시적으로 주입받아야 할 때 적합할 것 같습니다.
하지만 현재 구조에서는 App이 단순히 진입점 역할만 수행하고, 컨트롤러를 한 번만 사용하고 종료하기 때문에 지역 변수로 충분하다고 생각했습니다! 오히려 필드로 두면 왜 이게 필드인가?라는 의문이 생길 수 있을 것 같아요.
RaceController 내부에서도 RaceSimulator를 runRace() 메서드 안에서 생성하는 식으로, 필요한 시점에 만드는 흐름을 전체적으로 맞췄습니다~
| validatePositiveInteger(attemptCount); | ||
| this.#remainingAttempts = attemptCount; |
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.
양수 검증은 처음 입력받을 때 검증하지 않고 RaceSimulator 인스턴스 생성 후에 검증하는지 궁금합니다 !
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.
양수 검증은 비즈니스 규칙이라고 생각했고, 그렇다면 모델에서 검증하는 게 더 자연스럽다고 생각했습니다!
| return this.#cars | ||
| .filter((car) => car.getPosition() === maxDistance) | ||
| .map((car) => car.getName()); |
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.
고민해봤는데, 저는 현재 구조가 최대 거리 도달한 차들의 이름 반환이라는 단일 의도를 표현하는 하나의 흐름이라고 생각해요! filter와 map이 연결되어 있지만 각 단계의 역할이 명확하고, 우승자를 찾는다는 하나의 책임을 수행하는 흐름으로 읽힌다고 생각합니다.
다만 가독성 측면에서 메서드 체이닝이 더 길어지거나 복잡해진다면 분리하는 게 나을 수도 있을 것 같아요!

🧑💻 구현 내용
문자열로 입력받은 자동차 이름들과 시도 횟수로 경주를 시뮬레이션하고 우승자를 출력하는 프로그램을 구현했습니다.
💎 주요 기능
🧩 프로그램 구조
MVC 패턴을 기반으로 구조화했으며, 각 계층의 역할을 분리했습니다.
Car)RaceSimulator,raceUtils)InputView,OutputView)RaceController)각 레이어 별 자세한 기능 명세는 README.md를 참고해 주세요.
자동차 이름 검증
Car생성자 →validateNameLength)RaceController.start→validateNameListFormat)createCars→validateNameDuplication)시도 횟수 검증
RaceController.start→validateNumericValue)RaceSimulator생성자 →validatePositiveInteger)발생한 모든 예외는 각 계층에서 즉시 throw되며, 상위로 전파되어 프로그램이 종료됩니다.
🧪 테스트
단위 테스트
1)
Car도메인move()호출 시 위치 1 증가getName(),getPosition()메서드 동작 확인2)
RaceSimulatorexecuteStep()실행 후 자동차 이름과 위치 반환hasRemainingAttempts()메서드 동작 확인3)
raceUtils유틸리티통합 테스트
1) 정상 동작 시나리오
2) 유효성 검사 예외 케이스
💬 TO REVIEWERS
테스트 패턴 선택 이유
리뷰 포인트
다음 부분들도 함께 봐주시면 감사하겠습니다!