-
Notifications
You must be signed in to change notification settings - Fork 903
[자동차 경주] 김현수 미션 제출합니다. #912
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?
Conversation
| if (cars.isEmpty()) { | ||
| throw new IllegalArgumentException("[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.
만약 위의 반복문에서 오류를 throw를 한다고 하면 해당 코드는 필요 없는 것 같습니다.
| package racingcar; | ||
|
|
||
| public interface NumberGenerator { | ||
| int next0to9(); |
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.
0~9 사이의 난수를 발생시킨다는 함수명 같은데 이를 굳이 인터페이스로 만든 이유가 궁금합니다.
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.
그와 별개로 이를 주입해서, 난수를 제거하고 테스트 할땐 용이할 것 같습니다.
|
|
||
| public static List<String> of(final List<Car> cars) { | ||
| if (cars == null || cars.isEmpty()) { | ||
| throw new IllegalArgumentException("[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.
입력받은 시점에서, 자동차가 생성되어서 중복되는 코드인 것 같습니다.
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와 Winners 양쪽에서 일어나네요....
입력 시점에서만 검증하고, 도메인에서는 불필요한 중복을 제거하도록 개선해보겠습니다
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { |
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.
다음 미션에서는 이런 데이터 일관성 부분도 좀 더 신경 써보겠습니다!
| private static RacingGame createGame(List<Car> cars) { | ||
| MovePolicy policy = new DefaultMovePolicy(); | ||
| NumberGenerator generator = new MissionUtilsGenerator(); | ||
| return new RacingGame(cars, policy, generator); | ||
| } | ||
|
|
||
| private static void play(RacingGame game, List<Car> cars, int attempts) { | ||
| for (int i = 0; i < attempts; i++) { | ||
| game.step(); | ||
| OutputView.printRound(cars); | ||
| } | ||
| } | ||
|
|
||
| private static void printWinners(List<Car> cars) { | ||
| List<String> winners = Winners.of(cars); | ||
| OutputView.printWinners(winners); | ||
| } |
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.
레이싱 카 게임 실행 흐름을 제어하는 클래스를 하나 만들고, main에서는 단순 진입점만 제공하는 것은 어떨까요?
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.
다른 분들 코드를 보면서 Controller로 흐름을 제어하는 구조가 훨씬 명확하다는 걸 느꼈습니다...
말씀해주신 대로 이번 주 미션에서는 이런 구조적인 부분을 참고해서 적용해보겠습니다!
|
기능 목록에 맞게 구현하고 테스트 코드를 잘 작성하신 것 같습니다. 2주차 고생 많으셨습니다. |
No description provided.