[우테코 5기] 자동차 경주 게임 회고

자동차 경주 게임은 저번 주에 끝났지만... 새로운 미션에 적응하느라 이제서야 회고를 작성한다... 🥹

준팍이랑 진행한 첫 페어프로그래밍이다. 

하나의 노트북으로 같이 코딩하는 건 처음 해보는 경험이라 색달랐다.

지금 와서 느끼는 거지만, 혼자 코딩할 때보다 시간은 더 걸려도 페어에게 부끄럽지 않은 코드를 작성하기 위해서 더 좋은 방향으로 설계하기 위해서 의식적으로 노력하게 되는 것 같다.

회고를 어떻게 작성할까 고민했는데, 코드는 단계별로 branch가 나뉘어져 있고, 기능 목록도 더 상세하게 작성되어 있기 때문에 코드에 대한 설명보다는 어떤 식으로 문제를 해결했는지 적는 게 더 좋을 것 같아서 그렇게 적어보려고 한다 :D

 

✔️ 작성한 코드 

GitHub - Cl8D/java-racingcar: 자동차 경주 게임 미션 저장소

✔️ 1차 PR

[1단계 - 자동차 경주 구현] 져니(이지원) 미션 제출합니다. by Cl8D · Pull Request #455 · woowacourse/java-

✔️ 2차 PR 

[2단계 - 자동차 경주 리팩터링] 져니(이지원) 미션 제출합니다. by Cl8D · Pull Request #619 · woowacourse/

---

✔️ 기능 요구사항

- 주어진 횟수 동안 n대의 자동차는 전진 또는 멈출 수 있다.
- 각 자동차에 이름을 부여할 수 있다. 전진하는 자동차를 출력할 때 자동차 이름을 같이 출력한다.
- 자동차 이름은 쉼표(,)를 기준으로 구분하며 이름은 5자 이하만 가능하다.
- 사용자는 몇 번의 이동을 할 것인지를 입력할 수 있어야 한다.
- 전진하는 조건은 0에서 9 사이에서 random 값을 구한 후 random 값이 4 이상일 경우 전진하고, 3 이하의 값이면 멈춘다.
- 자동차 경주 게임을 완료한 후 누가 우승했는지를 알려준다. 우승자는 한 명 이상일 수 있다.

---

✔️ 1차 페어 프로그래밍

당시에는 TDD가 그냥 테스트 코드만 작성하면 되는 건 줄 알고 기능 구현부터 다 하고 나서 테스트 코드를 뒤늦게 작성했었다.

그래서 커밋 내역을 보면 이렇다. 누가 봐도 기능 먼저 구현하고 테스트 구현한 사람... 🥹

그래도 당시 작성한 일기를 보면 최대한 이런 내용을 구현했다.

 

1. 도메인에서 getter 사용하지 않기

2. DTO를 통한 데이터 전달

3. Stream을 활용하여 filter를 통해 조건 검증

4. private 메서드 테스트 -> 리플랙션 사용하기 (...)

5. 자동차 이름에 대한 equals, hashcode 오버라이딩을 통해 자동차 객체 관리하기

 

지금 와서 생각해보면 정말 부끄럽다. 도메인에서 Getter를 무작정 지양하고, DTO 무작정 사용하고, 무엇보다 테스트 코드를 리플래견까지 사용하면서 처리하는 건 낭비이기 때문이다. (당시에는 이거 구현하고 엄청 뿌듯했다. 원래 부끄러운 감정이 성장했다는 증거라고 했다...ㅎㅎ)

---

✔️ 1차 PR 

준팍과 같이 피드백을 합쳐서 서로 코드에 대한 문제점을 정리하기로 했다.

 

💬 뷰의 출력을 도메인에서 관리한 것

public class Car {

    private final String name;

    private final StringBuilder drivingMark;

    private Car(final String name) {
        ...
    }

    public static Car of(final String name) {
        return new Car(name);
    }

    public void move() {
        drivingMark.append("-");
    }
    ...
}

만약 뷰에 대한 출력 조건이 변경된다면 도메인도 함께 변경된다는 단점이 있다. 비교적 변경이 잦은 부분인 출력 조건에 대해서 이렇게 도메인과 연관을 지으면 안 된다 🥲  (도메인도 함께 변하게 되니까) 그래서 추후 리팩토링 때 자동차는 position만 가지고 있도록 만들고, 출력에 대한 작업은 view에서 처리하도록 변경하였다.

 

또한, toString()을 통해서 당시에는 자동차 이름과 주행거리를 출력하려고 했으나 (사실 프리코스 때 그렇게 해왔던 게 버릇이 되었다.) 이 부분도 리뷰어님이 toString()의 목적성에 대해 생각해 보라고 하셔서 다음과 같이 정의내렸다.

toString()을 재정의하는 건 좋지만, 보통 필드의 내용이 다 나와야 하면서, 로깅 용도로 사용하기 때문에 우리처럼 사용자에게 보여지는 용도 (= 뷰에서 처리할 일)로 사용하는 거는 맞게 쓰이지 않은 것이었다.

 

 

💬 역할과 책임의 분리

public class InputUtil {

    private final InputView inputView = new InputView();

    public <T> T getUserInput(Supplier<T> inputReader) {
        try {
            return inputReader.get();
        } catch (IllegalArgumentException | DuplicateException | ConsoleException e) {
            OutputView.printMessage(e.getMessage());
            return getUserInput(inputReader);
        }
    }

    public Cars getCarNames() {
        OutputView.printMessage(GET_CAR_NAMES_MESSAGE.getValue());
        String carNames = inputView.getUserInput();
        return Cars.of(carNames);
    }
    ...
}

1차 때는 InputUtil을 별도로 두어서 사용자에게 재입력을 받도록 했는데, (사실 유틸 클래스는 이렇게 사용하는 게 아니다. 보통 유틸 클래스는 static 메서드나 필드만 관리하는 클래스인데 당시에는 잘 모르고 썼다🥹 ) getCarNames를 보면, 뷰에서 도메인을 생성하는 역할까지 하고 있는 것이었다!

 

우선 InputUtil을 만들었던 이유부터 생각했다. 나는 InputView를 위한 진입점으로 InputUtil을 두었었는데, 이는 InputView와 OutputView를 이곳 하나에서 참조하도록 하고 싶어서였다.

근데 글로 쓰니까 길어보이지만 결론은 그거였다. 역할과 책임을 제대로 분리하지 못한 결과로 나온 해프닝... InputView에서 충분히 처리가 가능한 내용이었고, 단지 inputView에서 콘솔에 대한 입력 메서드만 두고 싶어서 그랬던 것이었다. 

피드백을 듣고 보니 오히려 지우는 게 깔끔할 것 같아서 과감하게 지웠고, view에서 도메인을 생성하던 로직도 변경하여 컨트롤러에서 생성하도록 진행하였다. 이 피드백 덕분에 도메인과 뷰를 나눠야 하는 확실한 이유를 제대로 알게 된 계기였다. 👍

 

 

💬 무분별한 DTO 생성

사실 웹 개발에 익숙해져있던 나는, DTO를 생성하는 게 너무 당연하게 느껴졌고 자연스럽게 사용했다. 하지만, 왜 사용해야 하는지는 한 번도 고려한 적 없이 사용했던 것이었다 🥹 그래서 왜 DTO를 사용하셨냐는 질문에 대해 쉽게 답변할 수 없었다.

현재 Car에서는 주행 결과에 대한 드라이빙 마크(-)를 가지고 있기 때문에, 해당 마크의 길이만 필요해서 DTO를 통해 포장하여 사용하려고 하였습니다! 물론 내부적으로 getter를 사용하여 .length()를 통해 가져올 수 있는 부분이었지만, getter는 해당 필드의 값 자체를 리턴하는 것이기 때문에 목적에 부합하지 않다고 판단하였고, 별도의 클래스로 선언하면 네이밍을 통해 해당 값이 어떤 것을 의미하는지 더 잘 나타낼 수 있다고 생각했습니다.

