[우아한테크코스] LV1 - 자동차 경주 미션 회고

우아한테크코스/회고

[우아한테크코스] LV1 - 자동차 경주 미션 회고

_Hiiro 2023. 2. 14. 09:06

우아한 테크코스 첫 미션은 자동차 경주 미션이었다. step1과 step2로 이루어져 있으며 step1은 매칭된 페어와 짝 프로그래밍으로 진행한다. 이후 제이온(리뷰어)의 피드백을 받고 각자 리팩토링하는 과정을 거치면서 PR을 merge한다. 이후 추가된 요구사항들을 반영하여 step2를 진행하고 step1때와 마찬가지로 피드백을 통한 리팩토링 과정을 거쳐 PR을 merge한다.

각 과정에서 어떤 부분에 초점을 맞춰 미션을 진행했고 어떠한 피드백을 받았는지, 그리고 어떻게 리팩토링 했는지에 집중해서 회고를 진행하고자 한다.

Step1

[일급 컬렉션]

첫 페어 프로그래밍은 아코와 진행하게 되었다. 주어진 요구사항들을 정리하며 이번 미션에서 집중해보고 싶은 부분을 얘기해 본 결과 일급 컬렉션을 제대로 사용해보자는 목표를 설정하게 되었다.

가장 중점이 되었던 부분은 자동차의 이름과 위치 상태 값을 가지는 Car 객체들을 저장하고 관리하는 일급 컬렉션 클래스인 Cars가 되었다. 자동차 경주 게임을 진행하는 기능을 담당하는 RacingGame 클래스를 따로 두어 해당 클래스에서 자동차들의 전진 여부를 파악하는 isMovable 메서드를 구현했다. 이렇게 되면 발생하는 문제는 일급 컬렉션 클래스인 Cars를 이용해 모든 자동차들을 한 라운드 전진시키는 기능을 수행해야할 때마다 내부에 저장하고 있는 모든 Car 객체들을 getter를 사용해 외부로(RacingGame) 반환해주어야 하는 문제가 발생했다. isMovable 메서드를 각 Car 객체들마다 따로 따로 사용해줘야 했기 때문이다.

일급 컬렉션 클래스 내부에서 출력 이외 목적의 getter 구현을 피하기 위해서 어떻게 해야할까에 대한 고민에 시간을 많이 쏟았다. 그래서 내린 결론은 다음과 같다.

일급 컬렉션을 사용하는데 getter 메소드가 남발된다면...
해당 getter 메소드를 요구하는 기능이 일급 컬렉션 혹은 일급 컬렉션이 저장하는 원소 클래스 내부로 이관되어야 하지는 않는지 생각해봐야 한다.

이번 미션에서 RacingGame 클래스에 구현되었던 isMovable 메서드가 이러한 경우에 해당되었다. 잘 생각해보면 RacingGame 클래스에 위치하는 isMovable 메서드를 사용해 변화시켜야 하는 position 상태값이 Car 내부에 존재했다. 그렇기 때문에 Cars 클래스에서의 getter 사용이 불가피 했던 것이다. 따라서 RacingGame 클래스에 구현된 isMovable 메서드를 행동의 주체가 되는 Car 클래스 내부로 이관하여 자동차가 전진 여부를 직접 판단할 수 있도록 하였다. 그 결과 경주 한 라운드를 진행하는 기능은 Cars 클래스가 Car 객체들을 외부로 반환하지 않고 내부적으로 모두 수행할 수 있게 되었고 최종적으로는 비즈니스 로직 측면에서 getter 사용을 피할 수 있었다.

[ToString]

IntelliJ 에서는 toString 메서드 오버라이딩을 자동 생성해준다. 기본적으로 toString은 디버깅 목적으로 사용되기 때문에 만약 테스트 코드 결과가 Object 클래스에 구현된 toString 내용 처럼 알아보기 힘든 값으로 나오게 된다면 toString 메서드를 생성해주도록 하자.

Step2

[입출력 기능 관련 테스트]

InputView 내부에 구현된 자동차 경주 라운드 횟수 입력값에 대한 검증 기능을 테스트 하고자 하는 상황이 있었다. 하지만 해당 기능은 InputView 내부에서만 사용했기에 private 접근 제어자로 선언해줘야 했다. 이런 이유로 InputView의 round 입력 메서드를 통해 검증 기능을 테스트 할 수 밖에 없었다.

가장 먼저 해야할 것은 표준 입력을 통해 테스트 코드에서 원하는 입력값을 넣어줄 수 있어야 했다. 하지만 Scanner를 InputView 입력 메서드 별로 내부에서 생성해서 사용하려다 보니 테스트 코드에서 NoSuchElementException이 발생했다. 해당 오류는 이미 닫힌 Scanner 객체를 조회하려고 해서 발생하는 것이었다.

Scanner 객체를 인스턴스 변수로 저장하여 닫히지 않고 유지될 수 있도록 했다. 또한 테스트를 위한 표준 입력 설정을 위해 InputView 생성 시 사용할 Scanner 객체를 외부에서 생성해서 주입할 수 있도록 했다.

Controller controller = new Controller(new InputView(new Scanner(System.in)), new OutputView(),
                new RandomNumberGenerator());

public class InputView {
    private final Scanner scanner;

    public InputView(Scanner scanner) {
        this.scanner = scanner;
    }
}

위와 같은 방식으로 구현한 결과 Scanner에 주입되는 표준 입력인 System.in 또한 사용자가 임의로 교체해줄 수 있었다.

그래서 테스트 코드에서는 미리 정해진 입력 값을 바이트 코드로 변환해 InputStream으로 담아서 표준 입력 Stream으로 설정해 주었다.

class InputViewTest {

    @ParameterizedTest
    @ValueSource(strings = {"r", "ㄱ", ",", "\n"})
    @DisplayName("시도 횟수 사용자 입력 값이 올바른 지 검증하는 테스트")
    void validateRoundTest(String input) {
        //Given
        InputStream in = new ByteArrayInputStream(input.getBytes());
        System.setIn(in);
        InputView inputView = new InputView(new Scanner(System.in));

        //When
        Throwable result = catchThrowable(inputView::readRacingRound);

        //Then
        assertThat(result).isInstanceOf(IllegalArgumentException.class);
    }
}

Scanner를 이용한 입출력 테스트를 진행해야 할 시에는 System.setIn / System.setOut을 통해 표준 입출력 버퍼를 교체할 수 있도록 Scanner 객체를 외부에서 생성하여 주입하도록 하자.

[@Disabled 어노테이션을 통한 테스트하지 않는 테스트 코드 이유 명시]

리팩토링 강의 중 테스트 하지 않는 메서드에 대한 명시를 어떻게 해주는 것이 효과적인가에 대한 질문이 나왔었다. 많은 의견들이 있었지만 그 중 크루 여우가 알려준 방법이 확실히 좋아 보여서 기록하고자 한다.

@Test
@Disabled("Cars의 moveEachCar 기능만을 수행하므로 테스트 하지 않습니다.")
void playOneRound() {
}

위와 같이 @Disabled 어노테이션을 사용하면 어떤 이유로 비즈니스 로직을 구현한 메서드에 대해 단위 테스트를 진행하지 않았는지 확실하게 명시할 수 있다. 처음 코드를 접하는 사람에게도 명확하게 의도를 전달할 수 있어 좋은 방법이라 생각한다.

(내용 추가)

자동차 경주 미션의 다음 미션인 사다리 게임 미션을 진행하며 알게된 부분을 추가로 정리한다.

@Disabled 어노테이션이 만들어진 본래 취지는 주로 존재했다가 사라진 기능에 대한 테스트 코드 위에 붙이는 용도라고 한다. 따라서 보는 사람으로 하여금 헷갈릴 수 있는 여지가 남아 있어 해당 방법으로 명시하는 것은 지양하는 것이 좋겠다.

