Skip to content

Conversation

@YBSeok
Copy link

@YBSeok YBSeok commented Oct 27, 2025

No description provided.

Copy link

@goodsmell goodsmell left a comment

Choose a reason for hiding this comment

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

코드 잘 읽었습니다!
2주차 수고 많으셨어요ㅎㅎ

Choose a reason for hiding this comment

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

에러 메시지와 기준값(이름 최대 길이 5, 시도 횟수 최소 1)을 하드코딩 하지 않고 상수로 분리해두면,
나중에 요구사항 바뀌었을 때 검증 로직이 아니라 상수만 바꿔도 돼서 유지보수성이 올라갈 것 같습니다!
저도 기준값을 하드코딩 해버려서 후회중입니다🥲

Comment on lines +15 to +33
async getCarNamesInput() {
const input = await Console.readLineAsync(
"경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n"
);

const carNames = input.split(",").map((name) => name.trim());
ErrorHandler.validateCarNames(carNames);
return carNames;
}

async getTryCountInput() {
const input = await Console.readLineAsync("시도할 횟수는 몇 회인가요?\n");

// 1. parseInt(input) 대신 원본 'input' (문자열)을 검사하도록 변경
// 2. ErrorHandler가 검증된 '숫자'를 반환하도록 함
const tryCount = ErrorHandler.validateTryCount(input);

return tryCount;
}

Choose a reason for hiding this comment

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

이 부분이 RacingGame 내부에도 정의해놓으신 것같은데 중복으로 입력하신 의도가 있으신가요?!?
입력 책임을 App에서만 가지게 하고, RacingGame은 도메인 로직만 갖도록 정리하면 더 명확해질 것 같습니다!

Choose a reason for hiding this comment

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

에러 처리 하신 것을 기록해 두는 자세가 참 좋은 것 같아요!
배워갑니다 ㅎㅎ

Choose a reason for hiding this comment

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

테스트 의도별로 파일을 분리해서 작성하는 것도 좋을 것 같습니다 !

Choose a reason for hiding this comment

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

RacingGame이 경주 진행, 현재 위치 출력,우승자 출력까지 모두 가지고 있어서 책임이 조금 넓어 보입니다!
출력 부분은 view 쪽으로 분리하고 RacingGame은 경주 결과 데이터만 반환하도록 하면 테스트하기도 쉬워지고 구조가 더 명확해질 것 같습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants