3단계 - 자동차 경주 #5736

yunji1201 · 2024-10-03T07:53:33Z

언제 테스트 코드를 짜야하고 언제 커밋을 나눠야하는지 잘 모르겠습니다.. 우선 todo에 적힌걸 기반으로 커밋을 올리기는 했지만, 막상 프로덕트 코드를 짜면서 거의 어느정도 진행하다가 중간중간 테스트 코드를 짜고 커밋을 했어서 스스로 이게 아닌거같은데.. 라는 생각을 하기도 했습니다 ..

seondongpyo

3단계 미션도 잘 진행해주셨네요 👍
몇 가지 코멘트 드렸는데 확인 후 다시 리뷰 요청을 부탁 드리겠습니다 🔥

seondongpyo · 2024-10-03T22:54:07Z

src/main/java/racingcar/Car.java

+
+public class Car {
+
+    private int position = 0;


Nit: Car 인스턴스 생성 시 자동으로 0으로 초기화되니 명시적으로 초기화하지 않아도 될 것 같습니다.

seondongpyo · 2024-10-03T22:54:10Z

src/main/java/racingcar/Car.java

+
+    private int position = 0;
+
+    public int move(int randomNum) {


객체지향 생활 체조 원칙 중 규칙 5: 줄여쓰지 않는다(축약 금지).를 참고해주세요.

seondongpyo · 2024-10-03T22:54:11Z

src/main/java/racingcar/Car.java

+    private int position = 0;
+
+    public int move(int randomNum) {
+        if (randomNum >= 4) {


숫자 리터럴 4를 하드 코딩하는 대신에 상수로 선언하여 의미를 명확하게 전달해보면 어떨까요?

seondongpyo · 2024-10-03T22:54:14Z

src/main/java/racingcar/InputView.java

+
+import java.util.Scanner;
+
+public class InputView {


상태가 없고 정적 메서드만을 포함하는 클래스이니, 인스턴스를 생성할 필요 없이 직접 사용이 가능하겠습니다만
누군가는 실수로 인스턴스를 생성할 수도 있으니, private 생성자 등을 통해 불필요한 인스턴스화를 막아보면 어떨까요?

seondongpyo · 2024-10-03T22:54:22Z

src/main/java/racingcar/InputView.java

+
+public class InputView {
+
+    private static final Scanner scanner = new Scanner(System.in);


static final을 통해 상수로 선언하셨으니, 이름을 대문자로 작성해보면 어떨까요?

Suggested change

private static final Scanner scanner = new Scanner(System.in);

private static final Scanner SCANNER= new Scanner(System.in);

seondongpyo · 2024-10-03T22:54:55Z

src/main/java/racingcar/Racing.java

+        this.roundNum = roundNum;
+    }
+
+    public void start() {


Racing 클래스는 핵심 비즈니스 로직을 수행하는 도메인 클래스로 보이는데, 내부에서 콘솔에 값을 출력하는 view와 관련된 로직을 수행하고 있어서 도메인이 view에 의존하는 구조로 보입니다. 도메인이 view에 의존할 경우 어떤 문제점이 있을까요?

도메인이 view에 의존할 경우 유지보수하기 까다로워질 것 같습니다. 출력 방식을 수정하려면 도메인을 건드려야할테니까요..! 말씀해 주신 부분 수정해두었습니다. 리뷰 감사합니다!

seondongpyo · 2024-10-03T22:54:58Z

src/main/java/racingcar/Racing.java

+        // 첫 번째 라운드는 무조건 강제로 전진
+        System.out.println(LINE + "1 라운드 시작!" + LINE);
+        playFirstRound();


첫 번째 시도에서 반드시 자동차가 전진해야 한다는 요구사항은 없지 않나요?

홈페이지에 올려져있는 실행결과만 보고 잘못 생각했던 것 같습니다. 수정하였습니다!

seondongpyo · 2024-10-03T22:55:37Z

src/main/java/racingcar/ResultView.java

+        for (int i = 0; i < position; i++) {
+            sb.append("-");
+        }


반복문 대신에 Racing에서 처리하신 것처럼 String의 repeat()를 활용해보시면 좋을 것 같습니다.

seondongpyo · 2024-10-03T22:55:49Z

src/test/java/racingcar/RacingCarTest.java

+
+    @ParameterizedTest
+    @MethodSource("randomNum")
+    @DisplayName("자동차가 이동하는 값이 4 이상일 경우에만 전진하고 그렇지않으면 멈춰있는지 확인")


위에서 각 경우에 대한 자동차 전진 여부 테스트를 잘 작성해주신 것 같은데, 해당 테스트를 추가하신 이유가 있으실까요?

위에서 각각 테스트를 추가해서 해보다가, 값이 랜덤이라는 것을 적용하지 못했던 것 같아 세번째로 추가해보았는데 말씀해주신 것처럼 생각해보니 위 사항들과 모두 중복된 내용인 것 같아 위 테스트를 제거하였습니다.

위의 테스트를 제거한다기보다는 아래에 작성된 무작위성을 포함하는 테스트를 제거하는 게 어땠을까 싶습니다.
지금 있는 테스트보다 오히려 제거한 테스트들이 자동차의 '이동', '멈춤'을 명확하게 테스트한다고 볼 수 있지 않을까요?

seondongpyo · 2024-10-03T23:03:24Z

src/test/java/racingcar/RacingCarTest.java

언제 테스트 코드를 짜야하고 언제 커밋을 나눠야하는지 잘 모르겠습니다.. 우선 todo에 적힌걸 기반으로 커밋을 올리기는 했지만, 막상 프로덕트 코드를 짜면서 거의 어느정도 진행하다가 중간중간 테스트 코드를 짜고 커밋을 했어서 스스로 이게 아닌거같은데.. 라는 생각을 하기도 했습니다 ..

혹시 TDD 사이클에 따라 미션을 진행 중이시라면, 저라면 사이클에 따라 다음과 같이 커밋을 작성해볼 것 같아요.

실패하는 테스트를 작성(컴파일 에러까진 해결)하고 커밋

해당 테스트가 통과하도록 프로덕션 코드를 작성하고 커밋

해당 프로덕션 코드를 리팩토링하고 커밋

아직 TDD가 익숙하지 않으시다면 지금처럼 TO-DO 리스트의 요구사항 하나를 만족하는 코드를 작성한 이후에 커밋을 하셔도 좋습니다. 물론 이전에 커밋했던 내역도 다시 되돌릴 수 있으니, 커밋 단위가 크다고 생각되시면 다시 작은 단위로 나누는 연습을 반복적으로 해보시면 좋을 것 같습니다.

감사합니다. 다음 스텝부터 적용해보면서 적응해가도록 하겠습니다.

seondongpyo

피드백 잘 반영해주셨네요 👍
몇 가지 추가로 코멘트 드렸는데 다음 단계에서 확인해주시면 좋을 것 같습니다.
즐거운 주말 되세요 🔥

seondongpyo · 2024-10-05T00:14:58Z

src/main/java/racingcar/Car.java

+    public static final int MOVE_CONDITION = 4;
+    private int currentPosition;


상수와 인스턴스 변수의 구분을 위해 개행을 추가해보면 어떨까요?

Suggested change

public static final int MOVE_CONDITION = 4;

private int currentPosition;

public static final int MOVE_CONDITION = 4;

private int currentPosition;

seondongpyo · 2024-10-05T00:15:00Z

src/main/java/racingcar/Car.java

+    public static final int MOVE_CONDITION = 4;
+    private int currentPosition;
+
+    public int moveCar(int randomNumber) {


자동차가 직접 움직임 여부를 결정하고 이동하거나 멈추니, 메서드 이름에 -Car는 포함되지 않아도 되지 않을까요?
오히려 이름에 -Car가 있어서 마치 다른 자동차를 움직이도록 하는 것처럼 보일 수 있을 것 같습니다.

seondongpyo · 2024-10-05T00:15:06Z

src/main/java/racingcar/Racing.java

+
+    private final List<Car> cars;
+    private final int roundNum;
+    private final ResultView resultView;


Racing이 ResultView를 상태로 갖는다는 건 여전히 도메인이 view에 의존하는 구조라고 볼 수 있지 않을까요?

seondongpyo · 2024-10-05T00:15:11Z

src/main/java/racingcar/ResultView.java

+
+public class ResultView {
+
+    private static final Scanner scanner = new Scanner(System.in);


scanner는 사용하지 않는 변수로 보이는데, 불필요하다면 제거하는 게 어떨까요?

seondongpyo · 2024-10-05T00:15:12Z

src/main/java/racingcar/ResultView.java

+public class ResultView {
+
+    private static final Scanner scanner = new Scanner(System.in);
+    public static final String LINE = "-".repeat(5);


해당 클래스 내부에서만 사용하는 상수라면, 접근 제어자를 private으로 선언해보면 어떨까요?

seondongpyo · 2024-10-05T00:15:36Z

src/main/java/racingcar/ResultView.java

+    }
+
+    public static void drawCarPosition(int position) {
+        StringBuilder sb = new StringBuilder();


객체지향 생활 체조 원칙 중 규칙 5: 줄여쓰지 않는다(축약 금지). 항목을 참고해주세요.

seondongpyo · 2024-10-05T00:20:08Z

src/test/java/racingcar/RacingCarTest.java

+
+    @ParameterizedTest
+    @MethodSource("randomNum")
+    @DisplayName("자동차가 이동하는 값이 4 이상일 경우에만 전진하고 그렇지않으면 멈춰있는지 확인")


위의 테스트를 제거한다기보다는 아래에 작성된 무작위성을 포함하는 테스트를 제거하는 게 어땠을까 싶습니다.
지금 있는 테스트보다 오히려 제거한 테스트들이 자동차의 '이동', '멈춤'을 명확하게 테스트한다고 볼 수 있지 않을까요?

yunji1201 added 6 commits October 3, 2024 14:39

docs : 기능 분석 후 todo.md 업데이트

6d90f3e

test : 0에서 9 사이에서 값이 4 이상일 경우에만 전진하고 4 이하일 경우 멈춰있는지 확인하는 테스트

ae52aa8

feat : 사용자가 자동자 수, 이동 수를 입력 할 수 있어야 함

4eb86ac

test : 0에서 9 사이에서 random 값을 구한 후 random 값이 4이상일 경우에만 전진하는 테스트

1551f21

feat : 자동차가 사용자가 입력한 값에 따라 경기를 진행하고 움직이도록 기능 추가

ad36628

feat : 자동차의 상태를 화면에 출력

034e01f

seondongpyo requested changes Oct 3, 2024

View reviewed changes

yunji1201 added 7 commits October 4, 2024 19:40

refactor : 중복된 내용의 테스트 메소드 제거

6876f60

refactor : 축약된 메소드명, 변수명 명확한 의미를 갖도록 수정

d0b3845

refactor : private 생성자로 불필요한 인스턴스화 막기

ed4b0e7

refactor : for문 -> repeat 함수 사용

3f45250

refactor : 첫번째 라운드에서 자동차가 반드시 전진하지 않아도 되도록 코드 수정

ef89a77

refactor : 레이싱 라운드 시작 표시 콘솔을 VIEW에서 하도록 수정

b73dbef

refactor : 비즈니스 로직과 UI 로직을 분리하기 위하여 의존성 주입을 사용한다

c0ac975

seondongpyo approved these changes Oct 5, 2024

View reviewed changes

seondongpyo merged commit e06edf2 into next-step:yunji1201 Oct 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3단계 - 자동차 경주 #5736

3단계 - 자동차 경주 #5736

yunji1201 commented Oct 3, 2024

seondongpyo left a comment

seondongpyo Oct 3, 2024

seondongpyo Oct 3, 2024

seondongpyo Oct 3, 2024

seondongpyo Oct 3, 2024

seondongpyo Oct 3, 2024

seondongpyo Oct 3, 2024

yunji1201 Oct 4, 2024

seondongpyo Oct 3, 2024

yunji1201 Oct 4, 2024

seondongpyo Oct 3, 2024

seondongpyo Oct 3, 2024

yunji1201 Oct 4, 2024

seondongpyo Oct 5, 2024

seondongpyo Oct 3, 2024

yunji1201 Oct 4, 2024

seondongpyo left a comment

seondongpyo Oct 5, 2024

seondongpyo Oct 5, 2024

seondongpyo Oct 5, 2024

seondongpyo Oct 5, 2024

seondongpyo Oct 5, 2024

seondongpyo Oct 5, 2024

seondongpyo Oct 5, 2024


		public class InputView {

		private static final Scanner scanner = new Scanner(System.in);

	private static final Scanner scanner = new Scanner(System.in);
	private static final Scanner SCANNER= new Scanner(System.in);

		public static final int MOVE_CONDITION = 4;
		private int currentPosition;


		public class ResultView {

		private static final Scanner scanner = new Scanner(System.in);

3단계 - 자동차 경주 #5736

3단계 - 자동차 경주 #5736

Conversation

yunji1201 commented Oct 3, 2024

seondongpyo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seondongpyo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment