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

고민한 점

1주차 공통 피드백을 반영하자

1주차 과제를 제출하고 공통 피드백을 확인했습니다.
미처 생각하지 못한 부분이 많아서 많이 배웠습니다.
1주차 과제를 피드백에 맞게 수정하고, 2주차 과제에 피드백을 고려하며 구현했습니다.
특히 아래와 같은 부분에서 코드를 수정했습니다.

 

의도를 드러내지 않는 이름 사용하지 않기

xx1, xx2처럼 파라미터에 연속된 숫자를 덧붙이는 것보다는 명확한 변수 이름을 사용하도록 수정했습니다.

public class IntegerComparable implements NumberComparable<Integer> { 
    // 기존
    @Override  
    public boolean meetsThreshold(final Integer number1, final Integer number2) {  
        return number1 >= number2;  
    }  

    // 수정
    @Override  
    public boolean meetsThreshold(final Integer value, final Integer threshold) {  
        return value >= threshold;  
    }  
}

 

문맥을 중복하는 이름을 자제하기

가장 수정할 부분이 많았던 부분입니다.
클래스명이나 메서드의 파라미터에서 충분히 유추할 수 있는 정보를 메서드에 중복으로 사용한 부분을 수정했습니다.

public class DelimiterExtractor {
    // 기존
    public Delimiters extractDelimitersFrom(final String input, final Delimiters delimiters) { ... }  

    // 수정
    public Delimiters extractFrom(final String input, final Delimiters delimiters) { ... } 
}

public class IntegerAdder implements NumberAdder {
    // 기존
    public Integer addNumbers(final List<Integer> numbers) { ... } 

    // 수정
    public Integer add(final List<Integer> numbers) { ... } 
}

getter 메서드의 경우 Java beans 스펙을 반드시 따르지 않아도 되는 상황이었습니다. 단순히 값을 조회하는 역할이라면 get접두어 없이 메서드명으로 충분히 의도가 전달되기 때문에 아래와 같이 수정했습니다.

public class DelimiterPattern {
      // 기존
    public String getRegex() { ... }    
    public Pattern getPattern() { ... }  

    // 수정
    public String regex() { ... }        
    public Pattern pattern() { ... }    
}

 

공백 라인 적절히 사용하기

제가 임의로 정한 규칙으로 공백 라인을 두다보니 코드 리뷰나 피드백에서 어떤 의미인지 이해하기 어렵다는 피드백을 받았습니다.
따로 팀 규칙이 정해지지 않는 이상 임의의 공백 라인은 두지 않고, 의미 단락을 나눌 때만 사용하도록 수정했습니다.

private Long getPositiveNumber(final Long number) {  
    // 기존
    if (number < 0) {  
        throw new IllegalArgumentException("양수가 아닙니다.");  
    }  

    return number;  

    // 수정
    if (number < 0) {  
        throw new IllegalArgumentException("양수가 아닙니다.");  
    }  
    return number;  
}

 

stream을 잘 사용하자

for문 vs stream 성능 비교

문자열을 반복하는 코드를 작성할 때, 문자열의 repeat 함수는 int 범위의 반복 횟수만 지원하므로 long 범위의 반복 함수를 구현했습니다.

참고 : 문자열 잇는 여러 방법: https://www.baeldung.com/java-string-of-repeated-characters

 

작성한 for문의 가독성을 높이기 위해 stream으로 변경하였고, 이 과정에서 성능을 비교해보았습니다.

// for문 사용
long startTime = System.nanoTime();  
StringBuilder repeatValue = new StringBuilder();  
for (long i = 0; i < count; i++) {  
    repeatValue.append(value);  
}  
long endTime = System.nanoTime();  
System.out.println("for : " + (endTime - startTime));

// stream 사용
startTime = System.nanoTime();  
String collect = Stream.generate(() -> value)  
        .limit(count)  
        .collect(Collectors.joining());  