보통 테스트하지 않는 메서드는 팀에서 컨벤션으로 가져가기 때문에 팀 내 문서화를 따로 해두는 것이 일반적이라고 한다. 미션을 진행할 때는 따로 문서화까지는 해도 되지 않을 듯 하여 테스트 클래스 상단에 javadoc으로 간단하게 명시하는 것으로 결정하였다.

[값의 검증 위치]

크루 스플릿이 던진 질문에 대한 답변과 생각을 정리하던 중 제이온에게 피드백 받은 사항을 더불어서 정리하고자 한다. 결론부터 말하자면 inputView와 도메인에서 검증 기능을 각각 수행하도록 둘 다 진행하는 것이 좋다고 이번에 생각하게 되었다.

먼저 도메인 객체 내에서 검증 과정이 필요하다고 생각한 이유는 도메인 클래스 객체 생성에 필요한 값이 반드시 inputView에서만 받아오는 것은 아니기 때문이다. InputView에서 받아올 때도 있겠지만 다른 도메인 객체로부터 넘겨받을 수도 있고 여러가지 가능성이 존재한다는 것이다. 이런 부분들을 처리하기 위해 도메인 객체 내에서 검증과정이 필요하다고 결론을 내렸다.

또한 InputView에서도 검증을 해야한다고 생각한 이유로는 도메인 객체 내에서 처리하기 힘든 view 단에서만의 검증 로직이 존재하기 때문이다. 예를 들어 사용자 입력 값에 대해 비즈니스 로직 측면에서 검증을 하는 것은 충분히 도메인 내에서 진행 할 수 있을 것이다. 하지만 그 값이 원하는 형식으로 이루어졌는지, 악의적으로 구성되진 않았는지 여부 등은 도메인 로직보다는 view 단에 더 가깝다고 보는 것이 합리적이라 생각된다.

결론은 도메인과 view에서 모두 검증을 수행하되 조금씩은 검증해야 하는 내용이 다르다는 것이다. 앞으로는 도메인과 view에서 검증해야할 기능을 나누어서 수행하도록 하자.

[unmodifiable]

미션을 진행하면서 Car 객체들을 저장하고 관리하는 Cars 일급 컬렉션 클래스를 구현하게 되었다.

public class Cars {

	private List<Car> cars;
    
	public Cars(List<Car> cars) {
        this.cars = new ArrayList<>(cars);
    }
    
    public List<Car> moveEachCar() {
        for (Car car : cars) {
            car.goForward();
        }
        return Collections.unmodifiableList(cars);
    }
    
}

moveEachCar 메소드는 컬렉션 내 모든 car 인스턴스들을 전진 시키고 결과 출력을 위해 해당 컬렉션을 반환하는 메소드다. 외부에서 경주에 참여 중인 자동차들을 담고 있는 리스트가 의도치 않게 수정되는 것을 방지하기 위해 unmodifiable을 사용해 보호하고자 했다.

여기에 대한 제이온의 피드백은 다음과 같았다.

추가로 단점을 말씀드리자면, unmodifiableList로 반환된 List는 수정이 발생하면 예외를 던지기 때문에 예외 핸들링을 필수로 해야 합니다. 하지만 같이 협업하는 동료 개발자는 단순히 getCars()라는 네이밍만 보았을 때 수정 가능한 가변 List로 생각할 확률이 높으므로 예외가 발생했을 때 당황할 확률이 높답니다 ㅎㅎ

확실히 list 값을 넘겨 받으면 가변 리스트일 것이라고 생각하는 것이 일반적인 경우라 생각한다. 그렇다면 사용자가 예상치 못한 예외 처리를 직면하는 문제를 해결함과 동시에 외부로 반환하는 list 데이터를 보호할 수 있는 방법이 없을까에 대해 고민하게 되었다. 하지만 뚜렷한 방법이 떠오르지 않았고 이에 대해 제이온에게 의견을 구했다.

unmodifiable collection을 사용하는 이유가 뭘까요?
아무래도 컬렉션의 불변함을 유지하기 위함이라고 생각해요. 하지만, 제가 전에 드린 아티클에서 보실 수 있듯이 unmodifiable collection을 사용한다고해서 항상 불변함이 유지가 되지는 않았어요. 물론 히이로께서는 생성자에서도 방어적 복사를 잘 해주셔서 불변함이 유지는 되겠으나 메서드에 unmodifiable같은 네이밍을 붙이지 않으면 동료 개발자가 예외 처리를 하지 않고 쓸 확률이 있을 거예요.
그래서 저는 개인적으로 unmodifiable collection을 반환하지 않고 한 번 더 new ArrayList() 등과 같은 방어적 복사를 해서 반환을 하는 편입니다. 그러면 반환 받은 컬렉션은 가변할지라도, 원래 컬렉션은 불변을 유지하므로 사용하는 쪽에서도 가변하게 쓰더라도 예외가 발생하지 않습니다!

public class Cars {

	private List<Car> cars;
    
	public Cars(List<Car> cars) {
    	this.cars = new ArrayList<>(cars);
	}
    
    public List<Car> moveEachCar() {
        for (Car car : cars) {
            car.goForward();
        }
        return new ArrayList<>(cars);
	} 
}

위의 피드백을 듣고 리팩토링한 moveEachCar 메서드는 다음과 같다. 데이터를 반환할 때에도 방어적 복사를 통해 모든 것을 해결할 수 있었다. 다만 이 경우에도 Cars의 요소값들인 Car는 스스로의 불변을 보장해야 한다.

[방어적 복사]

방어적 복사는 클래스 내부에 저장된 객체 데이터를 보호하기 위해 사용된다. 주된 사용 경우는 아래와 같다.

생성자의 매개 변수로 객체를 받아 클래스 내부 필드를 초기화 해야하는 경우
getter 메서드로 클래스 내부에 저장된 객체를 외부로 반환해야 하는 경우

자동차 경주 미션을 진행하면서 위 두 가지 경우를 모두 경험하게 되었다.

먼저 생성자의 매개변수로 객체를 받아 클래스 내부 필드를 초기화 하는 경우를 살펴보자.

public class Cars {

	private List<Car> cars;
    
	public Cars(List<Car> cars) {
    	this.cars = cars;
	}
}


public class DefensiveCars {

	private List<Car> cars;
    
	public DefensiveCars(List<Car> cars) {
    	this.cars = new ArrayList<>(cars);
	}
}

이 두 클래스 객체의 차이점은 외부에서 주입된 cars 객체가 외부에서 수정될 때 드러난다.

public static void main(String[] args) {
        List<Car> carList = new ArrayList<>(List.of(new Car("kevin"), new Car("hiiro")));

        Cars cars = new Cars(carList);
        DefensiveCars defensiveCars = new DefensiveCars(carList);

        carList.add(new Car("woong"));

        cars.printCars();
        defensiveCars.printCars();
}

결과를 보면 생성자에서 매개변수로 받은 carList에 대해 그대로 복사를 진행한 Car는 외부에서 carList에 변동사항이 생기면 그것이 Cars 내에 저장된 객체에도 반영되어 버린다. 하지만 방어적 복사를 채택한 DefensiveCar는 외부에서 carList에 생기는 변동과 독립적인 상태를 유지하는 것을 알 수 있다.

외부에 존재하는 carList와 Cars, DefensiveCars가 필드 값으로 가진 컬렉션의 주소 값을 각각 출력한 결과이다. carList와 Cars는 같은 주소 값을 가지므로 carList를 통한 접근이 Cars 내 컬렉션에도 영향을 주는 것이다. 반면 DefensiveCars는 새로운 주소값을 재할당하여 객체들을 가리키고 있으므로 가리키는 Car 객체들이 불변인 한 carList에 의해 변동될 일은 없다고 볼 수 있다.

