Step4 #4719
Step4 #4719
Conversation
| @@ -6,14 +6,15 @@ | |||
| public class Racing { | |||
There was a problem hiding this comment.
Main 함수를 만들어서 프로그램이 실행 될 수 있게 만들어 주세요 :)
src/main/java/racingcar/Racing.java
Outdated
| @@ -6,14 +6,15 @@ | |||
| public class Racing { | |||
| private final List<Car> cars; | |||
There was a problem hiding this comment.
Car 의 일급컬렉션을 만들어서 List<Car> cars 를 상태로 관리하게 해볼까요?
src/main/java/racingcar/Car.java
Outdated
| this.positions = new ArrayList<>(positions); | ||
| } | ||
|
|
||
| private void validateCarName(String name) { | ||
| if(name.length() > 5) { |
There was a problem hiding this comment.
5 매직 넘버를 변수로 선언해서 해당 값이 어떤의미인지 설명해주세요!
src/main/java/racingcar/Car.java
Outdated
| if(positions.isEmpty()) | ||
| return -1; |
There was a problem hiding this comment.
인텔리제이 자동정렬은 윈도우: Ctrl + Alt + L, macOS: Cmd + Alt + L 입니다 :)
코드 전체적으로 포맷팅이 필요 할 것 같아요!
그리고 if 문이 한줄이면 대괄호를 생략해도 되지만, 추후 수정할때 실수를 방지하기위해 대괄호를 쓰는건 어떠신가요?
| if(positions.isEmpty()) | |
| return -1; | |
| if (positions.isEmpty()) { | |
| return -1; | |
| } |
| private final List<Integer> positions; | ||
|
|
||
| Car(int trialCount) { | ||
| Car(String name, int trialCount) { |
There was a problem hiding this comment.
이 생성자는 proceedOrStop 메소드가 항상 동일한 값을 리턴하지 않아서 테스트하기 어려울 것 같은데요.
어떻게 하면 테스트 할 수 있을까요?
| @@ -50,14 +56,79 @@ public class RacingTest { | |||
| @Test | |||
| void 자동차_출력하기() { | |||
There was a problem hiding this comment.
요구사항에 View 에 대한 테스트는 제외되어있어서 이 테스트는 안해도 될 것 같아요.
만약 하고싶으시다면 ResultView 테스트에 있어야 할 것 같습니다 :)
|
|
||
| // Then | ||
| assertThat(inputView.getCarNames()).isEqualTo("pobi,crong,honux"); | ||
| assertThat(inputView.getTrialCount()).isEqualTo(5); | ||
| } | ||
|
|
||
| @Test | ||
| void 랜덤하게_전진() { |
| @@ -28,18 +30,22 @@ public class RacingTest { | |||
| @Test | |||
| void 문자_입력_받기() { | |||
There was a problem hiding this comment.
요구사항에 따르면 InputView에 관한 테스트는 생략해도 됩니다.
만약 하고싶으시다면, 해당 테스트는 InputViewTest 에 있어야 할 것 같네요.
| public class Racing { | ||
| private final List<Car> cars; | ||
| // private final List<Car> cars; | ||
| private CarCollection carCollection; |
| List<Car> cars = new ArrayList<>(); | ||
| for(int i=0; i<carCount; i++) { | ||
| cars.add(new Car(trialCount)); | ||
| private void buildCarCollection(String carNames, int trialCount) { |
There was a problem hiding this comment.
이 메소드는 CarCollection 내부로 이동 할 수 있지 않을까요?
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class CarCollection { |
There was a problem hiding this comment.
CarCollection 은 List 타입을 설명하기 위한 객체명인데요!
레이싱참가자들 과 같은 객체명으로 어떤 객체인지 표현하는게 좋지 않을까요?
| cars.add(car); | ||
| } | ||
|
|
||
| public void removeCar(Car car) { |
| return winner + "가 최종 우승했습니다."; | ||
| } | ||
|
|
||
| private int getBiggestLastPosition(CarCollection carCollection) { |
There was a problem hiding this comment.
이 메소드는 우승자를 구할때 필요한 도메인 로직으로 보이는데요!
View 객체가 아닌 Domain 객체 내부로 이동해야 하지 않을까요?
객체의 레이어 구분을 위해 main/java/racingcar 패키지 하위로 view, domain을 패키지를 나눠주시고 각 객체들을 이동시켜보세요.
| return max; | ||
| } | ||
|
|
||
| private String getWinner(CarCollection carCollection, int biggestLastPosition) { |
There was a problem hiding this comment.
이 메소드도 도메인 객체로 이동해야하는 로직과 View에 남아있어야 할 로직이 섞여 있는 것 같아요 🥲
| assertThat(car.proceedOrStop()).isIn(0, 1); | ||
| assertThat(car.proceedOrStop()).isIn(0, 1); | ||
| assertThat(car.proceedOrStop()).isIn(0, 1); | ||
| assertThat(car.proceedOrStop()).isIn(0, 1); | ||
| assertThat(car.proceedOrStop()).isIn(0, 1); |
There was a problem hiding this comment.
proceedOrStop 메소드는 내부에 랜덤값때문에 어떤 값이 나올지 몰라서 지금 같은 테스트 코드가 나온 것 같아요.
인터페이스를 이용해 랜덤숫자를 만드는 로직을 Car객체에서 분리해보세요 :)
과제 목적에 맞게 구현했는지 의문이 들고있습니다.
그래도 일단 구현해보았습니다.
피드백 부탁드립니다.
감사합니다.