다만 리팩토링 과정에서 해당 필드 대신에 position 값을 가질 것이기 때문에 원시값을 포장하기 위해서 ‘dto’라는 네이밍 대신에 해당 값을 포장하는 ‘CarPosition’ 등의 클래스로 다시 리팩토링 하고자 합니다. 

추가적으로, 원시값 포장을 위해서 차 이름 역시 별도로 분리하여 포장해서 validation을 해당 포장 클래스에서 진행하도록 해서 도메인에 대한 역할을 줄이는 방향으로 설계해보고자 합니다! (추후 필드가 추가되고, 제약 사항이 추가되면 Car 도메인 하나에서 너무 많은 역할을 할 것 같다는 생각이 들었습니다 :D)

당시에는 이렇게 답변했지만, 사실 도메인에서 DTO를 반환하고 있었기 때문에 도메인의 필드를 getter를 사용하지 않고 접근할 수 있다는 이유로 사용하였다. 하지만! 도메인에서 DTO를 참조하는 것도 좋지 않다는 사실을... 1차 리팩토링 이후에 알게 되었다. 이거는 다시 언급할 예정! 아무튼, '원시값 포장'이라는 명목하에 이때까지는 '왜 DTO를 쓰면 안 돼...?' 이러고 있었다. ㅎㅎ

 

 

💬 컨트롤러의 인터페이스

당시 컨트롤러에 대한 인터페이스를 구현했었는데, 사실 컨트롤러가 하나여서 굉장히 무의미했다. ㅎㅎ 그래서 다음 리팩터링 때 과감하게 삭제하였다. 삭제하는 겸 인터페이스에 대해서 공부를 했었는데, 그 내용을 블로그에 정리하면 좋을 것 같아서 여기에 적어둔다.

 

 

💬 Enum 활용, 하지만 어디까지...?

기존에는 'DomainConstant'이라는 enum 통해서 도메인의 제약사항을 모두 여기서 관리했다. 하지만, 도메인이 커진다면...? 그렇다면 관리해야 하는 필드가 매우 많아지게 된다. 그래서 다음 리팩터링 때는 enum 구조를 그대로 사용하고 싶어서, 도메인별로 constant enum을 분리하였다. (하지만 이 구조도 지금 와서 보면 아쉽다.)

 

당시, 왜 enum으로 상수를 관리했는지에 대해 다음과 같이 생각했었다.

저는 기본적으로 상수를 enum으로 관리하는 편인데, enum으로 관리하면서 좋았던 점은 다음과 같이 말씀드릴 수 있을 것 같아요!

1. 입출력 메시지의 경우 사용자에게 보여지는 메시지인 만큼 변화가 자주 일어날 수 있는 부분이라고 생각이 들어요! 이때, 해당 메시지를 다양한 곳에서 참조하는 경우 모든 곳의 코드를 바꿔줘야 하는데 enum으로 관리해주면 한 곳에서 관리할 수 있어서 관리 차원에서 용이하다는 생각이 들어요.

2. 도메인 제약 사항의 경우, 이번 과제에서는 대부분 숫자값을 가지고 있는데 (흔히 ‘매직 넘버’라고 하죠!) 해당 숫자값에 대해서 그대로 코드에 사용하게 되면 코드를 읽는 입장에서 해당 넘버가 무슨 의미를 가지고 있는지 한 눈에 알아보기가 힘들 것이라고 생각했어요. 그래서 enum을 통해 해당 숫자에 대해서 별도의 명칭으로 관리하고, 또 마찬가지로 변경이 용이할 수 있도록 한 곳에서 관리하려고 사용하는 것 같아요!

다만, 현재는 도메인 제약사항 자체를 하나의 ‘DomainConstant’에서 관리하다 보니까, 추후 확장성을 고려해봤을 때 도메인의 크기가 커지게 된다면 내부에 들어가야 하는 값이 많아져서 복잡도가 증가할 것 같아요. 그래서 이번 리팩토링 때 제약사항들은 각 도메인에 맞게 한 번 더 분리 작업을 진행해보려고 합니다!

