프리코스 2주차 후기: 랜덤값 테스트 그거 어떻게 하는건데

프리코스 2주차가 지나갔다. 이번 주차에는 테스트 코드까지 작성해야 해서 조금 힘들었지만 그만큼 좋은 경험이었다. 

리팩토링까지 끝마친 결과는 아래와 같다. 지난 주차에 비해 클래스의 수가 많아진게 눈에 띈다😂

 

그럼 이번 주차 회고도 시작~! ᕕ( ᐛ )ᕗ 

 

1. 랜덤값 테스트는 어떻게 해야할까?

2주차 미션에서 가장 어려웠던 점이었다. 0~9 사이의 수를 무작위로 뽑아 4 이상이면 차가 전진하는 방식이었는데, 요구사항에 테스트 코드 구현까지 있었다🥹 랜덤값은 컨트롤할 수 없는 부분이라 테스트를 어떻게 구현해야 하나 고민했는데, 결국 Car.move() 메서드에서 랜덤 함수를 직접 호출하지 않고 파라미터로 값을 전달받는 형식으로 단위 테스트를 진행했다.

public Car {
    public void move(int power) {
        if (power >= POWER_THRESHOLD) {
            this.distance++;
        }
    }
}

public CarTest {
	
    @ParameterizedTest
    @ValueSource(ints = {4, 6, 9})
    void 무작위로_뽑은_값이_4_이상이면_전진한다(int value) {
        car.move(value);
        assertThat(car.getDistance()).isEqualTo(3);
    }
}

 

 

그런데 나중에 찾아보니 NumberUtil 아예 인터페이스로 선언하고, 구현체를 갈아끼우는 방식으로도 테스트를 진행할 수 있었다.

public interface NumberUtil {
    public int getNumber();
}

public class Car {
    // 중략

    private final String name;
    private int distance;
    private final NumberUtil numberUtil;

    // 중략

    public void move() {
        int power = numberUtil.getNumber();

        if (power >= POWER_THRESHOLD) {
            this.distance++;
        }
    }
}

class CarTest {

    @ParameterizedTest
    @ValueSource(ints = {4, 6, 9})
    void 무작위로_뽑은_값이_4_이상이면_전진한다(int value) {
        Car car = new Car("pobi", () -> value);
        car.move();
        assertThat(car.getDistance()).isEqualTo(3);
    }
}

 

 

기존 방식은 외부에서 매번 RandomUtil을 이용해 Car에게 필요한 숫자를 넣어주어야 했다. 하지만 인터페이스와 구현체를 이용하는 방식은 Car 객체가 직접 숫자를 생성하고 그 결과를 바탕으로 자신의 상태를 변경한다는 점에서 좀 더 응집도가 높아졌다는 생각이 들었다. 그리고 나중에 랜덤값을 뽑는 방식이 변경되어도, 구현체만 바꿔주면 되기 때문에 이 방법이 유연성 측면에서 더 나은것같다👀

 

---

2. 입력값 검증 책임은 어디에 둬야 할까?

처음에는 InputValidator에서 입력값을 검증했다. 그런데 요구사항을 하나 둘 구현하다 보니 InputValidator가 점점 무거워졌다. 자동차명도 검증해야 하고, 자동차 목록도 검증해야 하고, 시도 횟수도 검증해야 하고... 검증 조건이나 검증 대상이 추가되면 클래스가 더욱 무거워질 것이라는 생각에 검증 책임을 각 도메인 객체로 이동했다.

public Car(String name) {
    this.name = validateCarName(name);
    this.distance = 0;
}

private static String validateCarName(String name) {
    validateNotNull(name);
    validateNotBlank(name);
    name = name.strip();
    validateNameLength(name);

    return name;
}

private static void validateNameLength(String name) {
    if (name.length() < MIN_CAR_NAME_LENGTH || name.length() > MAX_CAR_NAME_LENGTH) {
        throw new IllegalArgumentException(
                String.format(INVALID_LENGTH_RANGE_CAR_NAME.getMessage(),
                        MIN_CAR_NAME_LENGTH, MAX_CAR_NAME_LENGTH, name));
    }
}

private static void validateNotBlank(String name) {
    if (name.isBlank()) {
        throw new IllegalArgumentException(BLANK_CAR_NAME.getMessage());
    }
}

private static void validateNotNull(String name) {
    if (name == null) {
        throw new IllegalArgumentException(NULL_CAR_NAME.getMessage());
    }
}

 