endTime = System.nanoTime();  
System.out.println("Stream : " + (endTime - startTime));  

 

실행 결과

Stream : 5327625
for : 20875

 

예상보다 for문이 Stream보다 훨씬 더 빨랐기 때문에, for문을 그대로 사용하기로 결정했습니다.

 

속도의 차이가 나는 이유는, for문은 미리 할당된 StringBuilder에 append 연산만 수행하는 단순 반복 작업입니다.
반면에 Stream은 각 단계(중간 연산, 최종 연산)별로 새로운 Stream 객체를 생성하므로 오버헤드가 발생하여 시간이 더 걸립니다.

 

따라서 for문은 단순 반복 작업에 사용하고, Stream은 복잡한 데이터 처리 작업의 경우에 사용하는 것이 좋습니다. 특히 Stream은 데이터 파이프라인을 통해 가독성을 확보할 수 있습니다.

 

forEach는 주의해서 사용하자

반복문을 Stream으로 변경하며 가독성을 높이고 Stream 활용에 익숙해지려고 노력했습니다.

// 기존 코드
private void moveWithPositions(final List<Boolean> moves) {
    for (int j = 0; j < cars.size(); j++) {
        if (moves.get(j)) {
            positions.increase(j);
        }
    }
    history.add(positions);
}

// Stream 이용 코드
private void moveWithPositions(final List<Boolean> moves) {
   IntStream.range(0, cars.size())
           .filter(index -> moves.get(index))
           .forEach(positions::increase);
   history.add(positions);
}

이 과정에서 forEach를 사용하는 경우가 있었는데, Effective Java의 Item 46가 떠올랐습니다.

forEach는 스트림이 수행한 연산 결과를 보여주는 일(최종 연산)에만 수행되어야 한다.
계산하는데(중간 연산)에 사용하는 것은 스트림 패러다임을 이해하지 못한 채 API만 사용한 예시이다.

 

위 코드에서 forEach는 단순 결과 출력이 아닌 positions라는 외부 상태를 변경하고 있기 때문에 적절하지 않습니다. 따라서 기존 for문을 유지하기로 결정했습니다.

 

그리고 스트림을 사용할 때 side effect를 발생시키지 않고 각 연산의 단계를 생각하면서 사용하기 위해서는 Stream을 공부할 필요가 있다고 느꼈습니다.

 

SOLID 고려하기

OCP : 전략 패턴 적용

초기 코드에서는 자동차의 이동 로직이 Car 클래스에 직접 구현되어 있었습니다.

package racingcar;

public class Car {

    private final String name;

    private long position;

    public Car(final String name) {
        this.name = name;
        this.position = 0;
    }

    public void move() {
        boolean canMove = Randoms.pickNumberInRange(RANDOM_NUMBER_START_INCLUSIVE, RANDOM_NUMBER_END_INCLUSIVE) >= 4;
        if (canMove){
            position++;
        }
    }
}

이 구현은 자동차의 이동 정책이 변경될때마다 Car 클래스를 수정해야 하는 문제가 있었습니다.
따라서 OCP를 지키기 위해 전략 패턴(인터페이스+조합)을 사용했습니다.

public interface MovingStrategy {  
    boolean canMove();  
}

public class RacingCarMovingStrategy implements MovingStrategy {  

    public static final int RANDOM_NUMBER_START_INCLUSIVE = 0;  
    public static final int RANDOM_NUMBER_END_INCLUSIVE = 9;  

    @Override  
    public boolean canMove() {  
        int randomValue = Randoms.pickNumberInRange(RANDOM_NUMBER_START_INCLUSIVE, RANDOM_NUMBER_END_INCLUSIVE);  
        return randomValue >= 4;  
    }  
}

public class Car {  

    private final String name;  
    private final MovingStrategy movingStrategy;  

    private long position;  

    public Car(final String name, final MovingStrategy movingStrategy) {  
        this.name = name;  
        this.position = 0;  
        this.movingStrategy = movingStrategy;  
    }  

