Step5 - 자동차 경주(리팩토링) 및 피드백 반영 #2396
Conversation
| private static final int LIMIT_CAR_NAME_LENGTH = 5; | ||
| private String carName; |
| this.carName = carName; | ||
| } | ||
|
|
||
| public static Name validateByName(String carName) { |
There was a problem hiding this comment.
해당 메서드가 하는 역할은 검증 보다는 이름의 생성에 가까워보이는데요
더욱 적절한 네이밍으로 변경해보면 어떨까요?
|
|
||
| public static Name validateByName(String carName) { | ||
| if(carName.length() >= LIMIT_CAR_NAME_LENGTH) { | ||
| throw new IllegalArgumentException("이름은 5글자가 최대입니다."); |
There was a problem hiding this comment.
예외처리 메시지에서 검증 조건의 값을 하드코딩한다면 어떠한 문제가 있을지 고민해보고 상수를 활용해보아요
| @@ -22,17 +30,11 @@ private int getFirstResultPoint() { | |||
| return firstResultPoint; | |||
| } | |||
|
|
|||
| public void extracted(ResultView resultView, List<Car> cars, Winner winner) { | |||
| public void winnerSelection(ResultView resultView, List<Car> cars) { | |||
There was a problem hiding this comment.
domain 패키지의 객체는 view 패키지 객체에 의존하지 않도록 구현한다.의 조건을 지켜보아요
| private int randomNumber; | ||
|
|
||
| public int producedRandomNumber() { | ||
| randomNumber = createRandomNum(); |
There was a problem hiding this comment.
producedRandomNumber()에서 createRandomNum()를 바로 호출하고 있는데 랜덤에 대한 의존을 분리할 수 있을까요!?
| } | ||
|
|
||
| private int createRandomNum() { | ||
| Random random = new Random(); |
There was a problem hiding this comment.
매번 Random을 생성하지 않고 재사용할 수 있도록 수정한다면 어떠한 장점이 있을까요?
충분한 고민 후 이펙티브 자바 아이템 6. 불필요한 객체 생성을 피하라를 참고해주세요!
There was a problem hiding this comment.
random 객체를 새로 생성해서 난수를 발생시키는줄 알았었는데 불필요했네요.
확인하고 수정했습니다.👍
| @@ -24,18 +25,11 @@ | |||
| private String[] createNameOfCarsArray(String text) { | |||
| String[] cars = text.split(","); | |||
| for (int i = 0; i < cars.length; i++) { | |||
| carLengthValidate(cars[i]); | |||
| Name.validateByName(cars[i]); | |||
There was a problem hiding this comment.
View에서 Name을 통해 검증을 하여도 실제 도메인에서 Name을 사용하지 않는다면 동일한 문제가 남아있을 것 같습니다
Car에서 String으로 가지고 있는 name을 Name으로 변경한다면 어떠한 차이가 있을지 고민해보아요
|
|
||
| public class RacingGame { | ||
| private static final int MAX_VALUE = 10; | ||
|
|
||
| public static void main(String[] args) { |
There was a problem hiding this comment.
지금은 main 메서드에서 많은 역할을 담당하고 있는데요
비즈니스 로직의 흐름제어를 담당하는 일종의 Controller와 같은 객체를 분리하여 추상화해보면 좋을 것 같습니다!
해당 이미지를 참고하여 주세요!
| @@ -2,17 +2,23 @@ | |||
|
|
|||
| public class Name { | |||
| private static final int LIMIT_CAR_NAME_LENGTH = 5; | |||
| private static final String ERR_MESSAGE = "이름은 5글자가 최대입니다."; | |||
There was a problem hiding this comment.
#2396 (comment) 에서 남긴 피드백은 "이름은 " + LIMIT_CAR_NAME_LENGTH + "글자가 최대입니다."와 같이 예외처리에서 사용하는 상수에 대한 하드코딩을 말씀드렸는데 전달이 잘못된 것 같군요 😮
| private int randomNumber; | ||
|
|
||
| public int producedRandomNumber() { | ||
| randomNumber = createRandomNum(); |
There was a problem hiding this comment.
producedRandomNumber()에서 createRandomNum()를 바로 호출하고 있는데 랜덤에 대한 의존을 분리할 수 있을까요!?
| for (Car winner : winners) { | ||
| winnerText.append(winner.getName()).append(" "); | ||
| } | ||
| System.out.println("우승자는 " + winnerText + "입니다."); | ||
| } | ||
|
|
||
| public void printAll(List<Car> cars) { | ||
| for(Car car : cars) { | ||
| moveNumber = car.move(randomNumber.producedRandomNumber()); |
There was a problem hiding this comment.
자동차를 이동시키는 행위는 로직에 해당할 것 같네요!
View보다는 Controller 또는 Domain에서 담당하는 것이 좋을 것 같은데 어떻게 생각하시나요?
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class RacingCarController { |
There was a problem hiding this comment.
Controller를 사용하는 main과 Controller 둘 다 View에 의존성을 가지고 있는 상태인데요
View를 사용하는 객체가 많아지면 어떠한 문제가 있을지 고민해보면 좋겠어요!
Controller 내부에서 View를 사용하는 구조나
main에서 View와 Controller를 연결하는 구조 둘 중 하나를 선택하여 한 곳에서만 View와 의존성을 가지도록 수정해보아요
There was a problem hiding this comment.
다른 view 객체를 가지고 진행을 할수도 있겠네요. 도메인을 설계하는데에 있어서 의존성 문제를 계속 생각하면서 해볼게요.
|
|
||
| import java.util.List; | ||
|
|
||
| /* | ||
| * 자동차 경주 결과 View | ||
| */ | ||
| public class ResultView { | ||
| private static final RandomNumber randomNumber = new RandomNumber(); | ||
|
|
||
| private int moveNumber; |
No description provided.