-
Notifications
You must be signed in to change notification settings - Fork 1.2k
4단계 - 자동차 경주(우승자) #6213
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: username0w
Are you sure you want to change the base?
4단계 - 자동차 경주(우승자) #6213
Conversation
- 기본 생성 테스트 입력값 수정
javajigi
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.
4단계 미션 구현 잘 했네요. 👍
객체 설계 역량이 충분해 보여 다음 미션에서 진행하려했던 '규칙 3: 모든 원시값과 문자열을 포장한다.' 원칙을 적용하도록 피드백 남겨봤어요.
이 미션의 여러 곳에 이 원칙을 적용할 수 있는 부분이 있는데요.
적용할 수 있는 모든 부분을 찾아 적용해 보면 좋겠습니다.
적용할 때 가능하면 TDD로 클래스를 분리해 보면 좋겠습니다.
객체를 분리한 후. 객체를 좀 더 능동적인 존재로 바라보면서 더 많은 책임을 가지도록 구현해 보면 좋겠어요.
이렇게 구현했을 때 tdd 관점에서 로직을 테스트하는 것이 얼마나 쉬워지는지도 경험해 봤으면 합니다.
| @@ -1,11 +1,24 @@ | |||
| package racingcar; | |||
|
|
|||
| public class 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.
👍
src/main/java/racingcar/Car.java
Outdated
| private static final int MOVE_THRESHOLD = 4; | ||
|
|
||
| private final String name; | ||
| private int position = 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.
로또에서 진행해도 되지만 객체 설계 역량이 충분해 보여 지금 단계에서 도전해 보면 좋을 것 같아 제안해 봅니다.
'규칙 3: 모든 원시값과 문자열을 포장한다.' 원칙을 지키도록 리팩터링해보세요.
이 원칙을 지켰을 때의 잇점에 대해 생각해 보면 좋겠습니다.
리팩터링할 때 TDD 원칙을 지키보면 도전해 보세요.
src/main/java/racingcar/Car.java
Outdated
| private int position = 0; | ||
|
|
||
| public Car(String name) { | ||
| validateName(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.
👍
| } | ||
|
|
||
| private static void addWinnerIfMaxPosition(List<String> winners, Car car, int maxPos) { | ||
| if (car.position() == maxPos) { |
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.
객체의 상태 값을 꺼내지 말고 객체에 메시지를 보내라.
OOP적 사고에서 정말 중요한 원칙이니 지키기 위해 노력해 보면 어떨까요?
| private int getMaxPosition(List<Car> cars) { | ||
| int maxPosition = Integer.MIN_VALUE; | ||
| for (Car car : cars) { | ||
| maxPosition = Math.max(car.position(), maxPosition); |
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.
객체의 상태 값을 꺼내지 말고 객체에 메시지를 보내라.
OOP적 사고에서 정말 중요한 원칙이니 지키기 위해 노력해 보면 어떨까요?
| @Test | ||
| void getSingleWinner() { | ||
| RaceGame raceGame = new RaceGame(List.of("car1", "car2"), 3); | ||
| raceGame.cars().getFirst().moveIfPossible(4); |
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.
우승자를 구하기 위한 상태값을 초기화하기 위해 moveIfPossible()을 테스트에서 매번 실행해야 하는데요.
이 같은 단점을 보완하는 방법을 없을까요?
자동차 목록을 생성할 때 경주가 끝났을 때의 상태 값을 가지도록 초기화할 수는 없을까요?
- RaceGame에 테스트 편의를 위한 생성자 추가
javajigi
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.
피드백 반영 잘 했네요. 👍
바로 merge하려다 지금 단계에서 연습해 보는 것이 좋을 것 같아 피드백 몇 개 남겼어요.
아직까지 객체의 값을 꺼내 로직을 구현하는 곳이 몇몇 곳 보이는데요.
가능하면 메시지를 보내는 방식으로 구현해 볼 것을 추천해요.
GameCount도 값을 포장했으면 더 많은 로직 처리를 담당해도 좋을 것 같아요.
| private Position position; | ||
|
|
||
| public int position() { | ||
| public Car(CarName name, Position position) { |
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 void move() { | ||
| position++; | ||
| public boolean isAtSamePositionAs(Car maxCar) { |
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 boolean isAtSamePositionAs(Car maxCar) { | |
| public boolean isAtSamePositionAs(Position maxPosition) { |
Car가 아닌 Position을 전달해 비교하는 것은 어떨까?
| return this.position.equals(maxCar.position); | ||
| } | ||
|
|
||
| public boolean isAheadOf(Car 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가 아닌 Position을 전달하는 것은 어떨까?
| } | ||
|
|
||
| public boolean isAheadOf(Car car) { | ||
| return this.position.asInt() > car.position.asInt(); |
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.
이 또한 position에서 값을 꺼내는 방식으로 구현
position 객체에 메시지를 보내 구현해 보는 것은 어떨까?
| "3, 5, 5", | ||
| "5, 3, 5", | ||
| "4, 4, 4", | ||
| "0, 0, 0", | ||
| "0, 1, 1", | ||
| "1, 0, 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.
| "3, 5, 5", | |
| "5, 3, 5", | |
| "4, 4, 4", | |
| "0, 0, 0", | |
| "0, 1, 1", | |
| "1, 0, 1" | |
| "3, 5, 5", | |
| "5, 3, 5", | |
| "4, 4, 4" |
이 정도로도 충분하지 않을까?
|
|
||
| private final List<Car> cars; | ||
| private final int gameCount; | ||
| private final GameCount gameCount; |
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.
GameCount로 포장을 했는데 특별히 하는 역할이 없다.
단순히 값만 포장하고 꺼내는 것이 아니라 로직을 위임할 수 있는 부분은 없을까?
안녕하세요.
4단계 - 자동차 경주(우승자) PR 입니다.