또한, 도메인 내에서 상수를 static final로 관리한다면 내부적으로 조금 더 코드를 작성하는 입장에서는 덜 귀찮을(?) 수도 있을 것 같고, 단순히 상수가 아니라 enum 역시 별도의 타입(이라고 표현해도 될지 모르겠지만)이기 때문에 메모리를 더 잡아먹을 것 같아요…!

하지만, 지금 와서 생각해보면 static final로 관리하는 게 더 좋을 것 같다. ㅎㅎ

 

 

💬 테스트 코드

@ParameterizedTest
@ValueSource(ints = {4, 9})
@DisplayName("랜덤값이 4 이상이면 자동차가 전진한다.")
void givenFourMoreNumber_thenCarMove(int randomNumber) throws Exception {
        // given
        Cars cars = TestProvider.createTestCars();
        Car car = TestProvider.createTestCar("pobi");

        // when
        Method method = cars.getClass().getDeclaredMethod("checkNumberAndMove", Car.class, int.class);
        method.setAccessible(true);
        method.invoke(cars, car, randomNumber);

        // then
        assertThat(car)
        .isEqualTo(TestProvider.createTestMoveCar(car));
}

아마 많이들 고민을 했던 부분일 것이다. 테스트하기 어려운 부분 (=랜덤으로 값을 생성하는 부분)에 대해서 어떤 식으로 테스트를 할 것인지 분리를 해야 하는 상황이다. 기존에는 테스트 하기 어렵기 때문에 그냥 리플랙션으로 아예 값을 넣어줬다. 하지만, 이런 테스트 방식은 지나치게 복잡하다는 생각이 아무래도 개발자 입장에서 들 수밖에 없고, 무엇보다 '자동차의 움직임'을 테스트하기 위해서, Cars까지 생성하는 건 역할과 책임의 분리가 제대로 이루어지지 않은 것이라고 볼 수 있다.

그래서 리팩터링 때 가장 신경쓰면서 수정한 부분이다. 테스트하기 어려운 부분은 외부로 빼기 위해 인터페이스에 대한 Mock 클래스를 생성하여 진행하였다.

---

✔️ 1차 리팩터링

1차 리팩터링 때는 거의 갈아엎듯이 엄청 많이 수정했다. 크게 뽑아보자면, 다음과 같은 항목을 수정했다.

- private method의 테스트는 최대한 public을 통해 진행하도록 만들기
- 각 객체에 대한 역할을 확실하게 하기 (ex. 뷰에서 Cars, Race 같은 도메인을 생성하지 않도록 만들기)
- Wrapper 클래스 활용하기 (각 필드에 대한 검증도 wrapper에서 진행하기)
- 정적 팩터리 메서드 이용하기
- 화면 출력은 view에서 진행하도록 진행하기
- 메서드의 순서 변경하기 

우선 가장 가벼운 메서드의 순서의 경우, 다음과 같은 순서를 지키려고 했다.

클래스 > 정적 변수 (public-protected-private) > 인스턴스 변수 > 생성자 > 정적 메서드 > 메서드 > 스탠다드 메서드 (toString, equals, hashcode...) > getter / setter 메서드.

 

이때, public이랑 private 메서드 사이의 순서를 고민했었는데 public을 먼저 선언하는 방향으로 진행했다. private를 public 사이에 선언하는 것을 선호하는 분들도 계시겠지만, public을 먼저 둬야 해당 객체가 하는 일을 명확하게 볼 수 있기 때문에 (=public으로 선언된 메서드만 바로 확인하면 되니까) 나는 public을 먼저 작성하는 방향으로 진행했다.

 

 

💬 테스트 코드 쉽게 만들기

public class Cars {

    private final List<Car> cars;

    private final NumberGenerator numberGenerator;

    private Cars(final String carNames, final NumberGenerator numberGenerator) {
        this.cars = createCars(carNames);
        this.numberGenerator = numberGenerator;
        validateDuplicateCarName();
    }
}

테스트하기 어려운 부분을 나누기 위해서, 기존에는 NumberGenerator를 필드에서 바로 주입했었지만, 의존 관계를 주입하는 형식으로 변경하였다. (인터페이스 주입) 이 덕분에 실제 프로그램에서는 RaceNumberGenerator를 통해 랜덤값을 가져오도록 하고, 테스트 코드에서는 MockNumberGenerator를 주입받아서 원하는 값만 계속해서 리턴받을 수 있도록 진행하였다.

