[LBP] 송하은 자동차 경주 미션 4단계 제출 #100
Conversation
There was a problem hiding this comment.
4단계 미션 고생하셨습니다! 👍 👍
알림을 확인하지 못해서 늦게 리뷰하게 되었습니다. 죄송합니다.
개인적으로 테스트 메서드명을 한국어로 작성하는 것이 가장 보기 편해서 한국어로 작성했는데 한국어로 작성하는 경우도 있는지요?
아니면,@DisplayName을 사용하는 일이 더 많을까요?
- 혼자 개발한다면, 선호하는 방식으로 작성하셔도 괜찮아요. 팀 작업이라면 팀 내에서 컨벤션을 지정하고 일관성만 지키면 상관 없는 문제인 것 같아요.
- 저는
@DisplayName을 선호해서 거의 사용하는 것 같아요.
여전히 View 가 아닌 곳에서 System.out.println() 을 사용하는 코드가 섞여있습니다. 이 부분은 수정을 하지 못했는데, 차차 더 해보도록 하겠습니다. 아니면, 넘어갈 수 있는 부분인지요..!
- view 패키지를 나눈 이상 해결해야할 문제라고 생각합니다.
- 객체가 책임지고 있는 일이 많은지 점검하는 시간을 가지면 좋을 것 같아요.
- 특히 RacingGame 부분에서 그런 상황이 발생하는 것 같아요.
- dto를 사용하여 값을 만들어 전달하고 출력하게 하는 방법도 시도해볼 수 있을 것 같네요. 😄
| public static void main(String[] args) throws Exception { | ||
| final var carNames = InputView.getCarNames(); | ||
| final var tryCount = InputView.getRaceRounds(); | ||
|
|
||
| final var racingGame = new RacingGame(carNames, tryCount); |
| public List<Car> getWinner() { | ||
| int maxPosition = getMaxPosition(); | ||
| return cars.stream() | ||
| .filter(car -> car.getPosition() == maxPosition) | ||
| .collect(Collectors.toUnmodifiableList()); | ||
| } |
There was a problem hiding this comment.
참고하면 좋을 글을 소개해드릴게요! getter를 사용하는 대신 객체에 메시지를 보내자
| private void moveCars() { | ||
| for (Car car : cars) { | ||
| System.out.print(car.getName() + " : "); | ||
| int randomValue = random.nextInt(10); | ||
| car.move(randomValue); | ||
| System.out.print("-".repeat(car.getPosition())); | ||
| System.out.println(); | ||
| } | ||
| } |
| private List<Car> createCars(String[] carNames) { | ||
| List<Car> cars = new ArrayList<>(); | ||
| for (String name : carNames) { | ||
| cars.add(new Car(name.trim())); | ||
| } | ||
| return cars; | ||
| } |
There was a problem hiding this comment.
cars를 반환받고, 새로운 요소를 추가하려고 하면 추가될까요?
List<Car> cars = createCars(carNames);
cars.add(new Car()); // 가능할까요?| @Test | ||
| void move_값이_4이상이면_전진한다() { | ||
| Car car = new Car("람보르기니"); | ||
| car.move(5); | ||
| assertThat(1).isEqualTo(car.getPosition()); | ||
|
|
||
| car.move(9); | ||
| assertThat(2).isEqualTo(car.getPosition()); | ||
| } |
There was a problem hiding this comment.
@ParameterizedTest를 학습하고 사용하면 더 나아질 것 같습니다.
| @Test | ||
| void get_winner_테스트() { | ||
|
|
||
| Car car1 = new Car("haeun"); | ||
| Car car2 = new Car("ram"); | ||
| Car car3 = new Car("com"); | ||
|
|
||
| car1.move(3); | ||
| car2.move(9); | ||
| car2.move(5); | ||
| car3.move(5); | ||
| car3.move(9); | ||
|
|
||
| RacingGame game = new RacingGame(List.of(car1, car2, car3), 1); | ||
| List<Car> winners = game.getWinner(); | ||
|
|
||
| assertTrue(winners.contains(car2)); | ||
| assertTrue(winners.contains(car3)); | ||
| } |
There was a problem hiding this comment.
어떤 자동차가 몇번 움직였는지 파악하기 어려운 것 같아요.
- 3, 9, 5, 5 라는 불규칙한 숫자
- car1, car2 등 어떤 자동차인지 읽기 어려움 등
어떻게 개선할 수 있을까요?
| private static String[] splitCarNames(String inputValue) { | ||
| return inputValue.split(","); | ||
| } |
There was a problem hiding this comment.
단순히 ,로 split 하는 기능을 수행하는 것 같아요.
해당 메서드에서 CarNames를 split한다고 꼭 알고 있어야 할까요?
| public class Car { | ||
|
|
||
| private final String name; | ||
| private int position; | ||
| private static final int MOVE_TRIGGER = 4; |
There was a problem hiding this comment.
랜덤 생성은 분리가 되어 있으나,
4 이상의 숫자가 들어와야 이동한다는 것을 알고 있는 것 같아요.
그냥 이동하면 1칸 움직이게 만드는 것은 어떨까요? 이것은 어떤 장점이 있을까요?
|
|
||
| public class InputView { | ||
|
|
||
| private static final int MAX_NAME_LENGTH = 5; |
There was a problem hiding this comment.
최대 이름 글자 수는 비즈니스 규칙 같은데, view가 알고 있어도 괜찮을까요?
개인적으로 테스트 메서드명을 한국어로 작성하는 것이 가장 보기 편해서 한국어로 작성했는데 한국어로 작성하는 경우도 있는지요?
아니면, @DisplayName 을 사용하는 일이 더 많을까요?
여전히 View 가 아닌 곳에서 System.out.println() 을 사용하는 코드가 섞여있습니다. 이 부분은 수정을 하지 못했는데, 차차 더 해보도록 하겠습니다. 아니면, 넘어갈 수 있는 부분인지요..!