4단계 - 자동차 경주(우승자) #6054
4단계 - 자동차 경주(우승자) #6054
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| private void validateName(String name) { | ||
| if (name.length() > MAX_NAME_LENGTH) { | ||
| throw new IllegalArgumentException("자동차 이름은 " + MAX_NAME_LENGTH + "자를 초과할 수 없습니다."); |
| if (randomNumber < 0 || randomNumber > 9) { | ||
| throw new IllegalArgumentException("랜덤 값은 0에서 9 사이여야 합니다."); | ||
| } |
There was a problem hiding this comment.
요 코멘트를 이번 단계에서 드리려했는데 이미 만들어주셨군요 😄
0과 9는 상수로 분리되어도 좋을 것 같아요!
| @@ -6,17 +6,24 @@ | |||
|
|
|||
| public class Racing { | |||
|
|
|||
| List<Car> cars = new ArrayList<>(); | |||
| private final List<Car> cars = new ArrayList<>(); | |||
There was a problem hiding this comment.
List<Car>를 핵심적으로 다루는 객체가 되면서(움직임, 우승자) Racing보다는 "자동차들"을 관리한다는 이름을 잘 전달해주는 네이밍으로 변경해보면 어떨까요?
| private static String[] StringToArray(String carNamesRaw) { | ||
| return carNamesRaw.split(","); | ||
| } |
There was a problem hiding this comment.
","로 분리되어 들어온다는 것은 화면단의 요구사항이므로 domain보다는 view에서 처리되면 좋을 것 같아요 😄
| @@ -30,5 +37,28 @@ public void simulateRace() { | |||
| } | |||
| } | |||
|
|
|||
| public List<String> getMaxPosition() { | |||
There was a problem hiding this comment.
기능상 네이밍이 가장 높은 위치를 가져온다라기보다는 우승자를 찾는다의 네이밍에 맞게 변경되어야겠네요 :)
| int maxPosition = 0; | ||
| for (Car car : cars) { | ||
| maxPosition = findMaxPosition(maxPosition, car); | ||
| } |
There was a problem hiding this comment.
사실상 이 부분이 findMaxPosition에 가깝고, 기존의 findMaxPosition은 compareMaxPosition과 같은 비교하여 더 높은 것을 가져온다에 가까워보이네요!
결과적으로는
int maxPosition = findMaxPosition(...);으로 메서드를 사용할 수 있도록 구성해보면 어떨까요?
| for (Car car : cars) { | ||
| addMaxPositionCar(maxPositionCars, maxPosition, car); | ||
| } | ||
| return maxPositionCars; |
There was a problem hiding this comment.
이부분도 하나의 메서드로써 return calcualteMaxPositionCars(...);와 같이 기능 제공을 하면 메서드를 가독성있게 분리해볼 수 있겠네요 😄
|
|
||
| } | ||
|
|
||
| @Test | ||
| void 제일_높은_position_가진_자동차_구하기() { |
| @@ -20,5 +21,26 @@ public class CarTest { | |||
| car.driveOrStop(4); | |||
| assertThat(car.getPosition()).isEqualTo(1); | |||
| } | |||
|
|
|||
| @Test | |||
| void 랜덤값_마이너스() { | |||
There was a problem hiding this comment.
테스트는 결국 랜덤값이 0미만인 경우 예외를 던진다라는 기능이기 때문에 어떤 결과를 만들어내는지에 대한 의미도 메서드명에 전달되면 어떨까요?
4단계 진행하였습니다.
리뷰 부탁드리겠습니다:)