public class MockNumberGenerator implements NumberGenerator {

    private int randomNumber = 0;

    private final boolean isIncrease;

    public MockNumberGenerator(final boolean isIncrease) {
        this.isIncrease = isIncrease;
    }

    @Override
    public int generate() {
        if (isIncrease) {
            return randomNumber = randomNumber * 2 + 1;
        }
        return randomNumber;
    }
}

    @Test
@DisplayName("우승한 자동차를 뽑는다.")
    void givenCarInfo_thenPickWinner() {
            // given
            MockNumberGenerator numberGenerator = TestProvider.createMockNumberGenerator(true);
            Cars testCars = TestProvider.createTestCars(testCarNames, numberGenerator);

            // when
            testCars.race();
            List<String> winners = testCars.pickWinners();

            // then
            assertThat(winners.size())
            .isEqualTo(1);

            assertThat(winners)
            .isEqualTo(List.of("honux"));
            }

덕분에 우승한 자동차를 뽑는 로직에서 1, 3, 5... 이런 식으로 값을 반환받도록 하여 자동차 3개가 들어왔을 때 마지막 자동차만 주행을 하게 되어서 (4 이상이 주행 조건)  우승한 자동차를 뽑기가 쉬워졌다.

// Car.java
public void move(final int power) {
        if (power >= CAR_FORWARD_NUMBER.getValue()) {
        position.addPosition();
        }
}

또한, 기존에는 자동차를 이동시키는 로직을 외부에서 판단해서, 자동차에서는 move()만 진행하도록 만들었었는데, 자동차에게 랜덤값 (=power)을 주고 직접 움직임을 판단하도록 해서, 매개변수를 통해 자동차에게 랜덤값을 주입시킬 수 있었기 때문에 자동차에 대한 테스트 코드 역시 쉽게 작성할 수 있었다.

 

 

💬 Wrapper 클래스 활용하기

Wrapper 클래스를 사용하면서, 도메인의 모든 필드마다 Wrapper가 있어야 하는지에 대한 의문이 있었다. 지금은 미션이기 때문에 이와 같이 다 감싸는 식으로 진행했지만, 추후에는 비즈니스 로직이 없는 경우에는 감싸지 않고, 검증 로직 같은, 별도의 로직이 필요하다면 감싸서 처리하는 게 좋다고 느꼈다. 

 

 

💬 정적 팩터리 메서드 생성하기

private Car(final String name) {
        this.name = CarName.of(name);
        this.position = CarPosition.of();
        }

정적 팩터리 메서드의 네이밍에 대해서 고민하게 된 계기였다. 지금까지 프로젝트를 진행하면서 이 레퍼런스를 참고해서 진행했었다. 그래서 대부분 of로 진행했었는데, 이는 파라미터로 넘어온 값들에 대해 검증하여 인스턴스를 생성할 때 - 혹은 집계 메서드에서 주로 사용한다고 한다. (ex. List.of) 그래서 create를 사용하여 새로운 인스턴스를 생성하는 것을 나타내도록 변경하였다. 😎

 

 

💬 도메인에서 DTO를 생성하게 된다면...? 

 public List<CarRaceDto> initStatus() {
        return cars.stream()
        .map(Car::getCarRaceResult)
        .collect(Collectors.toList());
}

위의 코드는 Cars에서 초기 자동차의 상태를 출력해주는 코드이다. 그러나, 도메인 내부에서 DTO를 생성하고 있다는 점이다.

도메인 내부에서 Dto를 생성하게 된다면, Dto의 생성자가 변하게 되었을 때 도메인 코드 역시 함께 변경된다는 단점이 존재해요. Dto의 경우 도메인에 비해 변경이 잦은 부분이기 때문에, 도메인 로직 역시 함께 변경된다는 점이 좋지 않다고 생각이 들어요. 또한, DTO는 이름 그대로 Data Transfer Object로서, 계층 간의 데이터를 전달해주는 역할을 하는 객체인데 도메인이 DTO를 의존하게 되는 건 이상하다고 생각이 들어요.

