요구사항 반영해서 올립니다. #5049
요구사항 반영해서 올립니다. #5049
Conversation
car가 이동하면 - 가 추가되는 메서드 생성
- 중복된 테스트 제거 - I/O 관련 사항은 view 클래스로 이전 - randomNumber 구하는 부분을 interface를 사용해서 구현
중복된 move 제거
| private void race() { | ||
| RacingCars racingCars = new RacingCars(carNumber); | ||
| for (int i = 0; i < rounds; i++) { | ||
| new Round(racingCars).race(); | ||
| showRacingCarsMovingDistance(racingCars); | ||
| System.out.println(ROUND_END); | ||
| } | ||
| } | ||
|
|
||
| private void showRacingCarsMovingDistance(RacingCars racingCars) { | ||
| for (Car car : racingCars.getRacingCars()) { | ||
| System.out.println(showCarMovingDistance(car)); | ||
| } | ||
| } | ||
|
|
||
| private String showCarMovingDistance(Car car) { | ||
| StringBuilder distance = new StringBuilder(); | ||
| for (int i = 0; i < car.getMoving().getMovingDistance(); i++) { | ||
| distance.append(MOVE); | ||
| } | ||
| return distance.toString(); |
There was a problem hiding this comment.
앗 view에 도메인 로직이 너무 많이 스며들어 있습니다.
domain 로직은 view와 관련이 없도록, 독립적인 형태를 띄도록 다른 패키지에서 작성해보시면 좋을것 같습니다.
There was a problem hiding this comment.
@pci2676 궁금한게 저번 pr 에서 Car 에 "-" 변수는 view에 있어야 할거 같다고 해서 뺐는데 도메인 로직이(Car) 없이는 이동거리를 구현하기가 쉽지않아 보입니다. 좋은 방법이 있을까요?
| car.move(5); | ||
| assertThat(car.getMovingDistance()).isEqualTo("-"); | ||
| } | ||
| System.out.println("inputValue=" + inputValue); |
There was a problem hiding this comment.
보통 테스트 코드에는 console에 추가하는 코드를 제거하는 편입니다.
테스트 코드만 보고 이해하기 힘들어 넣은것이 아니라면 제거해주시고
테스트 코드만 보고 이해하기 힘들어 넣은것이라면 전체적인 구조의 개선이 필요하다는 신호로 눈치챌 수 있을것 같습니다.
| @@ -9,8 +8,7 @@ public class Main { | |||
| public static void main(String[] args) { | |||
| InputView inputView = new InputView(); | |||
| inputView.prepareRace(); | |||
| Race race = new Race(inputView.getCarNumbers(), inputView.getTryNumbers()); | |||
| ResultView resultView = new ResultView(race); | |||
| ResultView resultView = new ResultView(inputView.getCarNumbers(), inputView.getTryNumbers()); | |||
There was a problem hiding this comment.
아래와 같은 그림이 되도록 한번 개선해보시겠어요?
도메인 로직은 view의 구체적인 타입과 관련없이 사용할 수 있는 상태로 관리하고
조금 더 응집되도록 하나의 객체로 관리해보시면 좋을것 같습니다.
RacingApplication application = new RacingApplication(new InputView(), OuputView())
application.run()|
|
||
| private static final int MINIMUM_RANDOM_NUMBER = 4; | ||
|
|
||
| private Moving moving; |
There was a problem hiding this comment.
moving이 가변적이라서 final을 붙이지 않을거라면
move를 할때마다 새로운 Moving을 주입해주는것도 하나의 방법입니다.
다만 이렇게 할때는 moving내부의 값도 불변인것이 좋습니다. 추적이 힘들어져요..
그렇지 않다면 final을 굳이 붙이지 않을 이유가 없습니다. IDE에서도 경고를 하고 있네요 😅
| public CarMoving() {} | ||
|
|
||
| public CarMoving(int movingDistance) { | ||
| this.movingDistance = movingDistance; | ||
| } |
There was a problem hiding this comment.
| public CarMoving() {} | |
| public CarMoving(int movingDistance) { | |
| this.movingDistance = movingDistance; | |
| } | |
| public CarMoving(int movingDistance) { | |
| this.movingDistance = movingDistance; | |
| } | |
| public CarMoving() { | |
| this(0); | |
| } |
merge 해주셨지만 요구사항 반영해서 올립니다.
혹시 수정할게 더 있으면 알려주세요