    public void move() {  
        if (movingStrategy.canMove()) {  
            position++;  
        }  
    }  

    public String getName() {  
        return name;  
    }  

    public long getPosition() {  
        return position;  
    }  
}

전략 패턴을 사용하면 이동 로직이 변경되더라도 Car 클래스를 수정하지 않고 MovingStrategy 구현체만 주입하면 됩니다.
전략을 setter로 주입할지, 생성자로 주입할지 고민하다 전략이 생성 후에 변경되지 않고 필수 의존성이므로 생성자 주입으로 결정했습니다.

 

SRP: 자동차 경주 이동 전략에서 랜덤값 생성 책임 분리

기존의 RacingCarMovingStrategy는 랜덤값 생성과 이동 여부를 판단이라는 두 가지 책임을 가지고 있었습니다.

public class RacingCarMovingStrategy implements MovingStrategy {  

    public static final int RANDOM_NUMBER_START_INCLUSIVE = 0;  
    public static final int RANDOM_NUMBER_END_INCLUSIVE = 9;  

    @Override  
    public boolean canMove() {  
        int randomValue = Randoms.pickNumberInRange(RANDOM_NUMBER_START_INCLUSIVE, RANDOM_NUMBER_END_INCLUSIVE);  
        return randomValue >= 4;  
    }  
}

단일 책임 원칙(SRP)를 적용하여 각 책임을 별도의 컴포넌트로 분리했습니다.

• RandomNumberGenerator: 랜덤값 생성
• NumberComparable: 숫자 비교
• RacingCarMovingStrategy: 이동 여부 결정

@FunctionalInterface  
public interface RandomNumberGenerator<T extends Number> {  
  T pickNumber();  
}

public class RandomIntegerGenerator implements RandomNumberGenerator<Integer> {  

    private final int randomNumberStartInclusive;  
    private final int randomNumberEndInclusive;  

    public RandomIntegerGenerator(final int randomNumberStartInclusive, final int randomNumberEndInclusive) {  
        this.randomNumberStartInclusive = randomNumberStartInclusive;  
        this.randomNumberEndInclusive = randomNumberEndInclusive;  
    }  

    @Override  
    public Integer pickNumber() {  
        return Randoms.pickNumberInRange(randomNumberStartInclusive, randomNumberEndInclusive);  
    }  
}

@FunctionalInterface  
public interface NumberComparable<T extends Number> {  
    boolean meetsThreshold(T value, T threshold);  
}

public class RacingCarMovingStrategy implements MovingStrategy {  

    private final RandomNumberGenerator randomNumberGenerator;  
    private final NumberComparable<Number> numberComparable;  
    private final int forwardMinInclusive;  

    public RacingCarMovingStrategy(final RandomNumberGenerator randomNumberGenerator,  
                                   final NumberComparable<Number> numberComparable, final int forwardMinInclusive) {  
        this.forwardMinInclusive = forwardMinInclusive;  
        this.randomNumberGenerator = randomNumberGenerator;  
        this.numberComparable = numberComparable;  
    }  

    @Override  
    public boolean canMove() {  
        Number value = randomNumberGenerator.pickNumber();  
        return numberComparable.compare(value, forwardMinInclusive) >= 0;  
    }  
}

또한 랜덤값 생성 범위를 외부에서 설정하게 하여 유연하게 구현하였습니다.
인터페이스를 도입함으로써 테스트용 구현체(ex) 람다)를 주입할 수 있어 테스트가 용이해졌습니다.

@Test  
@DisplayName("랜덤값이 기준값 이상이면 이동한다")  
void 성공_이동_기준값이상() {  
    // Given  
    RandomNumberGenerator randomNumberGenerator = () -> 4;  
    NumberComparable numberComparable = new IntegerComparable();  
    RacingCarMovingStrategy strategy = new RacingCarMovingStrategy(randomNumberGenerator, numberComparable,  
            4);  

    // When & Then  
    assertThat(strategy.canMove()).isTrue();  
}

 