하지만 여기에서 주목해야 할 것은 각 Car들이 반환한 자신들의 주소 값이다. 방어적 복사를 채택하는 것과 상관 없이 처음 생성된 Car 인스턴스들의 주소값을 모든 컬렉션들이 공유 하고 있다. 즉 방어적 복사는 깊은 복사가 아닌 얕은 복사라는 것을 명심해야 한다. 만약 getter를 통해 컬렉션 요소 객체들이 외부로 반환되고 해당 객체들이 불변을 보장하지 않는다면 외부 어디에서든 요소 객체들을 수정할 수 있게 되는 것이다.

따라서 외부로부터 클래스 내부 필드 객체들을 보호하고자 한다면 해당 내부 필드의 객체와 내부 요소들까지 모두 불변이어야 한다.

[unmodifiable VS Immutable]

크루 애쉬가 귀가하는 도중 물어봤던 unmodifiable과 immutable의 차이를 테스트 코드 작성을 통해 알게 되어 정리한다. 애쉬가 물어봤던 사항은 원본 컬렉션에 변동사항이 생기면 unmodifiable로 반환한 컬렉션에도 영향이 미치는가에 대한 것이었다. 아래는 모두 통과하는 테스트 코드이다.

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

class TestElementTest {

    @Test
    @DisplayName("unmodifiable collection 객체 수정 연산 불가 테스트")
    void TestElementUnmodifiableTest() {
        //Given
        List<TestElement> originCollection = new ArrayList<>(List.of(new TestElement()));
        List<TestElement> unmodifiableList = Collections.unmodifiableList(originCollection);

        //Then
        assertThatThrownBy(() -> {
            unmodifiableList.add(new TestElement());
        }).isInstanceOf(UnsupportedOperationException.class);
    }

    @Test
    @DisplayName("immutable collection 객체 수정 연산 불가 테스트")
    void TestElementImmutableTest() {
        //Given
        List<TestElement> originCollection = new ArrayList<>(List.of(new TestElement()));
        List<TestElement> immutableList = Collections.unmodifiableList(new ArrayList<>(originCollection));

        //Then
        assertThatThrownBy(() -> {
            immutableList.add(new TestElement());
        }).isInstanceOf(UnsupportedOperationException.class);
    }

    @Test
    @DisplayName("unmodifiable 객체의 원본 컬렉션 변경 시 같이 변경되는지 테스트")
    void UnmodifiableReferenceTest() {
        //Given
        List<TestElement> originCollection = new ArrayList<>(List.of(new TestElement()));
        List<TestElement> unmodifiableList = Collections.unmodifiableList(originCollection);

        //Then
        assertThat(unmodifiableList.size()).isEqualTo(originCollection.size());

        originCollection.add(new TestElement());
        assertThat(unmodifiableList.size()).isEqualTo(originCollection.size()); // 원본 컬렉션이 변동되면 unmodifiable은 같이 변동됨
    }

    @Test
    @DisplayName("immutable 객체의 원본 컬렉션 변경 시 같이 변경되는지 테스트")
    void ImmutableReferenceTest() {
        //Given
        List<TestElement> originCollection = new ArrayList<>(List.of(new TestElement()));
        List<TestElement> immutableList = Collections.unmodifiableList(new ArrayList<>(originCollection));

        //Then
        assertThat(immutableList.size()).isEqualTo(originCollection.size());

        originCollection.add(new TestElement());
        assertThat(immutableList.size()).isNotEqualTo(originCollection.size()); // 원본 컬렉션이 변동되어도 immutable은 변동되지 않음
    }
}

정리하자면 unmodifiable은 컬렉션에 대한 수정 연산을 막아줄 뿐 원본 컬렉션에 대한 불변을 보장하지 않기 때문에 원본 컬렉션에 생기는 변동사항은 unmodifiable 반환 컬렉션에도 영향을 준다. 반면 Immutable은 내부적으로 원본 컬렉션에 대해 방어적 복사를 취하기 때문에 원본 컬렉션에 생기는 변동사항에 영향을 받지 않는다.

Reference

https://steady-coding.tistory.com/559
https://tecoble.techcourse.co.kr/post/2021-04-26-defensive-copy-vs-unmodifiable/

저작자표시