장점이라고 한다면, 외부에서 getter를 통해 값을 접근하지 않고 내부의 필드값을 통해서 바로 DTO를 생성할 수 있다는 장점이 있을 것 같아요. (getter를 지양한다는 관점 하에서...! 이 부분은 생각해봐야 될 문제 같아요. 지금까지 너무 getter를 무작정으로 안 쓰려고 했던 것 같아서요. ㅎㅎ)

하지만 전자의 경우가 리팩토링이 더 필요한 부분이라고 생각이 들어요. 별도의 서비스 계층을 만들어서 DTO를 반환하도록 하고, 이 결과를 컨트롤러에서 사용하도록 하는 게 더 좋은 구조가 아닐까… 라는 생각이 들었어요. 리팩토링 하면서 한 번 수정해보겠습니다! 😊

그래서 2차 리팩토링 시, 서비스 계층을 활용하여 DTO를 반환하도록 생성하였다. (하지만, 지금 와서 생각해보면 그냥 getter로 도메인 객체 자체를 넘겨주고 뷰에서 처리하도록 하는 게 더 깔끔했을 것 같다. ㅎㅎ 지금 상태로는 너무 헤비한 로직이라는 생각이 들었다.)

 

 

💬 Getter를 지양하는 이유

당시 구현할 때 도메인의 getter를 굉장히 지양하려고 했었다. 처음에는 별이유 없이 'getter를 지양해야 돼!'라는 말 한 마디만 듣고, 리뷰어님께 질문을 듣고 다음과 같은 결론을 내렸었다.

객체를 더 객체스럽게 사용하기 위해서는, 객체에게 값을 받아오는 것이 아닌 객체에게 직접 질문을 던지는 형태로 이루어져야 된다고 생각하기 때문이에요. 객체가 가지고 있는 값을 객체 스스로 판단하고, 외부에 대해서 최대한 영향을 덜 받도록 하고 싶어서 getter / setter를 지양하려고 하는 편이에요.

하지만, 생각해 보니까 무작정으로 getter를 지양하는 것도 별로 좋은 설계라는 생각이 들지 않았어요. 특히 이전처럼 getter를 사용하지 않기 위해서 도메인 단에서 DTO를 리턴하는 등의 행위는 오히려 객체 간의 결합도를 증가시키는 행위라고 생각이 들었어요...! 그래서 저만의 작은 기준을 세웠는데, 출력을 위해 필요하거나 DTO 변환 등에서는 getter의 사용이 필수적이라고 생각이 되기 때문에 이런 경우에는 사용하는 게 더 낫다고 생각이 들어요!

그래서 이번 리팩토링 때는 cars에 대한 getter를 사용하면서 DTO 생성을 도메인에서 처리하지 않도록 변경하였어요. 또한, unmodifableList를 통해서 외부에서 변경하지 않도록 진행했습니다! 😊 이런 식으로 getter를 사용하는 건 괜찮을까요?

1차 미션을 완전히 끝낸 지금은, DTO를 사용하지 않는 구조를 사용하고 있지만 당시에 내릴 수 있던 최선의 결론이었던 것 같다.

 

---

✔️ 2차 리팩터링

2차 리팩터링에서는 기간이 그렇게 길지 않았어서, 다음과 같이 수정하였다.

- 도메인에서 DTO를 참조하지 않도록 개선하기.
- Cars / Race에 대한 Service Layer 추가하기. (컨트롤러에서 도메인 대신 서비스를 참조하도록 변경하기)
- 하나의 메서드는 하나의 일만 하도록 분리하기, 변수명 리팩토링 하기.
- 서비스에 대한 테스트 추가 및 도메인의 모든 public 메서드에 대한 단위 테스트 진행하기.
- 자동차에 대한 Comparable 인터페이스 구현을 통해 position 구하는 로직 개선하기. (getter 사용 X) 