관심사 분리 : toString를 적절하게 사용하자

초기에는 객체의 출력 형식을 toString 메서드를 오버라이드를 하여 구현했습니다.

@Override  
public String toString() {  
    return name.value() + " : " + repeatHyphen(position);  
}

하지만 출력 로직이 객체와 강하게 결합되는 문제가 있었습니다. 출력 형식을 변경하려면 코드를 수정해야합니다. 또한 toString의 본래 용도는 디버깅, 로깅이었으므로 사용자에게 보여주는 view 로직과는 맞지 않았습니다.

 

따라서 출력 책임을 toString() 메서드가 아닌 출력 담당하는 객체(outputHandler)에 두어 관심사를 분리했습니다.

private static void printResult(final List<Car> cars, final long attempt, OutputHandler outputHandler) {  
    outputHandler.showCommentForResult();  
    for (int i = 0; i < attempt; i++) {  
        for (Car car : cars) {  
            car.move();  
            outputHandler.showCarPosition(car.getName(), repeatHyphen(car.getPosition()));  
        }  
        outputHandler.showBlankLine();  
    }  
}

 

불변과 가변 특성을 분리한 객체 설계

처음에 Car 구현시에는 불변 특성(이름, 이동 전략)과 가변 특성(위치)이 함께 존재했습니다.

public class Car {  

    private final Name name;  
    private final MovingStrategy movingStrategy;  

    private long position;  
}

하지만 위치는 이동마다 계속 변경될 수 있는 속성이지만, 위치가 다르다고 다른 자동차가 되는 것은 아닙니다.

자동차의 불변 특성(이름, 이동전략) 과 가변 특성(위치) 를 분리해서 자동차는 움직이는 것에만 집중하고, 위치는 위치를 관리하는 책임만 가지는 것이 적절하다고 생각했습니다(SRP). 후에 게임이 확장되더라도 불변 객체인 Car은 계속 재활용 가능한 장점도 있습니다.

 

Position은 매 이동마다 계속 변경되므로 불변 객체로 설계할 경우 상태 변경마다 새로운 객체를 생성해야 하는 오버헤드가 발생합니다.
따라서 가변 객체로 설계했습니다.

public class Position {  

    private long value;  

    public Position(final long value) {  
        this.value = value;  
    }  

    public Position deepCopy() {  
        return new Position(value);  
    }  

    public void increase(){  
        value++;  
    }  

    public long value() {  
        return value;  
    }  

    @Override  
    public boolean equals(final Object o) {  
        if (this == o) {  
            return true;  
        }  
        if (o == null || getClass() != o.getClass()) {  
            return false;  
        }  
        Position position = (Position) o;  
        return value == position.value;  
    }  

    @Override  
    public int hashCode() {  
        return Objects.hash(value);  
    }  
}

이를 통해 Car의 정체성(이름, 이동 전략)과 상태(위치)가 분리되어 캡슐화 및 도메인을 더 자연스럽게 표현할 수 있게 되었습니다.
또한 위치 변경시 Position 클래스만 수정하면 되므로 불변 객체인 Car을 안전하게 재사용할 수 있게 되었습니다.

 

메모리 최적화 : 객체 생성 줄이기

초기에는 자동차 경주 기록을 관리하기 위해 각 라운드별로 차량의 위치 정보를 객체(Position, Positions)로 저장했습니다.

• Position 객체 : 차량 수 x 라운드 수
• Positions 객체 : 게임 라운드 수

public class History {  
  
    private final List<Positions> history;  
  
    public History(final List<Positions> history) {  
        this.history = new ArrayList<>(history);  
    }  
  
    public void add(Positions positions) {  
        history.add(positions.copy());  
    }  
}

그런데 이렇게 구현하면 Positions 객체가 라운드마다 새로 생성되고, Position 객체도 라운드 수 x 차량 수만큼 생성됩니다.
이는 Position의 가변 특성을 살리지 못하고 불필요하게 객체가 많이 생성되는 문제가 발생됩니다.

 

