Step4 - 자동차 경주(우승자 출력) #6047
Step4 - 자동차 경주(우승자 출력) #6047
Conversation
src/main/java/Car.java
Outdated
| @@ -1,22 +1,29 @@ | |||
| import java.util.Random; | |||
|
|
|||
| public class Car { | |||
There was a problem hiding this comment.
형민님만의 커밋 컨벤션이 있겠지만 이 미션의 요구사항을 지켜보면서 커밋을 남겨봐도 좋겠습니다!
There was a problem hiding this comment.
README.md 파일에 어떤 기능을 구현할지 정리해 보면
테스트 케이스도 쉽게 도출할 수 있지 않을까요?
src/main/java/Car.java
Outdated
| public class Car { | ||
| private int distance; | ||
| private String carName; |
There was a problem hiding this comment.
각 자동차에 이름을 부여할 수 있다. 자동차 이름은 5자를 초과할 수 없다.
규칙에 어긋난 이름으로 자동차가 생성될 수 있어보여요
관련해서 테스트 코드도 함께 작성해 보면 어떨까요?
src/main/java/Constants.java
Outdated
| public class Constants { | ||
|
|
||
| static final int MOVE_THRESHOLD = 4; | ||
|
|
||
| } |
There was a problem hiding this comment.
상수를 분리하신 점은 좋지만, Constants라는 네이밍은 모든 상수가 한 곳에 집중될 위험이 있습니다.
이렇게 되면 관심사가 다른 상수들까지 한 곳에 모이면서 유지보수가 어려워질 가능성이 있어보여요
특정 도메인과 관련된 상수라면, 각 도메인별로 적절한 네이밍을 가진 클래스로 분리하는 것도 고려해보시면 좋을 것 같습니다.
예를 들어
자동차 자체와 관련된 상수라면 Car 내부에 상수를 선언하셔도 좋고, CarConstants, RacingCarConstants처럼 관리할 수 있겠네요! 😊
src/main/java/RacingCar.java
Outdated
| } | ||
| return carList; | ||
| return cars; |
src/main/java/RacingCar.java
Outdated
| ResultView.askCarNames(); | ||
|
|
||
| int tryTimes = InputView.getAnswerToInteger("시도할 회수는 몇 회 인가요?"); | ||
| String[] carNames = InputView.getCarNames(); |
There was a problem hiding this comment.
String[] carNames = InputView.getCarNames();이전 처럼 getCarNames를 호출하면서 메시지를 같이 출력하는 것이 더 일반적일 것 같아요
혹시 println 메소드 사용이 ResultView의 책임이라고 느껴지셨을까요?
InputView는 콘솔 환경에서 사용자의 입력을 처리하는 책임을 갖고 있기 때문에
적절한 입력을 받기위해 콘솔에 어떤 문구를 출력할지에 대한 책임도 갖고 있다고 생각합니다.
반대로 ResultView는 비즈니스 로직이 완료된 후
사용자에게 모델을 적절히 가공해서 출력하는 책임을 갖고 있다고 이해하면 좋을 것 같아요
src/main/java/ResultView.java
Outdated
| int maxDistance = cars.stream().max(Comparator.comparing(Car::getDistance)).get().getDistance(); | ||
|
|
||
| List<Car> maxCars = cars.stream().collect(Collectors.groupingBy(Car::getDistance)).get(maxDistance); | ||
|
|
||
| return maxCars.stream().map(Car::getCarName).collect(Collectors.joining(",")); |
There was a problem hiding this comment.
우승자를 구하는 로직이 뷰영역에 존재하네요 🥲
뷰는 우승자만을 받아 출력하도록 만들어보면 좋겠습니다!
There was a problem hiding this comment.
우승자를 구하는 역할의 클래스를 만들면서 관련된 테스트코드도 만들어보면 좋겠네요~
|
기존 리뷰 내용 :
1-1) 이 부분에 대해서 ResultView 를 이용해 스스로 출력하게끔 변경한 것인데 제가 잘못 이해한 걸까요? 어떤 느낌을 말씀하신 건지 잘 이해가 가지 않습니다. ㅠ 기존 리뷰 내용 : 2-1) 이 부분도 읽으면서 ResultView에게 출력을 맡긴다고 생각하고 변경을 한것인데 제가 잘못 이해 한건지요..
자동차 경주 게임 이 부분이 보이기만 하네요. |
src/main/java/CarUtils.java
Outdated
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class CarUtils { |
There was a problem hiding this comment.
XXXUtils 네이밍을 봤을 때 유틸 클래스를 만드셨네요 😮
유틸리티 클래스는 특정 객체에 종속되지 않는 범용적인 기능에 적합한 것 같습니다.
대표적으로 빈 문자열 체크, 날짜 변환, ...에 해당됩니다.
그런데 여기서 우승자를 구하는 로직은 핵심 로직이므로
해당 도메인에 맞는 객체에게 책임을 주는 것이 좋을 것 같습니다.
예를들어 자동차의 우승자를 판단하는 객체로는
심판 또는 우승자 같은 객체를 만들어 볼 수 있습니다.
var 심판 = new 심판();
List<Car> 우승자목록 = 심판.getWinners(cars);이런 느낌으로 객체 간 협력을 통한 코드가 나오길 바라면서 리뷰를 드렸었습니다. 😄
src/test/java/CarTest.java
Outdated
| @DisplayName(value = "차량 이름 5글자 넘어간 경우 에러 케이스") | ||
| void 차량명_초과_테스트() { | ||
| assertThatThrownBy(() -> { | ||
| new Car("가나다라마바"); | ||
| }).isInstanceOf(IllegalArgumentException.class); |
src/main/java/Car.java
Outdated
| @@ -1,22 +1,36 @@ | |||
| import java.util.Random; | |||
|
|
|||
| public class Car { | |||
There was a problem hiding this comment.
개발하면서 클래스가 많아진 것 같은데
클래스들을 응집도 있게 패키지로 나눠보면 어떨까요?
가령 입출력은 ui 패키지 처럼
묶어보면 보기좋게 변하지 않을까요?
제 답변이 애매했었네요 🥲
위 답변과 동일합니다.
네 꼭 동일하게 작성할 필요는 없어요
오른쪽 상단 |
No description provided.