자동차 경주 게임! #15

BangKiHyun · 2020-04-30T08:25:52Z

첫 번째 자동차 경주 게임입니다!

src/test/java/racing/domain/CarsTest.java

src/main/java/racing/view/InputView.java

jyami-kim · 2020-05-01T15:10:22Z

src/main/java/racing/view/InputCount.java

+    }
+
+    private void checkPositive(int count) {
+        if (isNotPositive(count)) {


이거는 개인 코드 차이인데요, 만약 이 조건문이 일회성으로 사용된다면 저는 private 이어도 method로 빼지 않는 편입니다.
조건문을 메서드로 빼서 중복을 해결할 수 있다면 정답이지만
한번만 사용되는 조건문이라면 지역변수로 넣어두기도 하는 분도 있다고 들었습니다..!
여튼 조건문이 어떤걸 확인하는지 리터럴로 지정해주는건 보는 사람이 코드를 읽기 쉽게 만들어주는 거라서 좋다고 봅니다.

boolean isNotPositive = count <= 0;

jyami-kim · 2020-05-01T15:12:35Z

src/main/java/racing/domain/Car.java

+        return distance;
+    }
+
+    public void move() {


이부분 테스트를 어떻게 하고 있나요?
랜덤이라서 테스트가 어려울 것 같은데 조언을 받아보세요!

public void move(int distance){ this.distance += distance }

이런식으로 함수를 만들고 인자로 넣어줄 때 randomtuil 을 사용한다면 테스트도 가능한 함수가 됩니다 :)
지금도 좋은데!! 다른분들은 어떻게 테스트를 했는지 얘기해보면 좋을것 같아요

jyami-kim · 2020-05-01T15:16:36Z

src/main/java/racing/domain/Cars.java

+
+    private List<Car> collectCars(int carCount) {
+        List<Car> cars = new ArrayList<>();
+        while (remainCar(carCount--)) {


음 오히려 while 문을 사용해서 가독성이 떨어지는 것 같아요.
다들 많이 사용하는 for문 사용해도 좋을것 같아요 혹시라도 리터럴이 걱정이라면

for(int carIndex =0; carIndex < carCount; carIndex++)

이런식으로 평소 자주 사용하는 int i =0 보다 내용을 나타낼 수 있습니다. 아니면 IntStream을 사용하는 방법도 있구요!

jyami-kim · 2020-05-01T15:17:03Z

src/main/java/racing/domain/Cars.java

+import java.util.ArrayList;
+import java.util.List;
+
+public class Cars {


일급 컬렉션 굿b

jyami-kim · 2020-05-01T15:23:55Z

src/main/java/racing/domain/Car.java

+import racing.util.RandomUtil;
+
+public class Car {
+    private int distance;


엇근데 random으로 받은 distance 값을 바탕으로 outputview를 작성하는데,
이렇게 하면 주어진 요구사항에 맞지 않을 것 같습니다.
레이싱 게임 요구사항을 확인해주세요.

pci2676

소소한 리뷰를 남겼어!
이제 2단계로 넘어가보자!

pci2676 · 2020-05-01T11:06:14Z

src/main/java/racing/domain/Cars.java

@@ -0,0 +1,54 @@
+package racing.domain;
+
+import racing.view.ResultView;


도메인에 View 의존성이 존재하네요!
구조를 바꿔봅시다!

pci2676 · 2020-05-01T11:06:38Z

src/main/java/racing/domain/Cars.java

+    public void operate(int tryCount) {
+        ResultView resultView = new ResultView();
+        while (remainTryCount(tryCount--)) {
+            moveCars();
+            resultView.printResult(this);
+        }
+    }


뷰 의존성 제거해볼게요!

pci2676 · 2020-05-01T11:07:42Z

src/main/java/racing/domain/Cars.java

+        while (remainCar(carCount--)) {
+            cars.add(new Car(0));
+        }


제 생각에는 for문을 사용하는게 오히려 가독성에 좋을것 같아요

pci2676 · 2020-05-01T11:09:17Z

src/main/java/racing/domain/Car.java

+    public void move() {
+        this.distance += RandomUtil.getRandomDistance();
+    }


이 부분에 대한 테스트가 가능하게 의존성 분리를 해보는 것은 어떨까요?

pci2676 · 2020-05-10T14:48:26Z

src/main/java/racing/util/RandomNumberGenerator.java

+import java.util.Random;
+
+public class RandomNumberGenerator implements NumberGenerator {
+    private final int MAX = 10;


상수는 static을 붙여주는게 좋을것 같아!

pci2676 · 2020-05-10T14:49:29Z

src/main/java/racing/util/RandomUtil.java

@@ -0,0 +1,13 @@
+package racing.util;
+
+public class RandomUtil {


사용되는 메서드의 이름이 public static int getRandomDistance 인데 클래스의 이름이 RandomUtil인 것은 조금 어색한것 같아 의미를 더 드러낼수 있도록 네이밍을 수정해 보는건 어떨까?

pci2676 · 2020-05-10T14:52:14Z

src/main/java/racing/view/InputView.java

+
+    public int inputCarCount() {
+        System.out.println(FIRST_QUESTION);
+        return new InputCount(sc.nextInt()).getCount();


단순히 검증을 거쳤다 바깥으로 반환하는 용도로 사용하고 있는 객체인데
검증 로직을 한번 안쪽으로 숨긴것 밖에 안되는 것 같아!
객체 자체를 바깥으로 내보내서 사용하게 하는건 어떨까?

pci2676 · 2020-05-10T14:59:27Z

src/test/java/racing/domain/CarTest.java

+    @DisplayName("자동차가 랜덤으로 들어온 값이 4이상일 때 전진한다.")
+    @ParameterizedTest
+    @ValueSource(ints = {4, 9})
+    void move1(int distance) {
+        Car car = new Car(distance);
+        car.move(RandomUtil.getRandomDistance(new FixedNumberGenerator(distance)));
+        assertEquals(distance + 1, car.getDistance());
+    }
+
+    @DisplayName("자동차가 랜덤으로 들어온 값이 4 보다 작을 때 움직이지 않는다.")
+    @ParameterizedTest
+    @ValueSource(ints = {1, 2})
+    void move2(int distance) {
+        Car car = new Car(distance);
+        car.move(RandomUtil.getRandomDistance(new FixedNumberGenerator(distance)));
+        assertEquals(distance, car.getDistance());
+    }


두개의 테스트 코드를 하나로 합쳐보자!
@CsvSource를 잘 사용한다면 하나로 통합할 수 있어!

pci2676 · 2020-05-10T15:00:04Z

src/test/java/racing/util/RandomUtilTest.java

+    @DisplayName("랜덤으로 생성된 값이 4보다 작을경우 0을 리턴한다.")
+    @ParameterizedTest
+    @ValueSource(ints = {1, 3})
+    public void getRandomDistance1(int distance) {
+        int actual = RandomUtil.getRandomDistance(new FixedNumberGenerator(distance));
+        assertEquals(0, actual);
+    }
+
+    @DisplayName("랜덤으로 생성된 값이 4 이상을 경우 1을 리턴한다.")
+    @ParameterizedTest
+    @ValueSource(ints = {4, 5})
+    public void getRandomDistance2(int distance) {
+        int actual = RandomUtil.getRandomDistance(new FixedNumberGenerator(distance));
+        assertEquals(1, actual);
+    }


이것 또한 하나로 합칠수 있을것 같아!

pci2676 · 2020-05-10T15:02:11Z

src/main/java/racing/view/ResultView.java

+    private final String OPERATE_RESULT = "실행 결과";
+    private final String DISTANCE = "-";
+    private final String ENTER = "\n";
+    private final String BORDER = "라운드 종료!!";


매직넘버와 매직리터럴은 static을 붙여주는게 좋아!

jyami-kim · 2020-05-12T14:42:51Z

src/main/java/racing/domain/CarFactory.java

@@ -4,10 +4,10 @@
 import java.util.List;

 public class CarFactory {


굿 좋습니다b 기존코드와의 변경사항이 적어서 좋네요

jyami-kim · 2020-05-12T15:05:39Z

src/main/java/racing/domain/RacingResult.java

+        return racingResult
+                .stream()
+                .filter(car -> (car.getDistance() == maxDistance))
+                .map(car -> car.getName())


람다를 사용할 때 car::getName 와 같은 참조 사용방식도 있습니다! 바꾸라는게 아니라 그런 방식도 있다는 그런..ㅎㅎ

오 이렇게 또 하나 알아갑니다 ㅎㅎ 감사합니다!

jyami-kim · 2020-05-12T15:15:36Z

src/main/java/racing/domain/RacingResult.java


 public class RacingResult {
-    private final List<Integer> racingResult;
+    private final List<Car> racingResult;


여기서 RacingResult는 Cars 에서 움직임이 끝난 Car 객체들의 리스트를 받아서 수행을 하는데요..!!
기현님 의도가 Cars는 정말 게임을 실행하는 일급 컬렉션, RacingResult는 Cars에서 실행이 끝난 객체를 확인하는 일급 컬렉션으로 이해했습니다. (그런면에서 new Car() 를 이용해서 객체를 복사하는 효과를 낸 의도도 좋습니다b)

이때 RacingResult도 List를 담고있는데요, 이때 Car 대신에 다른 객체를 두면 좋을 것 같아요.
왜냐하면 현재 Car 객체에는 move라는 메서드가 있으니까..! Car 대신 Car의 내용(name, distance)을 담고있는 VO 객체를 주면 좋을 것 같습니다.

만약, 누군가가 RacingResult 클래스를 수정한다고 할 때. 현재 상태라면 RacingResult에서 racingResult.get(0).move() 와 같이 접근해서, List 에 있는 Car의 위치를 조작하는 메서드를 만들 수 있기 때문입니다.

따라서 위에서 RacingResult를 생성할 때 VO 객체를 만들어서, 이 객체 안에는 move와 같이, 객체의 상태를 변경하는 메서드를 제공하지 않아 불변 객체로 만드는 방식을 생각할 수 있을 것 같습니다. 만약 그 객체 이름이 CarData가 된다면

List<CarData> racingResulut; new CarData(car.name, car.distance); class final CarData{ }

위 코드가 살짝의 키워드가 될 수 있겠네요.

와 그렇네요. 좀 더 깊게 생각해야 될 거 같습니다! 감사합니다. 또 하나 배웁니다!

자동차 경주 게임!

ececbca

jyami-kim reviewed May 1, 2020

View reviewed changes

BangKiHyun added 2 commits May 3, 2020 11:56

List<Car>생성 분리, view와 domain 관계 분리, Random 테스트 구현

34ee1a3

CarFactory, moveCars메소드 테스트 코드 추가

e47e617

pci2676 requested changes May 10, 2020

View reviewed changes

자동차 경주 2단계 구현

8e3228a

jyami-kim reviewed May 12, 2020

View reviewed changes

BangKiHyun added 2 commits May 16, 2020 11:42

피드백 반영

deefa90

Winners 객체 추가, 각각의 라운드 결과값을 갖을 수 있도록 수정, 임계값 기준으로 테스트코드 수정

36c7945

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

자동차 경주 게임! #15

자동차 경주 게임! #15

BangKiHyun commented Apr 30, 2020

jyami-kim May 1, 2020

jyami-kim May 1, 2020

jyami-kim May 1, 2020

jyami-kim May 1, 2020

jyami-kim May 1, 2020

pci2676 left a comment

pci2676 May 1, 2020

pci2676 May 1, 2020

pci2676 May 1, 2020

pci2676 May 1, 2020

pci2676 May 10, 2020

pci2676 May 10, 2020

pci2676 May 10, 2020

pci2676 May 10, 2020

pci2676 May 10, 2020

pci2676 May 10, 2020

jyami-kim May 12, 2020

jyami-kim May 12, 2020

BangKiHyun May 13, 2020

jyami-kim May 12, 2020

BangKiHyun May 13, 2020

		@@ -0,0 +1,54 @@
		package racing.domain;

		import racing.view.ResultView;

		@@ -0,0 +1,13 @@
		package racing.util;

		public class RandomUtil {

		@@ -4,10 +4,10 @@
		import java.util.List;

		public class CarFactory {

자동차 경주 게임! #15

Are you sure you want to change the base?

자동차 경주 게임! #15

Conversation

BangKiHyun commented Apr 30, 2020

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

pci2676 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