DTO 변환을 위해서 Service 레이어를 추가하였다는 것이 가장 크게 바뀐 점이다. 그리고, 당시 리뷰어님께 받았던 피드백 중에서 "부정형 네이밍을 사용하지 않기" 에 대해서 리뷰를 받았었는데, 이 부분도 자잘하게 수정했던 점이 기억에 남는다. 그리고, 여전히 한 가지 메서드는 하나의 행위를 하도록 만드는 게 잘 지켜지지 않아서, 리팩토링 때 최대한 개선하려고 노력했다.

 

💬 for문과 for-each, stream

// Cars.java
cars.forEach(car -> {
        int power = numberGenerator.generate();
        car.move(power);
        });

또한, 페어랑 진행하면서 for문 대신에 .forEach()를 사용했었는데, 이 부분에 대해 정확히 인지하지 못하고 사용했던 것 같아서 따로 공부를 진행했다.

내부에서 iterator를 사용해서 값을 가져온다는 점과, for-each가 일반 for에 비해서 느리다는 것은 처음 알았다 🫢 또한, Collections.forEach 역시 iterator를 사용하며 반복하기 때문에, enhanced for문과 동일하고, 성능상으로 동일하다고 한다! (Stream이라서 더 느릴 줄 알았는데 아니었다...) 이렇게 또 배울 수 있었다.

 

---

✔️ 3차 리팩터링

사실 3차 리팩터링은 많이 진행하지 못했다. 연극도 겹치고, 다음 날에 새로운 미션을 진행해야 돼서 크게 바꾸지는 못했다.

 

💬 우승자 선출 로직에 대하여

 public List<String> pickWinners() {
        Car maxPositionCar = getMaxPositionCar();

        return cars.stream()
        .filter(maxPositionCar::isSamePosition)
        .map(Car::getName)
        .collect(Collectors.toUnmodifiableList());
        }

기존에는 Cars에서 우승자를 뽑는 로직을 두었었다. 이유는 일급컬렉션인 Cars에서 이미 자동차에 대한 정보를 가지고 있기 때문에, 객체에게 질문하기 위해서 자동차 내에서 진행하고자 했었다. (getter를 통해 받아와서 처리하지 않으려고 했었다.) 다만, 리뷰어님께서는 '우승자 선출에 대한 책임'은 경주를 관리하는 Race에서 이루어지는 게 더 맞지 않냐고 말씀해 주셔서, WinnerCar라는 객체를 생성해서 관리하고자 했다.

public class WinnerCar {

    private final Cars carInfo;

    private WinnerCar(final Cars carInfo) {
        this.carInfo = carInfo;
    }

    public static WinnerCar create(Cars carInfo) {
        return new WinnerCar(carInfo);
    }

    public List<String> getName() {
        Car maxPositionCar = carInfo.getMaxPositionCar();
        return getWinnerNames(maxPositionCar);
    }

    private List<String> getWinnerNames(Car maxPositionCar) {
        return carInfo.getCars()
                .stream()
                .filter(maxPositionCar::isSamePosition)
                .map(Car::getName)
                .collect(Collectors.toUnmodifiableList());
    }
}

다만, 내부적으로 Cars의 정보를 받아와서 수행하기 때문에 WinnerCar가 특별히 필요하다는 생각이 들지 않아서, 최종적으로 봤을 때는 그렇게 좋은 리팩토링이 아니었다고 생각이 든다. (아무래도 시간이 촉박해서 더 신경을 못 쓴 것 같다 ㅠㅠ) 지금 진행한다면, maxPosition에 대한 정보를 관리하거나, 혹은 우승자에 대해 따로 저장하는 필드를 두어서 조금 더 객체가 책임을 가지도록 진행했을 것 같다. 이 부분은 아쉽다.

---

 

아무튼, 이렇게 회고 작성이 끝났다! 🥲

본 회고에 담긴 것보다 사실 더 많은 것들을 했던 것 같지만, 그런 건 PR에서 충분히 드러나는 정보인 것 같아서 조금 더 내 감정과 배운 점들을 위주로 작성하였다. 앞으로 미션하면서 회고도 꾸준히 작성해보고자 한다 💪 파이팅...!