🚀 5단계 - 자동차 경주(리팩토링) #5818

jonghyeonLeee · 2024-10-15T12:26:55Z

안녕하세요!

5단계를 진행했습니다.

인터페이스로 분리하는 부분이 계속.. 이해가 안됐지만 파라미터를 받는 방향으로 진행해봤습니다!!

4단계 피드백 적용 하였으며,
MVC 분리, 프리미티브 타입을 클래스로 구현, model에선 view와 controller를 상속 받지 않도록 최대한 분리해봤습니다!!

피드백 주시면 반영하도록 하겠습니다!

감사합니다~ :)

dhmin5693

안녕하세요.

이번 과제를 진행하면서 고민을 되게 많이 하신 것 같아요.
어려운 점 있으시면 언제든지 DM으로 질문해주세요.
몇 가지 피드백을 남겼으니 수정하시면서 꼭 참고 부탁드립니다.

dhmin5693 · 2024-10-15T16:03:37Z

src/main/java/com/racing/Main.java


-import java.util.ArrayList;
 import java.util.List;

 public class Main {


이번엔 Main의 책임을 일부 분리해보는건 어떨까요?
Main에서는 로직을 시작하는 정도의 책임만 가지는게 더 적절합니다.
그 자체로도 진입점 역할을 하고 있거든요.

9a3199f

메인 진입점을 분리했습니다!

dhmin5693 · 2024-10-15T16:04:55Z

src/main/java/com/racing/utils/RandomStrategy.java

@@ -1,5 +1,5 @@
 package com.racing.utils;

-public interface RandomMover {
+public interface RandomStrategy {


이 인터페이스명에서 Random을 언급하면 구체적인 방법을 명시하게 됩니다.
인터페이스는 방향을 "제시"하는 용도로만 사용하시는게 좋아요.

근데 이 인터페이스는 이미 방향뿐만 아니라 갈 길까지 전부 알려주는 느낌이네요.

81149cf

RandomStrategy -> MovementStrategy 로 변경했습니다!
인터페이스명에서 구체적인 내용을 제거하는 방향으로 수정했습니다.

dhmin5693 · 2024-10-15T16:06:35Z

src/main/java/com/racing/Main.java

-        for (Car car : carList) {
-            car.move();
+    private static void moveAllCars(Cars cars) {
+        RacingHelper racingHelper = new RacingHelper();


RacingHelper 보다는 더 괜찮은 이름으로 변경해주세요.
상위 인터페이스인 RandomStrategy 를 연상해내기 어렵습니다.
인터페이스의 이름이 이미 하위 구현체의 역할을 정해버려서 그런 것일수도 있어요.

그리고 하위 구현체를 그대로 사용하면 인터페이스를 통한 약결합이라는 장점을 가져갈 수 없습니다.
구현체가 아니라 인터페이스를 활용하는 방법으로 바꿔보세요.

방법은 여러가지가 있습니다.

Car 의 생성자에서 RandomStrategy 를 주입받아 사용합니다.

moveAllCars 에서 직접 사용하지 않고 매개변수 형태로 RandomStrategy을 사용합니다.

이렇게 되면 moveAllCars에서 직접 생성하지 않고 외부로부터 주입받는 형태가 됩니다.

따라서 사용자의 필요에 따라 다른 구현체로 바꾸어 사용할 수 있습니다.

47bdbbc

RacingHelper는 불명확한 부분이 있어 차의 움직임을 나타내는 CarMovement로 변경했습니다.

moveAllCars의 파라미터로 받아오는건 이해하였지만 구현을 어떻게 할지 도저히 생각나지 않아 수정하지 못했슷빈다.. ㅜㅜ
혹시 시간 나실 때 페어코딩이나 다른 도움을 받을 수 있는 부분이 있을지 문의드립니다!

service의 필드에 CarMovement 를 필드로 가집니다. 그럼 service를 초기화할 때 전략을 선택적으로 밀어넣을 수 있습니다.

moveAllCars의 매개변수가 지금 cars 하나만 있는데 cars, movementStrategy 2개로 해두시면 이 메소드를 호출할 때 전략을 선택적으로 사용할 수 있습니다.

전략 패턴의 기본은 언제든지 전략을 바꾸어 사용할 수 있어야 한다는 것임을 생각해보시면 도움이 될 수 있습니다.

dhmin5693 · 2024-10-15T16:07:49Z

src/main/java/com/racing/domain/CarName.java

+
+import java.util.Objects;
+
+public class CarName {


원자 단위의 책임 분리 👍

dhmin5693 · 2024-10-15T16:07:55Z

src/main/java/com/racing/domain/Position.java

+
+import java.util.Objects;
+
+public class Position {


dhmin5693 · 2024-10-15T16:08:55Z

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

+    public void printRacingPositions() {
+        for (Car car : carList) {
+            System.out.println(car.displayRacingPosition());
+        }
+    }


domain에서는 sout 을 사용하시면 UI를 분리한 의미가 퇴색됩니다.

sout은 UI에서만 사용해주세요.

16ee9f8

해당 내용 적용했습니다 :)

dhmin5693 · 2024-10-15T16:09:29Z

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

+import java.util.stream.Collectors;
+
+public class Cars {
+    List<Car> carList;


private이 아닌 이유가 있을까요?

그리고 List와 같은 자료형은 필요에 따라 언제든지 변경할 수 있습니다.
예를 들면 Set으로 바꿀 수 있겠죠?

따라서 carList보다는 복수형 용어를 사용하시는걸 추천드립니다.
Cars처럼 일급컬렉션인 경우에는 items 같은 용어를 사용해도 나쁘지 않아요.

be64fc4

private로 필드를 막았어야 했는데 빼먹었네요 ㅜㅜ 수정했습니다.
items로 변경할 경우 Cars.getItems() 와 같이 일반적인 메서드로 표현할 수 있어 가독성에서도 좋아 보여 수정했습니다.

중복된 값에 대한 처리 여부가 문제에 명시되지 않아 습관적으로 List를 사용했는데 추후 관련된 내용이 있으면 Set으로 리팩토링 해보겠습니다! :)

dhmin5693 · 2024-10-15T16:10:34Z

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

+    public Cars(List<Car> carList) {
+        this.carList = carList;
+    }


방어적 복사 기법에 대해 공부해보시면 좋습니다.

Cars를 생성한 위치 주변에서 Cars를 생성하는데 사용한 List를 변경하면 Cars도 영향을 받게 됩니다.

13b1a3c

방어적 복사 기법 공부 후 외부에서 변경하지 못하도록 해당 내용 반영 했습니다.

dhmin5693 · 2024-10-15T16:13:22Z

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

+    public List<Car> getCarList() {
+        return this.carList;
+    }


Suggested change

public List<Car> getCarList() {

return this.carList;

}

public List<Car> getCarList() {

return Collections.unmodifiableList(this.carList);

}

읽기 전용 데이터로 변경해서 내보내셔야 외부의 영향을 받지 않게 됩니다.
getCarList().add(car) 를 수행하면 Cars가 변경되어 버립니다.

dhmin5693 · 2024-10-15T16:14:22Z

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

+        return this.carList;
+    }
+
+    public int findMaxPosition() {


private 메소드여도 괜찮지 않을까요?

0c567b0

작성할 때 직접 테스트를 하였는데 추후 간접 테스트를 하고 있다 판단하여 private 변경 후 테스트 코드 제거했습니다!
일반적으로 이런식으로 처리하는지 궁금합니다!

private 도 테스트가 필요할 만큼 복잡한 순간도 분명 있지만, public을 통해 간접적으로 테스트하는게 방법 자체가 더 올바릅니다.

private을 굳이 따지자면 public의 코드를 따로 접어둔 느낌이니까요.

jonghyeonLeee · 2024-10-22T10:11:32Z

안녕하세요!
피드백 내용 반영하여 커밋했습니다!
감사합니다!

dhmin5693

안녕하세요.

마지막 미션이라 일단 수정 요청 한번 더 드려보는데요,
시간 상 다음 미션 수행하시는게 더 도움이 될 것 같기는 합니다.

일단 남긴 피드백 확인해보시고 수정이 필요없다고 생각하시거나 다음 미션 진행하고 싶으시면 수정없이 바로 요청주셔도 괜찮습니다.

어려운 점 있으실 때 또 DM주시면 상세히 답변드리겠습니다.

dhmin5693 · 2024-10-22T15:05:36Z

src/main/java/com/racing/service/RacingService.java

+import java.util.List;
+
+public class RacingService {
+    public static void startRacingGame() {


startRacingGame에서 input까지 다 하기보다는 input은 미리 받은 뒤 dto 등으로 넘기면 조금 더 깔끔하게 구현할 수 있을 것 같네요.

dhmin5693 · 2024-10-22T15:08:02Z

src/main/java/com/racing/service/RacingService.java

+    private static void advanceAllCars(Cars cars) {
+        RacingHelper racingHelper = new RacingHelper();
+
+        for (Car car : cars.getCarList()) {
+            car.move(racingHelper.shouldMove());
+        }
+    }


startRacingGame에서 직접 input을 수행하지 않고 Main(혹은 다른 클래스)에서 input을 수행한 다음 dto로 보낸다면 어떨까요?

startRacingGame 자체 테스트가 지금은 불가능한데 가능해질 수 있습니다.

하지만 아직 걸림돌이 하나 있는데 advanceAllCars 에서 직접적으로 Random 로직에 의존하고 있어요.
DM으로 말씀드렸듯 여기서 이렇게 직접 사용하기 때문에 startRacingGame 가 불가능해 지는 것입니다.

RacingService 의 필드로 Strategy를 가져가고 사용하는 방법도 있으니 한 번 고려해보세요.

dhmin5693 · 2024-10-22T15:11:07Z

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

+        return this.carList;
+    }
+
+    public int findMaxPosition() {


private 도 테스트가 필요할 만큼 복잡한 순간도 분명 있지만, public을 통해 간접적으로 테스트하는게 방법 자체가 더 올바릅니다.

private을 굳이 따지자면 public의 코드를 따로 접어둔 느낌이니까요.

dhmin5693 · 2024-10-22T15:16:38Z

src/main/java/com/racing/utils/CarMovement.java

@@ -5,7 +5,7 @@
 /**
 * 레이싱에서 사용 할 헬퍼 메서드를 모아놓은 클래스
 */
-public class RacingHelper implements MovementStrategy {
+public class CarMovement implements MovementStrategy {


MoveStrategy는 적절한 명칭이 맞는데요,
CarMovement 보다는 RandomMovementStrategy는 어떠신지요?

저번에도 남긴 부분인데 구현체와 인터페이스는 이름으로도 연관이 있어보여야 좋습니다.

dhmin5693 · 2024-10-22T15:19:11Z

src/main/java/com/racing/Main.java

-        for (Car car : carList) {
-            car.move();
+    private static void moveAllCars(Cars cars) {
+        RacingHelper racingHelper = new RacingHelper();


service의 필드에 CarMovement 를 필드로 가집니다. 그럼 service를 초기화할 때 전략을 선택적으로 밀어넣을 수 있습니다.

moveAllCars의 매개변수가 지금 cars 하나만 있는데 cars, movementStrategy 2개로 해두시면 이 메소드를 호출할 때 전략을 선택적으로 사용할 수 있습니다.

전략 패턴의 기본은 언제든지 전략을 바꾸어 사용할 수 있어야 한다는 것임을 생각해보시면 도움이 될 수 있습니다.

jonghyeon added 24 commits October 15, 2024 19:32

refactor: state를 position으로 변경

e66ce53

refactor: random 인터페이스에 대한 명칭을 추상적으로 변경

065e71c

feat: Cars 일급 콜랙션 구현

90057c1

refactor: 코드 정리

d905f4a

refactor: Random을 controller 단에서 제어하도록 수정

9d3b593

refactor: 외부에서 사용하지 않는 값의 접근 제어자 변경

99749ca

refactor: 간접 테스트 제거

f6053fa

feat: position 구현

8d16b5f

feat: position 생성자 추가 구현

516281d

feat: postion add, get 함수 구현

1332ebf

refator: 프리미티브 타입의 position을 객체로 변경

f0c3b6a

refactor: 대시를 반환할 때 position 객체의 메서드를 사용하도록 기능 수정

ed49ad8

feat: 큰 값인지 여부를 구분하여 반환하는 함수 구현 및 car 메서드 대체

2591e7f

refator: 간접 테스트 제거

2a99564

refactor: 사용하지 않는 코드 제거

c01dacd

refactor: cars 일급 콜랙션에 car 객체를 추가하는 코드 구현

ff2a4e7

refactor: List로 선언된 파라미터, 반환 값을 Cars로 변경

8533d99

refactor: cars 를 사용하도록 리팩토링 진행

e386be1

feat: carName 객체 구현

4fd6712

refactor: String 프리미티브 타입에서 CarName 객체를 사용하도록 수정

7651e0c

refactor: cars에서 maxposition을 반환하도록 리팩토링 진행

094b6be

refactor: Cars 객체 내부에서 우승자를 결정하는 함수를 반환하도록 리팩토링

0bee42d

refactor: carSize를 반환하는 매서드 구현

8e94225

refactor: resultview에서 사용하는 함수를 Cars 클래스 내부에서 구현하여 테스트 하도록 수정

1643450

dhmin5693 requested changes Oct 15, 2024

View reviewed changes

jonghyeon added 5 commits October 22, 2024 18:55

refactor: 메인 함수는 시작점 역할만 하고 실제 서비스(컨트롤)는 분리, 함수명을 명확하게 수정

9a3199f

refactor: sout을 view 단으로 분리, 사용하지 않은 테스트 코드 제거

16ee9f8

refactor: carlist -> caritems로 네이밍 수정, 일급 객체의 item을 private로 선언

be64fc4

refactor: 방어적 복사 기법 사용하여 get으로 리턴받은 값을 변경할 수 없도록 수정

13b1a3c

refactor: 간접 테스트를 통한 private 함수 테스트, 기존 테스트 제거

0c567b0

fix: 테스트 오류 수정

8e329e8

jonghyeon added 2 commits October 22, 2024 19:33

refactor: 인터페이스에서 구체적인 내용을 방향성을 나타내도록 변경

81149cf

refactor: 자동차 움직임에 해당하는 객체를 명확하게 표현

47bdbbc

dhmin5693 requested changes Oct 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 5단계 - 자동차 경주(리팩토링) #5818

🚀 5단계 - 자동차 경주(리팩토링) #5818

jonghyeonLeee commented Oct 15, 2024

dhmin5693 left a comment

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024 •

edited

Loading

dhmin5693 Oct 22, 2024

dhmin5693 Oct 15, 2024

dhmin5693 Oct 15, 2024

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024

dhmin5693 Oct 15, 2024

dhmin5693 Oct 15, 2024

jonghyeonLeee Oct 22, 2024 •

edited

Loading

dhmin5693 Oct 22, 2024

jonghyeonLeee commented Oct 22, 2024

dhmin5693 left a comment

dhmin5693 Oct 22, 2024

dhmin5693 Oct 22, 2024

dhmin5693 Oct 22, 2024

dhmin5693 Oct 22, 2024

dhmin5693 Oct 22, 2024

🚀 5단계 - 자동차 경주(리팩토링) #5818

Are you sure you want to change the base?

🚀 5단계 - 자동차 경주(리팩토링) #5818

Conversation

jonghyeonLeee commented Oct 15, 2024

dhmin5693 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

jonghyeonLeee Oct 22, 2024 • edited Loading

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

jonghyeonLeee Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonghyeonLeee commented Oct 22, 2024

dhmin5693 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

jonghyeonLeee Oct 22, 2024 •

edited

Loading

jonghyeonLeee Oct 22, 2024 •

edited

Loading