그런데 이렇게 되면 Car 객체를 상속했을 때 검증 로직까지 함께 상속된다고 한다😱 당연한 건데 왜 생각을 못했을까...🥹

심지어 static 메서드로 선언하는 바람에 오버라이딩도 불가능한 대쪽같은 메서드가 되어버렸다 ^_ㅠ

 

Car 객체에서는 졸지에 입력값 검증이라는 책임까지 지게 되었다. 그래서 리팩토링 때 이 부분을 수정했는데 자세한 내용은 후술..!

 

---

3. 출력은 반드시 OutputHandler에 넣어야 할까?

public class InputHandler {

    public List<String> getCarNames() {
        System.out.println(CAR_NAME_INPUT_MESSAGE.getMessage());

        String input = Console.readLine();
        return Arrays.stream(input.split(DELIMITER)).toList();
    }

    public Integer getRoundSize() {
        System.out.println(ROUND_SIZE_INPUT_MESSAGE.getMessage());
        String input = Console.readLine();

        validateRoundSize(input);
        return Integer.parseInt(input);
    }
}

 

지난 주차와의 차이점! 지난 주에는 출력은 전부 OutputHandler에서 관리했는데, 이번엔 입력을 넣어달라는 메세지는 InputHandler에서 호출했다✨

 

코드를 변경한 가장 큰 이유는 INPUT_MESSAGE 출력을 OutputHandler의 메서드로 분리할 경우, 해당 메세지가 단독으로 출력될 수 있다는 점이었다. INPUT_MESSAGE를 출력하고 입력은 안받는다면? 지금이야 나 혼자 개발하니까 헷갈리지 않겠지만 개발 담당 인원이 늘어나면 어떻게 될지 모르는 일이다.

또 논리적인 흐름 상 InputHandler에서도 이 정도의 메세지를 띄우는 건 어색하지 않다고 생각했기 때문에 이번에는 입력값을 넣어달라는 출력은 InputHandler에 위치시켰다.

 

---

4. 코드리뷰 반영하기

이번 주차에는 용기내서 글을 올려봤는데, 눈 깜빡하는 사이에 엄청 많은 분들이 와주셔서 깜짝 놀랐다. 누추한 곳에 귀한 분들이...🥹 귀한 분들의 피드백을 모아 반영해보려고 노력했다.

먼저 리뷰어분들께서 남겨주신 긍정적인 피드백은 아래와 같았다. 나름 신경썼던 부분인데 귀신같이 알아봐 주시니 기분은 좋네용🍀

• 참여 자동차 개수와, 최대 시도 횟수를 구체적으로 제한한 점이 좋다.
• @BeforeEach, @ParameterizedTest 등 적절한 테스트 어노테이션을 활용한 점이 좋다.
• 일급 컬렉션 사용 시, 리턴할 때 리스트 원본이 아닌 unmodifiableList()로 한번 감싸서 반환한 것이 좋다.

그리고 다시 생각해볼만한 피드백은 아래와 같았다.

 

(1) Rounds에서 출력까지 담당하고 있어요.

public class Rounds {

	private final Integer roundSize;
    
	public void proceedRounds(List<Car> cars, OutputHandler outputHandler) {
        for (int i = 0; i < roundSize; i++) {
            List<String> roundResult = processRound(cars);
            outputHandler.printRoundResult(roundResult);
        }
    }
}

 

Rounds 객체는 시도 횟수를 관리하고, 시도 횟수만큼 라운드를 진행시키는 클래스이다. 매 라운드가 끝날때면 OutputHandler를 호출해서 결과를 출력했는데, 도메인 모델에서 출력까지 담당한고 있는것같다는 의견을 받았다.

생각해보니 비즈니스 로직에 집중해야 하는 Rounds 객체에서 OutputHandler를 직접 호출하는 것은 좋아보이지 않았다. 도메인 모델은 여러 군데에서 재사용할 수 있어야 하는데 출력을 직접 담당하다 보니 그게 어려워지고, OutputHandler가 변경될 경우 Rounds도 영향을 받을 위험이 있었기 때문이다.

 

 

public List<List<String>> proceedRounds(List<Car> cars) {
    List<List<String>> result = new ArrayList<>();

    for (int i = 0; i < roundSize; i++) {
        List<String> roundResult = processRound(cars);
        result.add(roundResult);
    }

    return result;
}

 

