[재구현] PR #1
[재구현] PR #1
Conversation
| private final Cars cars; | ||
| private final TotalRound totalRound; |
There was a problem hiding this comment.
가지고 있는 필드가 많은 것 같아서
PlayController 가 아니라 서비스 계층인 GameManager 를 만들고 이 GameManager 가 Cars, TotalRound 를 가지고 있게 해도 되겠다.
그러면 MainController 에서 해야할 일이 좀 많아지겠다. 하나의 라운드 마다 carsDto를 받아서 outputView로 전달해줘야하니까.
→ PlayController 를 그대로 두고 GameManager 를 이용하는 것이 좋겠다.
playRound 할 때마다 CarsDto 생성해서 리턴하는 형식으로
OR PlayController 의 play() 를 호출할 때 파라미터로 Cars, TotalRound 를 넘겨줘도 되겠다.
junslog
left a comment
junslog
left a comment
There was a problem hiding this comment.
깔끔한 코드 잘 보았습니다.
객체의 역할, 캡슐화와 응집성 및 변경 가능성에 대한 대비를 중심으로 리뷰해봤어요
|
|
||
| public boolean isEqualPosition(int value) { | ||
| return position == value; | ||
| } |
There was a problem hiding this comment.
여기서 파라미터로 Car를 받아서, car 객체의 position을 비교해주는 건 어떨까요?
파라미터로 들어오는 Car의 position에 대한 getter가 필요할 것 같은데, 위 방법으로 getter를 없앨 수 있을 거 같습니다.
There was a problem hiding this comment.
오 그렇네요 최대한 getter 사용을 지양하도록
인자로 Car 를 넣는 것도 좋고
추가로 compareTo를 사용해도 되고
| int maxPosition = cars.stream() | ||
| .mapToInt(Car::providePosition) | ||
| .max() | ||
| .getAsInt(); |
There was a problem hiding this comment.
Comparable 인터페이스를 구현하면,
compareTo 메서드를 오버라이드 할 수 있고, 이를 이용해서 getter없이, Car 내부에서 최대 거리를 찾는 기능을 수행하도록 할 수 있습니다.
프리코스 3주차 피드백 중 - getter 대신 객체에 메세지를 보내자! 라는 테코블 참고자료를 보시는 것도 좋겠습니다.
| public static WinnerNamesDto from(List<Car> cars) { | ||
| List<String> names = cars.stream() | ||
| .map(Car::provideName) | ||
| .toList(); |
There was a problem hiding this comment.
DTO에서 생성로직을 가지는 것보다,
Mapper 계층이나, 서비스 계층에서 데이터를 변환한 뒤에 DTO에 데이터를 넣어주는 형식으로 구현하는 것은 어떨까요?
DTO를 만드는데 자주 사용되는 record를 이번에 사용해보았는데, 자동 생성되는 코드의 생성자에서는, 그저 넣어주는 데이터를 받는 식으로 구현되어 있더라구요. Java 표준 API가 제시하는 방향이라 어느정도?이유가 있지 않을까 한번 생각해봅니다.
- 만약, 도메인의 name 인스턴스 변수가 String이 아닌 새로운 타입으로 변환될 경우
연관된 모든 dto의 생성로직을 손봐줘야할 것 같습니다.
지금은 이 dto하나이지만, Cars로 부터 파생되는 dto가 20개라면, 20개의 dto 생성 로직을 전부 다 고쳐줘야 할 것 같아요.
| public List<String> readCarNames() { | ||
| System.out.println(START_MESSAGE); | ||
| String input = Console.readLine(); | ||
| List<String> carNamesInput = CarNamesValidator.safeSplit(input); |
There was a problem hiding this comment.
이 부분은 InputView가 CarNamesValidator의 호출 순서를 알아야하는 부분으로 보여요,
InputView가 그저 CarNamesValidator를 호출하고 그에 필요한 인자들만 넘겨주는 식으로 구현할 방법이 있을까요?
shin5774
left a comment
shin5774
left a comment
There was a problem hiding this comment.
짧은시간내에 이렇게 깔끔하게 구현하신거 대단하네요!
개인적인 의견 몇개만 적어놓습니다!
| public static MainController create() { | ||
| return new MainController(InputView.getInstance(), OutputView.getInstance()); | ||
| } | ||
|
|
There was a problem hiding this comment.
controller에 정적 팩토리 메서드 적용한거 상당히 인상 깊었습니다!
| @@ -0,0 +1,7 @@ | |||
| package racingcar.constants; | |||
|
|
|||
There was a problem hiding this comment.
해당 상수들의 사용처가 각각 한개의 클래스밖에 없는거 같은데 이렇게 상수 클래스를 별도로 선언해서 사용하는것보다는 각 클래스에 넣어서 관리하는건 어떨까요?
(2시간30분 이내 소요)
객체를 객체답게 사용하는지를 중점적으로 봐주세요 :)
입력 예외 발생 시 다시 입력받도록 규칙을 바꿔서 구현하였습니다.
감사합니다!