최성원 자동차 경주 게임 #4
Conversation
각 차량이 randomlyForward함수 통해서 이동할 수 있도록 구현 Related To : Dcom-KHU#4 Dcom-KHU#5
|
PR 메세지를 작성 부탁드려요!. 자신의 코드에 대해서 설명도 가능하고, 어떤 점을 중점으로 개발을 진행하셨는지, 리뷰는 어떤식으로 받고 싶으신지, 자신의 프로젝트에서 각 파일이 하는 일이 무엇인지 간단한 설명도 붙여주시면 더 좋을것 같습니다! PR의 제목도 설정할 수 있어요. PR 제목도 한번 고민해 보시면 좋을것 같아요! |
|
|
||
| public class InputValidator { | ||
| public String[] validateSplit(String carString) { | ||
| try { | ||
| return carString.split(","); | ||
| } catch (PatternSyntaxException e) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차 이름은 쉼표를 기준으로 구분되어야 한다."); | ||
| } | ||
| } | ||
|
|
||
| public void validateCarNames(List<Car> cars) { | ||
| for (Car car : cars) { | ||
| String carName = car.getName(); | ||
| if (carName.length() > 5) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차 이름은 5자 이하만 가능하다."); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
InputValidator는 본인이 field를 가지는 것이아니라, 모든 로직을 파라미터로 받아서 검증하고 있는것 같은데, Static class는 어떠신가요?
There was a problem hiding this comment.
오 그것도 좋을 것 같습니다!
제가 자바에 대해 아직 개념이 완벽히 잡히진 않아 그 부분에 대해 더 알아보겠습니다
src/main/java/racingcar/CarGame.java
Outdated
| int MAX_DISTANCE = -1; | ||
| for (Car car : cars) { | ||
| if (car.getPosition() == MAX_DISTANCE) { | ||
| winner.add(car.getName()); | ||
| } | ||
| if (car.getPosition() > MAX_DISTANCE) { | ||
| MAX_DISTANCE = car.getPosition(); |
There was a problem hiding this comment.
대문자 변수명은 주로 constant 변수를 표현할때 사용하는걸로 알고있습니다. 혹시 요 파트에만 특별히 대문자로 변수를 지으신 이유가 있으신가요?
There was a problem hiding this comment.
변수값이 변경될 때 직접 바뀌는 게 아니라(변수 값에 다른 값을 더하거나 빼는 등) 다른 값을 대입하기만 한다는 점에서 뭔가 대문자로 하는게 의미가 잘 전달될 것 같아서 지었습니다
생각해보니 저 변수는 constant 변수가 아니네요.. 그냥 소문자로 하는게 더 나은것처럼 보입니다
src/main/java/view/InputView.java
Outdated
| private List<Car> validateCarNames(String[] carNames) { | ||
| List<Car> cars = new ArrayList<>(); | ||
| for (String carName : | ||
| carNames) { | ||
| cars.add(new Car(carName)); | ||
| } | ||
| inputValidator.validateCarNames(cars); | ||
| return cars; | ||
| } |
There was a problem hiding this comment.
validate라는 method 명을 가진 함수가 Car 객체를 생성하기위한 정보를 validation check 하고, 직접 생성까지 하는건 저는 책임이 조금 많아 보입니다. 혹시 view class car 생성 로직을 담당시킨 이유가 있으신가요?
There was a problem hiding this comment.
필요한 기능을 생각대로 구현하다 보니 저 method가 책임이 많아진 것 같습니다.
저 for문에 해당하는 로직을 따로 method로 추출할 수 있어 보이네요!
| // TODO 구현 진행 | ||
| CarGame carGame = new CarGame(); | ||
| carGame.start(); | ||
| } |
| private final InputView inputView; | ||
| private final OutputView outputView; | ||
|
|
||
| public CarGame() { | ||
| this.inputView = new InputView(); | ||
| this.outputView = new OutputView(); | ||
| } |
There was a problem hiding this comment.
DI 적인 요소들이 많이 보여서 엄청 좋은것 같아요!
There was a problem hiding this comment.
spring에서 하던 느낌과 최대한 비슷하게 해보려 했습니다!
| public void validateEmpty(String[] carNames) { | ||
| if (carNames.length == 0) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차는 1대 이상 입력 되어야 한다."); | ||
| } | ||
| for (String carName : | ||
| carNames) { | ||
| if (carName.trim().isEmpty()) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차 이름은 빈칸일 수 없다."); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public Integer validateTrial(String trial) { | ||
| try { | ||
| Integer trialNum = Integer.valueOf(trial); | ||
| if (trialNum < 0) { | ||
| throw new IllegalArgumentException("[ERROR] 시도 횟수는 음수일 수 없다."); | ||
| } | ||
| return trialNum; | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException("[ERROR] 시도 횟수는 숫자여야 한다."); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
오류 케이스를 자세히 나누어 처리한게 정말 좋은 것 같습니다!
src/main/java/racingcar/CarGame.java
Outdated
| private void startGame(List<Car> cars, Integer trial) { | ||
| outputView.printInit(); | ||
| for (int i = 0; i < trial; i++) { | ||
| for (Car car : cars) { | ||
| goOrNot(car); | ||
| } | ||
| outputView.printResult(cars); | ||
| } |
There was a problem hiding this comment.
저는 java를 시작한지 얼마 안되어서 c++ 습관대로 정수형을 int로만 표현하는데, Integer 타입을 주로 사용하신 것 같습니다. 혹시 int와 Integer를 구분해서 사용하는 기준이 있을까요?
There was a problem hiding this comment.
저는 개인적으로 참조 타입(Integer, Double 등)이 원시 타입(int, long 등)과 같은 값을 갖고 있지만 많은 메소드들을 지니고 있어 여러모로 편리하다고 느껴 주로 사용하고 있었습니다. 그런데 찾아보니 성능 면에서는 원시 타입이 더 좋다고 하네요. 성능을 위해선 원시 타입을 우선시 하되 좀 더 범용적인 사용을 원한다면 참조 타입을 사용하는 게 좋아 보이네요.
| public void validateEmpty(String[] carNames) { | ||
| if (carNames.length == 0) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차는 1대 이상 입력 되어야 한다."); | ||
| } | ||
| for (String carName : | ||
| carNames) { | ||
| if (carName.trim().isEmpty()) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차 이름은 빈칸일 수 없다."); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public Integer validateTrial(String trial) { |
There was a problem hiding this comment.
validate 형식의 메소드끼리 리턴타입을 통일시키는 것도 좋을 것 같습니다!
| public String[] validateSplit(String carString) { | ||
| try { | ||
| return carString.split(","); | ||
| } catch (PatternSyntaxException e) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차 이름은 쉼표를 기준으로 구분되어야 한다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
쉼표를 구분해서 에러를 띄우는 방법을 고민했는데, 좋은 방법인 것 같습니다!
| for (String carName : | ||
| carNames) { | ||
| if (carName.trim().isEmpty()) { | ||
| throw new IllegalArgumentException("[ERROR] 자동차 이름은 빈칸일 수 없다."); |
There was a problem hiding this comment.
자동차 이름 빈칸 검증 부분이 단지 length가 0인 것만 생각했는데, 이런 방법도 있네요! 좋은 방법인 것 같습니다.
단순하고 간단한 입출력부분과 핵심 로직 부분을 분리하기 위해 view 패키지를 따로 만들고 View 클래스들을 만들었습니다. 입력시 여러가지 검증을 위해 입력검증 클래스도 만들었습니다
Application(main)클래스에서는 속의 내용을 알지 않아도 되게끔 CarGame 이라는 클래스를 만들어 자동화스럽게 구현하려고 했습니다
핵심 로직들은 대부분 CarGame 에서 다 구현했습니다