그래서 2차원 리스트를 반환하고, 출력은 Rounds.proceedRounds()를 호출하는 Game에서 완전히 담당하기로 결정!

 

(2) Game.start() 메서드가 너무 많은 책임을 지고 있어요.

public void start() {
    List<String> carNames = inputHandler.getCarNames();
    cars = new Cars(carNames);

    String roundSizeInput = inputHandler.getRoundSize();
    rounds = new Rounds(roundSizeInput);

    outputHandler.printResultMessage();
    rounds.proceedRounds(cars.getCars(), outputHandler);

    List<String> winners = cars.getWinners();
    outputHandler.printWinner(winners);
}

 

Game.start()가 조금 뚱뚱하다 싶긴 했는데 역시나 코드리뷰에서 비슷한 의견을 받았다😂 별도 메서드로 분리해봤자 호출 뎁스만 깊어질 것 같아서 따로 분리하지는 않았는데... 이제보니 분리하는게 나을 것 같아서 분리 결정.

 

 

public void start(NumberUtil numberUtil) {
    getUserInputs(numberUtil);
    processRounds();
    announceWinner();
}

private void getUserInputs(NumberUtil numberUtil) {
    List<String> carNames = inputHandler.getCarNames();
    cars = new Cars(carNames, numberUtil);

    Integer roundSize = inputHandler.getRoundSize();
    rounds = new Rounds(roundSize);
}

private void processRounds() {
    outputHandler.printResultMessage();
    List<List<String>> roundResult = rounds.proceedRounds(cars.getCars());
    roundResult.stream().forEach(result -> outputHandler.printRoundResult(result));
}

private void announceWinner() {
    List<String> winners = cars.pickWinners();
    outputHandler.printWinner(winners);
}

 

그런데 이런다고 start() 메서드의 책임이 줄어드는 걸까? 라는 생각이 든다. 단순히 메서드만 분리했을 뿐인데? 여전히 start() 메서드는 입력값을 받는 메서드를 호출하고, 라운드 진행 메서드도 호출하고, 우승자 발표 메서드까지 호출하고 있다. 이걸 책임이 줄어들었다고 볼 수 있을까..?🤔

 

새 클래스를 생성해서 책임을 분리할까 했는데 아직 그럴 단계까진 아닌 것 같아서 그냥 두었지만 왠지 찝찝하다😂 이 부분에 대해서는 조금 더 고민해볼 예정.

 

 

(3) 정적 유틸리티 클래스인데 생성자 보호가 되어있지 않아요.

public class RandomUtil {
    private static final int MIN_VALUE = 0;
    private static final int MAX_VALUE = 10;

    public static int getRandomNumber() {
        return Randoms.pickNumberInRange(MIN_VALUE, MAX_VALUE);
    }
}

 

완전히 놓친 부분 ‧⁺◟( ᵒ̴̶̷̥́ ·̫ ᵒ̴̶̷̣̥̀ ) 리팩토링 과정에서  RandomUtil 클래스는 정적 유틸리티 클래스에서 벗어나 RandomNumberUtil이라는 구현체가 되었지만 의미있는 피드백이었다... 다음에는 까먹지 말자.

 

(4) 도메인 객체 클래스의 책임이 너무 많아요. 검증은 별도 클래스로 분리하는 게 좋아보여요.

public class InputHandler {
    private static final String DELIMITER = ",";


    public List<String> getCarNames() {
        System.out.println(CAR_NAME_INPUT_MESSAGE.getMessage());

        String input = Console.readLine();
        List<String> names = Arrays.stream(input.split(DELIMITER)).toList();

        CarsValidator.validateCars(names);
        names.stream().forEach(CarNameValidator::validateCarNames);

        return names.stream()
                .map(String::strip)
                .toList();
    }
}

 

미션을 수행하며 고민했던 부분과 이어지는 피드백이었다. 다른 분들 코드를 리뷰하다 보니 도메인 객체별로 Validator 클래스를 만드신 분들이 계셔서 나도 각 도메인별로 검증 클래스를 생성하여 메서드를 이동시켰다. 이렇게 하니 각 클래스별로 책임이 명확해서 만족중😆

 

---

5. 반영하지 않은 코드리뷰

InputHandler와 OutputHandler를 static화 하지 않고 인스턴스화한 이유가 있나요?

