자동차 경주 게임 제출합니다! #6
Conversation
| private void validateName() { | ||
| if (name.length() > 5) { | ||
| throw new IllegalArgumentException(name + ": 자동차 이름은 5글자 이하여야 합니다."); | ||
| } | ||
|
|
||
| if (name.isBlank()) { | ||
| throw new NotBlankException("공백으로만 이루어진 이름을 생성할 수 없습니다."); | ||
| } |
There was a problem hiding this comment.
입력에 대한 예외를 더욱 세분화하면 사용자 입장에서 더 편리할것 같아요✔
- 입력이 없을 경우
- 쉼표가 연속으로 있을 경우
- 영어 이외의 문자가 입력될 경우
- 쉼표로 시작할 경우
- 쉼표로 끝날 경우
- 이름의 길이가 6자 이상일 경우
- 같은 이름이 입력될 경우
저는 이렇게 이름 입력에 대한 예외를 7가지로 정했습니다!
| public Receiver() { | ||
| receiveCarNames(); | ||
| receiveRound(); | ||
| } |
There was a problem hiding this comment.
이름을 입력받고 라운드를 입력받기 전에 이름 입력에 대한 예외 처리를 먼저 해주면 더 좋을 것 같아요.
이름 입력에 오류가 있으면 라운드 입력을 받지 않고 진행할 수 있어서 더 효율적인 프로그램이 될 것 같아요!
| private void validateName() { | ||
| if (name.length() > 5) { | ||
| throw new IllegalArgumentException(name + ": 자동차 이름은 5글자 이하여야 합니다."); | ||
| } | ||
|
|
||
| if (name.isBlank()) { | ||
| throw new NotBlankException("공백으로만 이루어진 이름을 생성할 수 없습니다."); | ||
| } |
There was a problem hiding this comment.
이름 입력에 대한 예외를 throw만 해주고 받아주는 곳이 없네요
그러다 보니까 에러 메세지만 띄우고 프로그램이 끝나버려 프로그램이 불친절한 것 같아요.
valiateName 메서드를 Car 객체를 생성할 때 호출하는 것 보다 Receive 클래스에서 실행시켜주면 예외 처리하고 반복적으로 입력을 받기 편할 것 같아요!
src/main/java/domain/CarFactory.java
Outdated
| } | ||
|
|
||
| private String[] splitCarNames(String input) { | ||
| String s = input.replaceAll(BLANK, ""); |
There was a problem hiding this comment.
"" 이 부분도 상수로 선언해 주는 게 통일감 있어 보일 것 같아요!
src/main/java/utils/RandomUtils.java
Outdated
| public static int nextInt(final int startInclusive, final int endInclusive) { | ||
| if (startInclusive > endInclusive) { | ||
| throw new IllegalArgumentException(); | ||
| } | ||
|
|
||
| if (startInclusive < 0) { | ||
| throw new IllegalArgumentException(); | ||
| } | ||
|
|
||
| if (startInclusive == endInclusive) { | ||
| return startInclusive; | ||
| } | ||
|
|
||
| return startInclusive + RANDOM.nextInt(endInclusive - startInclusive + 1); | ||
| } |
There was a problem hiding this comment.
이 메서드에서도 예외를 throw만 해주고 받아주는 부분이 없네요!
| public int compareTo(Car o) { | ||
| return o.position - position; | ||
| } |
There was a problem hiding this comment.
사용되지 않는 메서드같아요!
이런 부분을 줄이려면 README에서 필요한 클래스와 메서드를 정리해가며 진행하는 것이 좋은것 같아요😃
| racingCarGame.printWinners(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Receiver 부분을 테스트하지 못하신 것 같아요.
저희도 입력 부분을 어떻게 처리해야 하나 고생했습니다😂
찾아보니 ByteArrayInputStream()를 사용해서 Test에서 입력을 넣어줄 수 있습니다!
저 부분을 참고해서 Receiver 부분도 테스트 코드를 작성해 보시기 바랍니다!
src/main/java/domain/CarFactory.java
Outdated
| public class CarFactory { | ||
| public static final String COMMA = ","; | ||
| public static final String BLANK = " "; | ||
| private final List<Car> cars; | ||
| private final String[] carNames; | ||
|
|
||
| public CarFactory(String input) { | ||
| this.carNames = splitCarNames(input); | ||
| cars = createCars(); | ||
| } |
There was a problem hiding this comment.
CarFactory 클래스를 따로 만드신게 너무 좋은 것 같아요!👍
Car들를 따로 관리해 줄 클래스가 필요하다고 생각했는데 이런 식으로 하면 되겠군요!
| public class Receiver { | ||
| public static final String REQUEST_CAR_NAME = "경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"; | ||
| public static final String REQUEST_ROUND = "시도할 횟수는 몇회인가요?"; | ||
| private static final Scanner scanner = new Scanner(System.in); |
src/main/java/ui/Receiver.java
Outdated
| public static final String REQUEST_CAR_NAME = "경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"; | ||
| public static final String REQUEST_ROUND = "시도할 횟수는 몇회인가요?"; |
There was a problem hiding this comment.
해당 클래스에서만 사용하는데 public하게 관리한 이유가 있을까요?
src/main/java/ui/Receiver.java
Outdated
| private void receiveCarNames() { | ||
| System.out.println(REQUEST_CAR_NAME); | ||
| carNames = scanner.nextLine(); | ||
| } | ||
|
|
||
| private void receiveRound() { | ||
| System.out.println(REQUEST_ROUND); | ||
| round = scanner.nextInt(); | ||
| } |
There was a problem hiding this comment.
여기서 next()를 통해 받은 값을 return한다면 getter와 멤버 변수가 사라질 것 같아요!
| import java.util.Random; | ||
|
|
||
| public class RandomUtils { | ||
| private static final Random RANDOM = new Random(); |
| private RandomUtils() { | ||
| } |
| public static final String COMMA = ","; | ||
| public static final String BLANK = " "; | ||
| private final List<Car> cars; | ||
| private final String[] carNames; |
There was a problem hiding this comment.
배열 보다는 컬렉션을 사용하는 것은 어떨까요?
그리고 이 경우엔 굳이 인스턴스 변수로 관리할 필요가 없을 것 같네요!
src/main/java/domain/CarFactory.java
Outdated
| } | ||
|
|
||
| private String[] splitCarNames(String input) { | ||
| String s = input.replaceAll(BLANK, ""); |
There was a problem hiding this comment.
이 기능은 String의 trim()이란 메소드로 해결할 수 있습니다!
api를 찾아보는 습관을 가져보세요 :)
|
|
||
| import utils.RandomUtils; | ||
|
|
||
| public class RacingCarGame { |
There was a problem hiding this comment.
RacingCarGame에선 어떤일을 할까요?
- 랜덤 숫자를 만들고(makeRandomValue)
- 자동차를 전진시키고(movePosition)
- 게임을 진행시키고(proceed)
- 상태를 출력하고(print)
다양한 기능을 하고있네요! 하지만 일관된 기능들은 아닌 것 같아요.
SRP(Single Responsibility Principle)을 지키도록 변경해보면 어떨까요?
| } | ||
|
|
||
| if (name.isBlank()) { | ||
| throw new NotBlankException("공백으로만 이루어진 이름을 생성할 수 없습니다."); |
There was a problem hiding this comment.
이 부분에 대해서만 커스텀 익셉션을 정의한 이유가 있을까요?
일관된 예외처리가 아니라면 무슨 차이인지 혼동을 줄 수 있을 것 같아요.
| System.out.print(cars.get(0).getName()); | ||
|
|
||
| for (int i = 1; i < cars.size(); i++) { | ||
| if (cars.get(i).getPosition() < max) { |
There was a problem hiding this comment.
객체의 값을 꺼내는 것이 아닌 메시지를 던지는 방식으로 변경해보면 어떨까요?
No description provided.