따라서 위치 정보를 객체 대신 원시 타입(Long)으로 저장하고, Position은 현재 게임 진행에만 사용하도록 변경했습니다.

• Position 객체 : 차량 수
• Positions 객체 : 게임 진행용 1개

이를 통해 Position의 가변 특성을 활용하여 객체 생성 오버헤드를 줄이고 메모리 사용을 최소화했습니다.

public class History {  
  
    private final List<List<Long>> history;  
  
    public History() {  
        this.history = new ArrayList<>();  
    }  
  
    public void add(Positions positions) {  
        history.add(positions.values());  
    }  
  
    public List<Long> at(int round) {  
        return Collections.unmodifiableList(history.get(round));  
    }  
}

이를 통해 Position의 가변 특성을 활용하여 객체 생성 오버헤드를 줄이고 메모리 사용을 최소화했습니다.

 

과도한 추상화 : History에서 List<Long>을 객체로 매핑하지 않은 이유

처음에는 List<Long>이 컬렉션이므로 객체로 분리했습니다.

그런데 List<Long>을 RoundResult와 같은 객체로 분리할 경우에 그 객체가 하는 일이 단순히 Positions객체의 값을 저장하고 조회하는 역할만 수행했습니다.

Positions는 자동차들의 현재 위치를 관리하고, History는 매 라운드마다 Positions를 받아 저장하는데, List<Long>을 래핑한 객체가 굳이 필요할까? 라는 생각이 들었습니다. 과도한 추상화는 아닐까 고민을 많이 해서 결론적으로 이 설계를 선택했습니다.

 

MVC 패턴 적용

다양한 책임을 분리하기 위해 MVC 패턴을 적용했습니다.

• Application: 구성
• Controller: 흐름 제어
• Model: 비즈니스 로직
• View: 입출력

public class Application {  
  
    private static final String DELIMITER = ",";  
    private static final int RANDOM_NUMBER_START_INCLUSIVE = 0;  
    private static final int RANDOM_NUMBER_END_INCLUSIVE = 9;  
    private static final int FORWARD_MIN_INCLUSIVE = 4;  
    private static final String HYPHEN = "-";  
  
    public static void main(String[] args) {  
        InputView inputView = new ConsoleInputView();  
        OutputView outputView = new ConsoleOutputView();  
        Splitter splitter = new Splitter(DELIMITER);  
        RandomNumberGenerator randomNumberGenerator = new RandomIntegerGenerator(RANDOM_NUMBER_START_INCLUSIVE,  
                RANDOM_NUMBER_END_INCLUSIVE);  
        NumberComparable numberComparable = new IntegerComparable();  
        MovingStrategy movingStrategy = new RacingCarMovingStrategy(randomNumberGenerator, numberComparable,  
                FORWARD_MIN_INCLUSIVE);  
        StringRepeater stringRepeater = new StringRepeater(HYPHEN);  
  
        RacingCarController controller = new RacingCarController(inputView, outputView, splitter, movingStrategy,  
                stringRepeater);  
        controller.process();  
        Console.close();  
    }  
}

이를 통해 관심사가 분리되었습니다.

 

느낀점

테스트 커버리지와 의미있는 테스트

초기에는 테스트 커버리지를 100% 달성하는 것을 목표로 했지만, 이 목표가 현실적이지 않고 오히려 불필요하다는 것을 깨달았습니다.
자동 생성된 코드인 equals, hashCode나 단순 getter, toString은 굳이 테스트할 필요가 없습니다.

 

작성한 테스트에서도 90%의 메서드 커버리지와 87%의 라인 커버리지에서도 핵심 로직들은 다 테스트가 되었습니다.

 

따라서 단순히 높은 커버리지를 목표로 하기보다는, 실제로 의미있는 테스트를 작성하는 것이 더 중요하다는 것을 깨달았습니다.