이 부분은 나도 고민했던 부분인데 때마침 코드리뷰에 비슷한 질문이 달려서 내 생각을 정리해봤다. 결론부터 말하자면 나는 static은 객체지향과 멀리 있는 개념이라고 생각하기 때문이다.

 

static 함수는 클래스 레벨에 선언된다. 이 말인 즉슨 인스턴스가 생성되기 전부터 메모리 영역에 계속 존재하고, 객체의 생명주기에서 벗어나 있다는 것을 의미한다. 그래서 인스턴스 생성 없이 호출할 수 있고, 오버라이딩이 불가능하다. (오버라이딩을 사용하면 호출할 메서드가 런타임 시점에 결정되는데, static 함수는 컴파일 시점에 호출이 결정되기 때문이다.)

 

이러한 이유로 나는 아래의 조건을 만족하는 특별한 경우에만 static을 사용한다.

• 객체의 상태값과 무관하게 파라미터를 기반으로 동작하며
• 애플리케이션 전역적으로 호출되어 메모리 상에 계속 존재할 필요가 있으며
• 오버라이딩을 통한 확장 가능성이 매우 낮을 때

입출력값 핸들러의 경우 (1)의 조건은 만족하지만 (2) 애플리케이션 전역이 아닌 Game에서만 호출된다. (3)의 경우도 지금은 콘솔에서 입력을 받고 있지만, 요구사항이 어떻게 변경될지 모른다는 점을 감안했을때 조건을 충족시키지 않는다고 판단하여 인스턴스를 생성하여 주입해주었다. 반영하지는 않았지만 생각해 볼 거리가 많은, 좋은 피드백이었다👍

 

---

6. 무엇을 배웠나요?

(1) 테스트 코드 작성법

원래도 조금은 알고 있었지만 @ParameterizedTest와 @ValueSource를 통해 하나의 메서드로 여러 입력값을 테스트하는 방법은 이번에 처음 알게 되었다. 지난 주차에는 테스트 코드를 나중에 작성했는데, 테스트 결과에 맞춰 요구사항을 변경하고 싶어지길래 이번에는 요구사항을 명확하게 정의하고 비즈니스 로직 작성과 동시에 테스트 코드를 작성했다.
테스트 코드를 먼저 작성하고 느낀 장점은 코드 리팩토링 시에도 테스트를 실행하여 변경된 코드가 요구사항을 충족하는지 확인할 수 있다는 점이었다. 단점은 리팩토링 때 테스트 코드도 원본 클래스를 따라 함께 변경해야 해서 좀 번거로웠다는거...😂 그치만 단점에 비해 장점이 큰 것 같아서 앞으로도 비즈니스 로직과 거의 동시에 작성해볼 생각이다 👀

 

(2) 일급 컬렉션 사용법

자동차 목록을 어떻게 관리할까..! 하다가 일급 컬렉션을 사용해봤다. 사용 소감은 대만족~! Game에서 직접 자동차 리스트를 관리할 때는 우승자도 Game에서 뽑아야 했다. 하지만 일급 컬렉션 객체로 분리하고 나니, 자동차 목록과 관련된 비즈니스 로직을 전부 Cars에서 처리할 수 있어서 Game 클래스가 한결 가벼워졌다. 또 반환 시에는 불변 리스트로 반환해서 외부에서 리스트에 원소를 추가하거나 삭제하는 걸 방지할 수도 있어서 아주 유용했다. unmodifiableList에 대해서는 조금 더 생각해봐야 할 것 같지만...🥹

 

(3) 언제 static으로 선언해야 할까?

사실 지금까지는 static을 느낌적으로 사용하고 있었는데 이번 기회에 static 사용에 대해 명확한 기준을 정리할 수 있었다. 이 기준이 옳은지 그른지와는 별개로 나의 생각과 기준이 생겼다는 점은 긍정적인 변화라고 생각하기 때문에 👀 static의 개념에 대해서도 돌아볼 수 있었던 좋은 기회라고 생각한다.

 

---

 

어째 점점 회고가 길어지는 것 같은데... 어차피 시간이 지나면 남는 건 기록밖에 없기 때문에 최대한 많은 것을 담으려고 노력했다.

아무리 생각해도 우테코 미션은 제출이 시작이고 리팩토링과 회고 작성까지가 진짜 끝인것같다😂 힘들지만 이제 절반 지났으니까 다음주 미션도 화이팅...🫠