Step3 - 자동차 경주 #2336
Merged
Step3 - 자동차 경주 #2336
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jaeyeonling
approved these changes
Jul 21, 2021
Comment on lines
+7
to
+8
| private final static int GO_VALUE = 4; | ||
| private int moveSpace; |
There was a problem hiding this comment.
Suggested change
| private final static int GO_VALUE = 4; | |
| private int moveSpace; | |
| private static final int GO_VALUE = 4; | |
| private int moveSpace; |
static이 final보다 앞에 오는 것이 컨벤션입니다!
또한 클래스 변수와 인스턴스 변수 사이에 공백을 추가한다면 가독성이 상승합니다 :)
| return moveSpace; | ||
| } | ||
|
|
||
| public static List<Car> createCars(int carNumber) { |
| * 자동차 경주 Input | ||
| */ | ||
| public class InputView { | ||
| Scanner sc = new Scanner(System.in); |
There was a problem hiding this comment.
Suggested change
| Scanner sc = new Scanner(System.in); | |
| private final Scanner sc = new Scanner(System.in); |
접근제어자를 생략한 것은 의도된 내용일까요!?
There was a problem hiding this comment.
Scanner 클래스를 사용할때 무의식적으로 접근제어자를 생략하는 버릇이 들었습니다.
항상 접근제어자를 써보도록 노력할게요
|
|
||
| public int getNumberOfCars() { | ||
| System.out.println("자동차 대수는 몇 대 인가요?"); | ||
| return Integer.parseInt(sc.nextLine()); |
There was a problem hiding this comment.
Suggested change
| return Integer.parseInt(sc.nextLine()); | |
| return sc.nextInt(); |
와 같이 작성해볼 수 있을 것 같네요 :)
| import java.util.List; | ||
|
|
||
| public class RacingGame { | ||
| private static final int MAX_VALUE = 10; |
| List<Car> cars = Car.createCars(carNumber); | ||
|
|
||
| for(int i = 0; i < tryNumber; i++) { | ||
| //5회 |
| for(Car car: cars) { | ||
| Random random = new Random(); | ||
| int randomNumber = random.nextInt(MAX_VALUE); | ||
| int moveNumber = car.move(randomNumber); |
There was a problem hiding this comment.
View에서 도메인 기능을 사용하는 것이 올바른 설계일지 고민해보면 좋겠어요~
도메인과 View를 분리하였을 때 어떠한 장점이 있을지를 중점으로 접근해보면 도움이 될 것 같습니다 😄
| result = car.move(randomNumber); | ||
| } | ||
|
|
||
| assertThat(car.getMoveSpace()).isEqualTo(result); |
There was a problem hiding this comment.
car.move에서 위치를 반환하기 때문에 사실상 getMoveSpace()의 결과와 같은 값이 나올 것 같은데요!
해당 테스트로 move 메서드가 잘 동작하는지 테스트가 가능할지 고민해보면 좋을 것 같습니다 :)
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| inputView = new InputView(); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
이번 PR에서는 미흡한 부분이 많았던 것 같습니다.
의견 주시는 부분 이해하면서 고쳐보도록 하겠습니다.
감사합니다.